Error handling

app/config.php

@@ -4,6 +4,7 @@
 
 use Colors\Color;
 use PhpSchool\PhpWorkshop\Check\FileComparisonCheck;
+use PhpSchool\PhpWorkshop\ExerciseRunner\Factory\ServerRunnerFactory;
 use PhpSchool\PhpWorkshop\Listener\InitialCodeListener;
 use PhpSchool\PhpWorkshop\Listener\TearDownListener;
 use PhpSchool\PhpWorkshop\Logger\ConsoleLogger;
@@ -96,6 +97,9 @@
     'currentWorkingDirectory' => function (ContainerInterface $c) {
         return getcwd();
     },
+    'basePath' => DI\decorate(function (string $previous) {
+        return canonicalise_path($previous);
+    }),
     WorkshopType::class => WorkshopType::STANDARD(),
     Psr\Log\LoggerInterface::class => function (ContainerInterface $c) {
         $appName = $c->get('appName');
@@ -150,7 +154,7 @@
         return $colors;
     },
     OutputInterface::class => function (ContainerInterface $c) {
-        return new StdOutput($c->get(Color::class), $c->get(Terminal::class));
+        return new StdOutput($c->get(Color::class), $c->get(Terminal::class), $c->get('basePath'));
     },
 
     ExerciseRepository::class => function (ContainerInterface $c) {
@@ -166,10 +170,10 @@
 
     //Exercise Runners
     RunnerManager::class => function (ContainerInterface $c) {
-        $manager = new RunnerManager;
+        $manager = new RunnerManager();
         $manager->addFactory(new CliRunnerFactory($c->get(EventDispatcher::class)));
         $manager->addFactory(new CgiRunnerFactory($c->get(EventDispatcher::class), $c->get(RequestRenderer::class)));
-        $manager->addFactory(new CustomVerifyingRunnerFactory);
+        $manager->addFactory(new CustomVerifyingRunnerFactory());
         return $manager;
     },
 

src/Application.php

@@ -204,9 +204,9 @@ public function configure(bool $debugMode = false): ContainerInterface
             }
         }
 
