Solve syntax error reporting and other minor improvements

Dockerfile

@@ -15,7 +15,8 @@ RUN curl -L -o phpunit-9.phar https://phar.phpunit.de/phpunit-9.phar && \
 WORKDIR /usr/local/bin/junit-handler/
 COPY --from=composer:2.5.8 /usr/bin/composer /usr/local/bin/composer
 COPY junit-handler/ .
-RUN composer install --no-interaction 
+# We need PHPUnit from junit-handler/ to run test-runner tests in CI / locally
+RUN composer install --no-interaction
 
 FROM php:8.2.7-cli-alpine3.18 AS runtime
 

README.md

@@ -1,25 +1,42 @@
-![Jest Tests](https://github.com/exercism/php-test-runner/workflows/Test%20JUnit-to-JSON/badge.svg) ![Smoke Test](https://github.com/exercism/php-test-runner/workflows/Smoke%20Test/badge.svg) ![Tooling image pushed](https://github.com/exercism/php-test-runner/workflows/Push%20Docker%20images%20to%20DockerHub%20and%20ECR/badge.svg)
+![JUnit to JSON Tests](https://github.com/exercism/php-test-runner/workflows/Test%20JUnit-to-JSON/badge.svg) ![Smoke Test](https://github.com/exercism/php-test-runner/workflows/Smoke%20Test/badge.svg) ![Tooling image pushed](https://github.com/exercism/php-test-runner/workflows/Deploy/badge.svg)
 
 # PHP Test Runner
 
-This is a minimal test runner for Exercism's v3 platform.  It meets the minimal spec for testing _practice exercises_.  It does not currently parse the test case code being run, therefore it does not meet the standard for testing _concept exercises_.
+This is the test runner for Exercism's v3 platform.
+It meets the complete spec for testing all exercises.
 
 ## Basic components
 
-### Dockerimage
+### Docker image
 
-The website uses isolated docker images to run untrusted code in a sandbox.  Image consists of PHP 8.2.7 (PHPUnit 9/10). All final assets are built into the image, because the image does not have network access once in use.
+The website uses isolated docker images to run untrusted code in a sandbox.
+The image provided by this repository consists of PHP 8.2.7 (PHPUnit 9/10).
+All final assets are built into the image, because the image does not have network access once in use.
 
-Includes php extensions: ds, intl
+Includes PHP extensions: ds, intl
 
 ### Test runner
 
 Test running a solution is coordinated by a bash script at `bin/run.sh` taking 3 positional arguments:
 
 ```text
-> bin/run.sh <test-slug> <directory path to solution> <directory path for output>
+bin/run.sh <test-slug> <directory path to solution> <directory path for output>
 ```
 
+This is what runs inside the production Docker image when students submit their code.
+
+### Testing the test runner
+
+In `./tests/` are golden tests to verify that the test runner behaves as expected.
+The CI uses `bin/run-tests.sh` to execute them.
+
+### Running tests in Docker locally
+
+This is the recommended way to use this locally.
+Use `bin/run-in-docker.sh <test-slug> <directory path to solution> <directory path for output>` and `bin/run-tests-in-docker.sh` to locally build and run the Docker image.
+
 ### JUnit to JSON
 
-PHPUnit can natively output tests run to junit xml format, but Exercism requires output in json format. A php-based app is located in the `junit-handler` folder. It provides a translation layer from one format to the other incorporating task_id identification and test code inclusion.
+PHPUnit can natively output tests run to JUnit XML format, but Exercism requires output in JSON format.
+A PHP-based app is located in the `junit-handler` folder.
+It provides a translation layer from one format to the other incorporating `task_id` identification and test code inclusion.

bin/run.sh

@@ -7,13 +7,22 @@ XML_RESULTS='results.xml'
 JSON_RESULTS='results.json'
 
 function main {
-  exercise_slug="${1}"
-  solution_dir="${2}"
-  output_dir="${3}"
+  local output=""
+  local test_files=""
+  local -i phpunit_exit_code
+
+  # local exercise_slug="${1}"
+  local solution_dir="${2}"
+  local output_dir="${3}"
   test_files=$(find "${solution_dir}" -type f -name '*Test.php' | tr '\n' ' ')
 
   set +e
-  phpunit_output=$(eval "${PHPUNIT_BIN}" \
+  if ! output=$(php -l "${solution_dir}"/*.php 2>&1 1>/dev/null); then
+    jo version=3 status=error message="${output/"$solution_dir/"/""}" tests="[]" > "${output_dir%/}/${JSON_RESULTS}"
+    return 0;
+  fi
+
+  output=$(eval "${PHPUNIT_BIN}" \
     -d memory_limit=300M \
     --log-junit "${output_dir%/}/${XML_RESULTS}" \
     --verbose \
@@ -23,8 +32,11 @@ function main {
   phpunit_exit_code=$?
   set -e
 
+  # This is only a theoretical failure case. This exit code is generated, when
+  # PHPUnit fails to catch some issue in its internals. It cannot be provoked
+  # by us for testing our code
   if [[ "${phpunit_exit_code}" -eq 255 ]]; then
-    jo version=2 status=error message="${phpunit_output}" tests="[]" > "${output_dir%/}/${JSON_RESULTS}"
+    jo version=3 status=error message="${output/"$solution_dir/"/""}" tests="[]" > "${output_dir%/}/${JSON_RESULTS}"
     return 0;
   fi
 
@@ -34,14 +46,16 @@ function main {
 }
 
 function installed {
+  local cmd
+
   cmd=$(command -v "${1}")
 
   [[ -n "${cmd}" ]] && [[ -f "${cmd}" ]]
   return ${?}
 }
 
 function die {
-  >&2 echo "❌ Fatal: ${@}"
+  >&2 echo "❌ Fatal: $*"
   exit 1
 }
 

junit-handler/src/Handler.php

@@ -14,6 +14,7 @@ class Handler
     private const STATUS_ERROR = 'error';
     private const STATUS_PASS = 'pass';
     private const STATUS_FAIL = 'fail';
+    private string $test_file_path = '';
 
     public function run(string $xml_path, $json_path): void
     {
@@ -38,6 +39,9 @@ public function run(string $xml_path, $json_path): void
 
         $test_class = $testsuite_attrs['name'];
         $test_file_path = $testsuite_attrs['file'];
+        $test_file_name = \basename($test_file_path);
+        $this->test_file_path = \str_replace($test_file_name, '', $test_file_path);
+
 
         $testcase_error_count = (int) $testsuite_attrs['errors'];
         $testcase_failure_count = (int) $testsuite_attrs['failures'];
@@ -165,10 +169,10 @@ private function parseTestCases(
                     $output['output'] = (string) $data;
                 } elseif ($name === 'error') {
                     $output['status'] = self::STATUS_ERROR;
-                    $output['message'] = (string) $data;
+                    $output['message'] = \str_replace($this->test_file_path, '', (string) $data);
                 } elseif ($name === 'failure') {
                     $output['status'] = self::STATUS_FAIL;
-                    $output['message'] = (string) $data;
+                    $output['message'] = \str_replace($this->test_file_path, '', (string) $data);
                 }
             }
 

tests/all-fail/Leap.php

@@ -4,5 +4,5 @@
 
 function isLeap(int $year): bool
 {
-    return 1(!($year % 4) && (!!($year % 100) || !($year % 400)));
+    throw new \BadFunctionCallException("Implement the isLeap function");
 }

tests/all-fail/expected_results.json

@@ -1 +1 @@
-{"version":3,"status":"fail","tests":[{"name":"testLeapYear","status":"error","test_code":"$this->assertTrue(isLeap(1996));\n","message":"LeapTest::testLeapYear\nParseError: syntax error, unexpected token \"(\", expecting \";\"\n\n\/opt\/test-runner\/tests\/all-fail\/Leap.php:7"},{"name":"testNonLeapYear","status":"pass","test_code":"$this->assertFalse(isLeap(1997));\n"},{"name":"testNonLeapEvenYear","status":"pass","test_code":"$this->assertFalse(isLeap(1998));\n"},{"name":"testCentury","status":"pass","test_code":"$this->assertFalse(isLeap(1900));\n"},{"name":"testFourthCentury","status":"pass","test_code":"$this->assertTrue(isLeap(2400));\n"}]}
+{"version":3,"status":"fail","tests":[{"name":"testLeapYear","status":"error","test_code":"$this->assertTrue(isLeap(1996));\n","message":"LeapTest::testLeapYear\nBadFunctionCallException: Implement the isLeap function\n\nLeap.php:7\nLeapTest.php:36"},{"name":"testNonLeapYear","status":"error","test_code":"$this->assertFalse(isLeap(1997));\n","message":"LeapTest::testNonLeapYear\nBadFunctionCallException: Implement the isLeap function\n\nLeap.php:7\nLeapTest.php:41"},{"name":"testNonLeapEvenYear","status":"error","test_code":"$this->assertFalse(isLeap(1998));\n","message":"LeapTest::testNonLeapEvenYear\nBadFunctionCallException: Implement the isLeap function\n\nLeap.php:7\nLeapTest.php:46"},{"name":"testCentury","status":"error","test_code":"$this->assertFalse(isLeap(1900));\n","message":"LeapTest::testCentury\nBadFunctionCallException: Implement the isLeap function\n\nLeap.php:7\nLeapTest.php:51"},{"name":"testFourthCentury","status":"error","test_code":"$this->assertTrue(isLeap(2400));\n","message":"LeapTest::testFourthCentury\nBadFunctionCallException: Implement the isLeap function\n\nLeap.php:7\nLeapTest.php:56"}]}

tests/empty-file/expected_results.json

@@ -1 +1 @@
-{"version":3,"status":"fail","tests":[{"name":"testLeapYear","status":"error","test_code":"$this->assertTrue(isLeap(1996));\n","message":"LeapTest::testLeapYear\nError: Call to undefined function isLeap()\n\n\/opt\/test-runner\/tests\/empty-file\/LeapTest.php:36"},{"name":"testNonLeapYear","status":"error","test_code":"$this->assertFalse(isLeap(1997));\n","message":"LeapTest::testNonLeapYear\nError: Call to undefined function isLeap()\n\n\/opt\/test-runner\/tests\/empty-file\/LeapTest.php:41"},{"name":"testNonLeapEvenYear","status":"error","test_code":"$this->assertFalse(isLeap(1998));\n","message":"LeapTest::testNonLeapEvenYear\nError: Call to undefined function isLeap()\n\n\/opt\/test-runner\/tests\/empty-file\/LeapTest.php:46"},{"name":"testCentury","status":"error","test_code":"$this->assertFalse(isLeap(1900));\n","message":"LeapTest::testCentury\nError: Call to undefined function isLeap()\n\n\/opt\/test-runner\/tests\/empty-file\/LeapTest.php:51"},{"name":"testFourthCentury","status":"error","test_code":"$this->assertTrue(isLeap(2400));\n","message":"LeapTest::testFourthCentury\nError: Call to undefined function isLeap()\n\n\/opt\/test-runner\/tests\/empty-file\/LeapTest.php:56"}]}
+{"version":3,"status":"fail","tests":[{"name":"testLeapYear","status":"error","test_code":"$this->assertTrue(isLeap(1996));\n","message":"LeapTest::testLeapYear\nError: Call to undefined function isLeap()\n\nLeapTest.php:36"},{"name":"testNonLeapYear","status":"error","test_code":"$this->assertFalse(isLeap(1997));\n","message":"LeapTest::testNonLeapYear\nError: Call to undefined function isLeap()\n\nLeapTest.php:41"},{"name":"testNonLeapEvenYear","status":"error","test_code":"$this->assertFalse(isLeap(1998));\n","message":"LeapTest::testNonLeapEvenYear\nError: Call to undefined function isLeap()\n\nLeapTest.php:46"},{"name":"testCentury","status":"error","test_code":"$this->assertFalse(isLeap(1900));\n","message":"LeapTest::testCentury\nError: Call to undefined function isLeap()\n\nLeapTest.php:51"},{"name":"testFourthCentury","status":"error","test_code":"$this->assertTrue(isLeap(2400));\n","message":"LeapTest::testFourthCentury\nError: Call to undefined function isLeap()\n\nLeapTest.php:56"}]}

tests/partial-fail/expected_results.json

@@ -1 +1 @@
-{"version":3,"status":"fail","tests":[{"name":"testLeapYear","status":"pass","test_code":"$this->assertTrue(isLeap(1996));\n"},{"name":"testNonLeapYear","status":"pass","test_code":"$this->assertFalse(isLeap(1997));\n"},{"name":"testNonLeapEvenYear","status":"pass","test_code":"$this->assertFalse(isLeap(1998));\n"},{"name":"testCentury","status":"pass","test_code":"$this->assertFalse(isLeap(1900));\n"},{"name":"testFourthCentury","status":"fail","test_code":"$this->assertTrue(isLeap(2400));\n","message":"LeapTest::testFourthCentury\nFailed asserting that false is true.\n\n\/opt\/test-runner\/tests\/partial-fail\/LeapTest.php:56"}]}
+{"version":3,"status":"fail","tests":[{"name":"testLeapYear","status":"pass","test_code":"$this->assertTrue(isLeap(1996));\n"},{"name":"testNonLeapYear","status":"pass","test_code":"$this->assertFalse(isLeap(1997));\n"},{"name":"testNonLeapEvenYear","status":"pass","test_code":"$this->assertFalse(isLeap(1998));\n"},{"name":"testCentury","status":"pass","test_code":"$this->assertFalse(isLeap(1900));\n"},{"name":"testFourthCentury","status":"fail","test_code":"$this->assertTrue(isLeap(2400));\n","message":"LeapTest::testFourthCentury\nFailed asserting that false is true.\n\nLeapTest.php:56"}]}

tests/syntax-error/expected_results.json

@@ -1 +1 @@
-{"version":3,"status":"fail","tests":[{"name":"testLeapYear","status":"error","test_code":"$this->assertTrue(isLeap(1996));\n","message":"LeapTest::testLeapYear\nParseError: syntax error, unexpected token \"@\", expecting \"(\"\n\n\/opt\/test-runner\/tests\/syntax-error\/Leap.php:5"},{"name":"testNonLeapYear","status":"pass","test_code":"$this->assertFalse(isLeap(1997));\n"},{"name":"testNonLeapEvenYear","status":"pass","test_code":"$this->assertFalse(isLeap(1998));\n"},{"name":"testCentury","status":"pass","test_code":"$this->assertFalse(isLeap(1900));\n"},{"name":"testFourthCentury","status":"pass","test_code":"$this->assertTrue(isLeap(2400));\n"}]}
+{"version":3,"status":"error","message":"PHP Parse error:  syntax error, unexpected token \"@\", expecting \"(\" in Leap.php on line 5","tests":[]}