Skip to content

Log file patching & don't revert patch when in debug mode #224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/config.php
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,11 @@
},
PrepareSolutionListener::class => create(),
CodePatchListener::class => function (ContainerInterface $c) {
return new CodePatchListener($c->get(CodePatcher::class));
return new CodePatchListener(
$c->get(CodePatcher::class),
$c->get(LoggerInterface::class),
$c->get('debugMode')
);
},
SelfCheckListener::class => function (ContainerInterface $c) {
return new SelfCheckListener($c->get(ResultAggregator::class));
Expand Down
24 changes: 23 additions & 1 deletion src/Listener/CodePatchListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use PhpSchool\PhpWorkshop\Event\EventInterface;
use PhpSchool\PhpWorkshop\Event\ExerciseRunnerEvent;
use PhpSchool\PhpWorkshop\Exercise\ProvidesSolution;
use Psr\Log\LoggerInterface;
use RuntimeException;

/**
Expand All @@ -20,17 +21,31 @@ class CodePatchListener
*/
private $codePatcher;

/**
* @var LoggerInterface
*/
private $logger;

/**
* @var bool
*/
private $debugMode;

/**
* @var array<string, string>
*/
private $originalCode = [];

/**
* @param CodePatcher $codePatcher
* @param LoggerInterface $logger
* @param bool $debugMode
*/
public function __construct(CodePatcher $codePatcher)
public function __construct(CodePatcher $codePatcher, LoggerInterface $logger, bool $debugMode)
{
$this->codePatcher = $codePatcher;
$this->logger = $logger;
$this->debugMode = $debugMode;
}

/**
Expand All @@ -46,6 +61,8 @@ public function patch(ExerciseRunnerEvent $event): void
}

foreach (array_filter($files) as $fileName) {
$this->logger->debug("Patching file: $fileName");

$this->originalCode[$fileName] = (string) file_get_contents($fileName);

file_put_contents(
Expand All @@ -64,6 +81,11 @@ public function revert(EventInterface $event): void
return;
}

//if we're in debug mode leave the students patch for debugging
if ($event instanceof ExerciseRunnerEvent && $this->debugMode) {
unset($this->originalCode[$event->getInput()->getArgument('program')]);
}

foreach ($this->originalCode as $fileName => $contents) {
file_put_contents($fileName, $contents);
}
Expand Down
6 changes: 6 additions & 0 deletions src/TestUtils/WorkshopExerciseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use PhpSchool\PhpWorkshop\ResultAggregator;
use PhpSchool\PhpWorkshop\Utils\ArrayObject;
use PhpSchool\PhpWorkshop\Utils\Collection;
use PhpSchool\PhpWorkshop\Utils\System;
use PHPUnit\Framework\ExpectationFailedException;
use PHPUnit\Framework\TestCase;
use Psr\Container\ContainerInterface;
Expand Down Expand Up @@ -53,6 +54,11 @@ public function setUp(): void
$this->container = $this->app->configure();
}

public function tearDown(): void
{
(new Filesystem())->remove(System::tempDir());
}

/**
* @return class-string
*/
Expand Down
56 changes: 51 additions & 5 deletions test/Listener/CodePatchListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
use PhpSchool\PhpWorkshop\Utils\System;
use PhpSchool\PhpWorkshopTest\Asset\ProvidesSolutionExercise;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Symfony\Component\Filesystem\Filesystem;

class CodePatchListenerTest extends TestCase
Expand Down Expand Up @@ -47,6 +49,11 @@ public function setUp(): void
touch($this->solution);
}

public function tearDown(): void
{
$this->filesystem->remove(dirname($this->file));
}

public function testPatchUpdatesCode(): void
{
file_put_contents($this->file, 'ORIGINAL CONTENT');
Expand All @@ -60,7 +67,7 @@ public function testPatchUpdatesCode(): void
->with($exercise, 'ORIGINAL CONTENT')
->willReturn('MODIFIED CONTENT');

$listener = new CodePatchListener($this->codePatcher);
$listener = new CodePatchListener($this->codePatcher, new NullLogger(), false);
$event = new ExerciseRunnerEvent('event', $exercise, $input);
$listener->patch($event);

Expand All @@ -80,7 +87,7 @@ public function testRevertAfterPatch(): void
->with($exercise, 'ORIGINAL CONTENT')
->willReturn('MODIFIED CONTENT');

$listener = new CodePatchListener($this->codePatcher);
$listener = new CodePatchListener($this->codePatcher, new NullLogger(), false);
$event = new ExerciseRunnerEvent('event', $exercise, $input);
$listener->patch($event);
$listener->revert($event);
Expand All @@ -101,16 +108,55 @@ public function testPatchesProvidedSolution(): void
->withConsecutive([$exercise, 'ORIGINAL CONTENT'], [$exercise, "<?php\n\necho 'Hello World';\n"])
->willReturn('MODIFIED CONTENT');

$listener = new CodePatchListener($this->codePatcher);
$listener = new CodePatchListener($this->codePatcher, new NullLogger(), false);
$event = new ExerciseRunnerEvent('event', $exercise, $input);
$listener->patch($event);

self::assertStringEqualsFile($this->file, 'MODIFIED CONTENT');
self::assertStringEqualsFile($exercise->getSolution()->getEntryPoint(), 'MODIFIED CONTENT');
}

public function tearDown(): void
public function testFileIsLoggedWhenPatches(): void
{
$this->filesystem->remove(dirname($this->file));
file_put_contents($this->file, 'ORIGINAL CONTENT');

$input = new Input('app', ['program' => $this->file]);
$exercise = $this->createMock(ExerciseInterface::class);

$this->codePatcher
->expects($this->once())
->method('patch')
->with($exercise, 'ORIGINAL CONTENT')
->willReturn('MODIFIED CONTENT');

$logger = $this->createMock(LoggerInterface::class);
$logger->expects($this->once())
->method('debug')
->with('Patching file: ' . $this->file);

$listener = new CodePatchListener($this->codePatcher, $logger, false);
$event = new ExerciseRunnerEvent('event', $exercise, $input);
$listener->patch($event);
}

public function testRevertDoesNotRevertStudentSubmissionPatchIfInDebugMode(): void
{
file_put_contents($this->file, 'ORIGINAL CONTENT');

$input = new Input('app', ['program' => $this->file]);
$exercise = $this->createMock(ExerciseInterface::class);

$this->codePatcher
->expects($this->once())
->method('patch')
->with($exercise, 'ORIGINAL CONTENT')
->willReturn('MODIFIED CONTENT');

$listener = new CodePatchListener($this->codePatcher, new NullLogger(), true);
$event = new ExerciseRunnerEvent('event', $exercise, $input);
$listener->patch($event);
$listener->revert($event);

self::assertStringEqualsFile($this->file, 'MODIFIED CONTENT');
}
}