-        set_error_handler(function () use ($container): bool {
-            $this->tearDown($container);
-            return false; // Use default error handler
+        //turn all notices & warnings in to exceptions
+        set_error_handler(function (int $severity, string $message, string $file, int $line) {
+            throw new \ErrorException($message, 0, $severity, $file, $line);
         });
 
         $this->container = $container;
@@ -259,12 +259,7 @@ public function run(): int
 
             $container
                 ->get(OutputInterface::class)
-                ->printError(
-                    sprintf(
-                        '%s',
-                        $message
-                    )
-                );
+                ->printException($e);
             return 1;
         }
 
@@ -280,13 +275,18 @@ public function run(): int
     private function getContainer(bool $debugMode): Container
     {
         $containerBuilder = new ContainerBuilder();
-        $containerBuilder->addDefinitions(
-            array_merge_recursive(
-                require $this->frameworkConfigLocation,
-                require $this->diConfigFile
-            )
+        $diConfig = require $this->diConfigFile;
+        $fwConfig = require $this->frameworkConfigLocation;
+
+        $fwConfig['eventListeners'] = array_merge_recursive(
+            $fwConfig['eventListeners'],
+            $diConfig['eventListeners'] ?? []
         );
 
+        unset($diConfig['eventListeners']);
+
+        $containerBuilder->addDefinitions($diConfig, $fwConfig);
+
         $containerBuilder->addDefinitions(
             [
                 'workshopTitle' => $this->workshopTitle,

src/Check/CodeExistsCheck.php

@@ -52,7 +52,7 @@ public function handleError(Error $error): void
 
         if (!$empty) {
             $openingTag = is_array($statements) && count($statements) === 1 ? $statements[0] : null;
-            $empty = $openingTag instanceof InlineHTML ? in_array($openingTag->value, ['<?php', '<?']) : false;
+            $empty = $openingTag instanceof InlineHTML && in_array($openingTag->value, ['<?php', '<?']);
         }
 
         if ($empty) {

src/Listener/InitialCodeListener.php

@@ -48,7 +48,6 @@ public function __invoke(Event $event): void
         }
 
         foreach ($exercise->getInitialCode()->getFiles() as $file) {
-            /** @var SolutionFile $file */
             if (!file_exists($this->workingDirectory . '/' . $file->getRelativePath())) {
                 copy($file->getAbsolutePath(), $this->workingDirectory . '/' . $file->getRelativePath());
                 $message = 'File successfully copied to working directory';

src/Logger/Logger.php

@@ -21,7 +21,7 @@ public function __construct(string $filePath)
 
     public function log($level, $message, array $context = []): void
     {
-        if (!file_exists($this->filePath)) {
+        if (!file_exists(dirname($this->filePath))) {
             if (!mkdir($concurrentDirectory = dirname($this->filePath), 0777, true) && !is_dir($concurrentDirectory)) {
                 throw new \RuntimeException(sprintf('Directory "%s" was not created', $concurrentDirectory));
             }

src/Output/NullOutput.php

@@ -4,13 +4,20 @@
 
 namespace PhpSchool\PhpWorkshop\Output;
 
+use Throwable;
+
 class NullOutput implements OutputInterface
 {
     public function printError(string $error): void
     {
         // noop
     }
 
+    public function printException(Throwable $exception): void
+    {
+        // noop
+    }
+
     public function write(string $content): void
     {
         // noop

src/Output/OutputInterface.php

@@ -4,6 +4,8 @@
 
 namespace PhpSchool\PhpWorkshop\Output;
 
+use Throwable;
+
 /**
  * Output interface
  */
@@ -17,6 +19,14 @@ interface OutputInterface
      */
     public function printError(string $error): void;
 
+    /**
+     * Write an Exception. Should be decorated in someway
+     * which highlights the severity.
+     *
+     * @param Throwable $exception
+     */
+    public function printException(Throwable $exception): void;
+
     /**
      * Write a string to the output.
      *

src/Output/StdOutput.php

@@ -6,6 +6,7 @@
 
 use Colors\Color;
 use PhpSchool\Terminal\Terminal;
+use Throwable;
 
 /**
  * Console output interface
@@ -22,10 +23,16 @@ class StdOutput implements OutputInterface
      */
     private $terminal;
 
-    public function __construct(Color $color, Terminal $terminal)
+    /**
+     * @var string
+     */
+    private $workshopBasePath;
+
+    public function __construct(Color $color, Terminal $terminal, string $workshopBasePath = '')
     {
         $this->color = $color;
         $this->terminal = $terminal;
+        $this->workshopBasePath = $workshopBasePath;
     }
 
     /**
@@ -41,6 +48,51 @@ public function printError(string $error): void
         echo "\n";
     }
 
+    /**
+     * @param Throwable $exception
+     */
+    public function printException(Throwable $exception): void
+    {
+        $message = $exception->getMessage();
+        if (strpos($message, $this->workshopBasePath) !== null) {
+            $message = str_replace($this->workshopBasePath, '', $message);
+        }
+
+        $file = $exception->getFile();
+        if (strpos($file, $this->workshopBasePath) !== null) {
+            $file = str_replace($this->workshopBasePath, '', $file);
+        }
+
+        $lines = [
+            sprintf("In %s line %d:", $file, $exception->getLine()),
+            sprintf("[%s (%s)]:", get_class($exception), $exception->getCode()),
+            '',
+            $message
+        ];
+
+        $length = max(array_map('strlen', $lines)) + 2;
+        $this->emptyLine();
+        $this->writeLine(' ' . $this->color->__invoke(str_repeat(' ', $length))->bg_red());
+
+        foreach ($lines as $line) {
+            $line = str_pad($line, $length - 2, ' ', STR_PAD_RIGHT);
+            $this->writeLine(' ' . $this->color->__invoke(" $line ")->bg_red()->white()->bold());
+        }
+
+        $this->writeLine(' ' . $this->color->__invoke(str_repeat(' ', $length))->bg_red());
+        $this->emptyLine();
+
+        $this->writeLine(
+            implode(
+                "\n",
+                array_map(function ($l) {
+                    return " $l";
+                },
+                explode("\n", $exception->getTraceAsString()))
+            )
+        );
+    }
+
     /**
      * Write a title section. Should be decorated in a way which makes
      * the title stand out.

src/UserStateSerializer.php

@@ -204,6 +204,8 @@ private function readJson(string $filePath): ?array
      */
     private function wipeFile(): void
     {
-        @unlink($this->path);
+        if (file_exists(sprintf('%s/%s', $this->path, static::SAVE_FILE))) {
+            unlink(sprintf('%s/%s', $this->path, static::SAVE_FILE));
+        }
     }
 }

test/ApplicationTest.php

@@ -100,7 +100,7 @@ public function testExceptionIsThrownIfResultRendererClassDoesNotExist(): void
 
     public function testTearDownEventIsFiredOnApplicationException(): void
     {
-        $configFile = $this->getTemporaryFile('config.php', '<?php return [];');
+        $configFile = $this->getTemporaryFile('config.php', '<?php return ["basePath" => __DIR__ . "../"];');
         $application = new Application('Testing TearDown', $configFile);
 
         $container = $application->configure();
@@ -125,7 +125,7 @@ public function testTearDownEventIsFiredOnApplicationException(): void
 
     public function testLoggingExceptionDuringTearDown(): void
     {
-        $configFile = $this->getTemporaryFile('config.php', '<?php return [];');
+        $configFile = $this->getTemporaryFile('config.php', '<?php return ["basePath" => __DIR__ . "../"];');
         $application = new Application('Testing tear down logging', $configFile);
         $exception = new \Exception('Unexpected error');
 
@@ -159,15 +159,15 @@ static function () use ($exception) {
 
     public function testConfigureReturnsSameContainerInstance(): void
     {
-        $configFile = $this->getTemporaryFile('config.php', '<?php return [];');
+        $configFile = $this->getTemporaryFile('config.php', '<?php return ["basePath" => __DIR__ . "../"];');
         $application = new Application('Testing Configure', $configFile);
 
         self::assertSame($application->configure(), $application->configure());
     }
 
     public function testDebugFlagSwitchesLoggerToConsoleLogger(): void
     {
-        $configFile = $this->getTemporaryFile('config.php', '<?php return [];');
+        $configFile = $this->getTemporaryFile('config.php', '<?php return ["basePath" => __DIR__ . "../"];');
         $application = new Application('My workshop', $configFile);
         $container = $application->configure(true);
 

test/BaseTest.php

@@ -44,8 +44,8 @@ public function getTemporaryFile(string $filename, string $content = null): stri
 
     public function tearDown(): void
     {
-        if ($this->tempDirectory) {
-            (new Filesystem())->remove($this->tempDirectory);
+        if (file_exists(System::tempDir($this->getName()))) {
+            (new Filesystem())->remove(System::tempDir($this->getName()));
         }
     }
 }

test/Logger/ConsoleLoggerTest.php

@@ -18,6 +18,7 @@ public function setUp(): void
 
         $this->container->set('phpschoolGlobalDir', $this->getTemporaryDirectory());
         $this->container->set('appName', 'my-workshop');
+        $this->container->set('basePath', __DIR__ . '/../');
         $this->container->set('debugMode', true);
     }
 

test/ResultRenderer/ResultsRendererTest.php

@@ -7,6 +7,7 @@
 use Kadet\Highlighter\KeyLighter;
 use Kadet\Highlighter\Language\Php;
 use PhpSchool\PhpWorkshopTest\Asset\CliExerciseImpl;
+use PhpSchool\PhpWorkshopTest\BaseTest;
 use PhpSchool\Terminal\Terminal;
 use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface;
 use PhpSchool\PhpWorkshop\Exercise\ProvidesSolution;
@@ -22,7 +23,7 @@
 use PhpSchool\PhpWorkshop\UserState;
 use PHPUnit\Framework\TestCase;
 
-class ResultsRendererTest extends TestCase
+class ResultsRendererTest extends BaseTest
 {
     public function testRenderIndividualResult(): void
     {
@@ -118,9 +119,7 @@ public function testRenderSuccessWithSolution(): void
         $exerciseRepo = $this->createMock(ExerciseRepository::class);
         $exerciseRepo->method('count')->willReturn(2);
 
-        $tmpFile = sprintf('%s/%s/some-file', sys_get_temp_dir(), $this->getName());
-        mkdir(dirname($tmpFile));
-        file_put_contents($tmpFile, 'FILE CONTENTS');
+        $tmpFile = $this->getTemporaryFile('some-file', 'FILE CONTENTS');
 
         $exercise = new CliExerciseImpl();
         $exercise->setSolution(new SingleFileSolution($tmpFile));
@@ -146,9 +145,6 @@ public function testRenderSuccessWithSolution(): void
             new UserState(['exercise1']),
             new StdOutput($color, $terminal)
         );
-
-        unlink($tmpFile);
-        rmdir(dirname($tmpFile));
     }
 
     public function testRenderSuccessWithPhpSolutionFileIsSyntaxHighlighted(): void
@@ -164,9 +160,7 @@ public function testRenderSuccessWithPhpSolutionFileIsSyntaxHighlighted(): void
         $exerciseRepo = $this->createMock(ExerciseRepository::class);
         $exerciseRepo->method('count')->willReturn(2);
 
-        $tmpFile = sprintf('%s/%s/some-file.php', sys_get_temp_dir(), $this->getName());
-        mkdir(dirname($tmpFile));
-        file_put_contents($tmpFile, 'FILE CONTENTS');
+        $tmpFile = $this->getTemporaryFile('some-file.php', 'FILE CONTENTS');
 
         $exercise = new CliExerciseImpl();
         $exercise->setSolution(new SingleFileSolution($tmpFile));
@@ -200,9 +194,6 @@ public function testRenderSuccessWithPhpSolutionFileIsSyntaxHighlighted(): void
             new UserState(['exercise1']),
             new StdOutput($color, $terminal)
         );
-
-        unlink($tmpFile);
-        rmdir(dirname($tmpFile));
     }
 
     public function testRenderSuccessAndFailure(): void