From 06e15674898b9856bd47c1427ec322bd75b1a5ba Mon Sep 17 00:00:00 2001 From: Michael Kramer Date: Sat, 27 Apr 2024 17:55:35 +0200 Subject: [PATCH 1/6] Introduce teamcity log based output handling - User input is not logged in JUnit log anymore - There is no way to make JUnit logs with the output (--fail-on-risky is not accounted for!) - To get the output logged in teamcity logger, the test must fail (--disallow-test-output --fail-on-risky) - Use the teamcity log to add output to tests from JUnit log --- bin/run-tests-in-docker.sh | 4 +- bin/run.sh | 25 +++-- junit-handler/run.php | 7 +- junit-handler/src/Handler.php | 23 +++-- junit-handler/src/TeamcityResult.php | 95 +++++++++++++++++++ tests/success-with-user-output/HelloWorld.php | 10 ++ .../HelloWorldTest.php | 38 ++++++++ .../expected_results.json | 1 + 8 files changed, 183 insertions(+), 20 deletions(-) create mode 100644 junit-handler/src/TeamcityResult.php create mode 100644 tests/success-with-user-output/HelloWorld.php create mode 100644 tests/success-with-user-output/HelloWorldTest.php create mode 100644 tests/success-with-user-output/expected_results.json diff --git a/bin/run-tests-in-docker.sh b/bin/run-tests-in-docker.sh index f88c0a0..f67e05d 100755 --- a/bin/run-tests-in-docker.sh +++ b/bin/run-tests-in-docker.sh @@ -16,7 +16,7 @@ set -e # Build the Docker image -docker build --rm -t exercism/php-test-runner . +# docker build --rm -t exercism/php-test-runner . # Run the Docker image using the settings mimicking the production environment docker run \ @@ -26,6 +26,8 @@ docker run \ --mount type=bind,src="${PWD}/tests",dst=/opt/test-runner/tests \ --mount type=tmpfs,dst=/tmp \ --volume "${PWD}/bin/run-tests.sh:/opt/test-runner/bin/run-tests.sh" \ + --volume "${PWD}/bin/run.sh:/opt/test-runner/bin/run.sh" \ + --volume "${PWD}/junit-handler:/opt/test-runner/junit-handler" \ --workdir /opt/test-runner \ --entrypoint /opt/test-runner/bin/run-tests.sh \ exercism/php-test-runner diff --git a/bin/run.sh b/bin/run.sh index dc3db0e..2fd21de 100755 --- a/bin/run.sh +++ b/bin/run.sh @@ -3,8 +3,9 @@ set -euo pipefail PHPUNIT_BIN="./bin/phpunit-10.phar" -XML_RESULTS='results.xml' -JSON_RESULTS='results.json' +JUNIT_RESULTS='results.xml' +TEAMCITY_RESULTS='teamcity.txt' +EXERCISM_RESULTS='results.json' # shellcheck disable=SC2034 # Modifies XDebug behaviour when invoking PHP XDEBUG_MODE='off' @@ -20,15 +21,22 @@ function main { set +e 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}" + jo version=3 status=error message="${output/"$solution_dir/"/""}" tests="[]" > "${output_dir%/}/${EXERCISM_RESULTS}" return 0; fi - output=$(eval "${PHPUNIT_BIN}" \ + # JUnit results contain the unit test failures only. But they contain `@testdox` - readable test names. + # Teamcity results contain user output in addition to failures as "failed risky tests" + # (command line arguments --disallow-test-output and --fail-on-risky). + # At the moment we require both logs to provide all information to the website. + output=$("${PHPUNIT_BIN}" \ -d memory_limit=300M \ - --log-junit "${output_dir%/}/${XML_RESULTS}" \ + --log-junit "${output_dir%/}/${JUNIT_RESULTS}" \ + --log-teamcity "${output_dir%/}/${TEAMCITY_RESULTS}" \ --no-configuration \ --do-not-cache-result \ + --disallow-test-output \ + --fail-on-risky \ "${test_files%%*( )}" 2>&1) phpunit_exit_code=$? set -e @@ -37,13 +45,14 @@ function main { # 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=3 status=error message="${output/"$solution_dir/"/""}" tests="[]" > "${output_dir%/}/${JSON_RESULTS}" + jo version=3 status=error message="${output/"$solution_dir/"/""}" tests="[]" > "${output_dir%/}/${EXERCISM_RESULTS}" return 0; fi php junit-handler/run.php \ - "${output_dir%/}/${XML_RESULTS}" \ - "${output_dir%/}/${JSON_RESULTS}" + "${output_dir%/}/${EXERCISM_RESULTS}" \ + "${output_dir%/}/${JUNIT_RESULTS}" \ + "${output_dir%/}/${TEAMCITY_RESULTS}" } function installed { diff --git a/junit-handler/run.php b/junit-handler/run.php index 1073fb5..1e57c80 100644 --- a/junit-handler/run.php +++ b/junit-handler/run.php @@ -2,8 +2,9 @@ require __DIR__ . '/vendor/autoload.php'; -$xml_in = $argv[1]; -$json_out = $argv[2]; +$json_out = $argv[1]; +$xml_in = $argv[2]; +$teamcity_in = $argv[3]; $handler = new \Exercism\JunitHandler\Handler(); -$handler->run($xml_in, $json_out); +$handler->run($json_out, $xml_in, $teamcity_in); diff --git a/junit-handler/src/Handler.php b/junit-handler/src/Handler.php index 01666c2..7c989bf 100644 --- a/junit-handler/src/Handler.php +++ b/junit-handler/src/Handler.php @@ -15,11 +15,16 @@ class Handler private const STATUS_PASS = 'pass'; private const STATUS_FAIL = 'fail'; private string $test_file_path = ''; + private ?TeamcityResult $teamcityResult = null; - public function run(string $xml_path, $json_path): void - { + public function run( + string $json_path, + string $xml_path, + string $teamcity_path, + ): void { $testsuites = simplexml_load_file($xml_path); - if ($testsuites === false) { + $this->teamcityResult = new TeamcityResult($teamcity_path); + if ($testsuites === false || !$this->teamcityResult->hasResults()) { $output = [ 'version' => self::VERSION, 'tests' => [], @@ -27,7 +32,7 @@ public function run(string $xml_path, $json_path): void 'message' => <<write_json($json_path, $output); @@ -42,7 +47,6 @@ public function run(string $xml_path, $json_path): void $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']; @@ -164,10 +168,13 @@ private function parseTestCases( $output['name'] = $testdox->getDescription(); } + $testName = $method->getName(); + if ($this->teamcityResult->hasOutputOf($testName)) { + $output['output'] = $this->teamcityResult->outputOf($testName); + } + foreach ($testcase->children() ?? [] as $name => $data) { - if ($name === 'system-out') { - $output['output'] = (string) $data; - } elseif ($name === 'error') { + if ($name === 'error') { $output['status'] = self::STATUS_ERROR; $output['message'] = \str_replace($this->test_file_path, '', (string) $data); } elseif ($name === 'failure') { diff --git a/junit-handler/src/TeamcityResult.php b/junit-handler/src/TeamcityResult.php new file mode 100644 index 0000000..6f5a94a --- /dev/null +++ b/junit-handler/src/TeamcityResult.php @@ -0,0 +1,95 @@ +fillOutputCollection(); + } + + public function hasResults(): bool + { + return $this->outputCollection !== null; + } + + public function hasOutputOf(string $method): bool + { + return isset($this->outputCollection[$method]); + } + + public function outputOf(string $method): ?string + { + return $this->outputCollection[$method] ?? null; + } + + private function fillOutputCollection(): void + { + try { + $linesWithOutput = \array_filter( + \file($this->teamcityFile, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES), + $this->islineWithOutput(...) + ); + $this->outputCollection = \array_combine( + $this->testNamesFrom($linesWithOutput), + $this->outputFrom($linesWithOutput), + ); + } catch (\Throwable $exception) { + // Intentionally empty. + } + } + + private function islineWithOutput(string $line): bool + { + return \str_starts_with($line, self::OUTPUT_LINE_START) + && \str_contains($line, self::OUTPUT_FIELD_START) + ; + } + + private function testNamesFrom(array $lines): array + { + return \array_map($this->testNameFromThisLine(...), $lines); + } + + private function testNameFromThisLine(string $line): string + { + $startOfTestName = \mb_strpos($line, self::NAME_FIELD) + \mb_strlen(self::NAME_FIELD); + $endOfTestName = \mb_strpos($line, "'", $startOfTestName); + + return \mb_substr($line, $startOfTestName, $endOfTestName - $startOfTestName); + } + + private function outputFrom(array $lines): array + { + return \array_map($this->outputFromThisLine(...), $lines); + } + + private function outputFromThisLine(string $line): string + { + $startOfOutput = \mb_strpos($line, self::OUTPUT_FIELD_START) + \mb_strlen(self::OUTPUT_FIELD_START); + $endOfOutput = \mb_strpos($line, self::OUTPUT_FIELD_END, $startOfOutput); + $rawOutput = \mb_substr($line, $startOfOutput, $endOfOutput - $startOfOutput); + + return $this->unescape($rawOutput); + } + + private function unescape(string $text): string + { + return \str_replace( + [ "|'", '|n', '||', ], + [ "'", "\n", '|', ], + $text, + ); + } +} diff --git a/tests/success-with-user-output/HelloWorld.php b/tests/success-with-user-output/HelloWorld.php new file mode 100644 index 0000000..c62854c --- /dev/null +++ b/tests/success-with-user-output/HelloWorld.php @@ -0,0 +1,10 @@ +. + * + * To disable strict typing, comment out the directive below. + */ + +declare(strict_types=1); + +class HelloWorldTest extends PHPUnit\Framework\TestCase +{ + public static function setUpBeforeClass(): void + { + require_once 'HelloWorld.php'; + } + + public function testHelloWorld(): void + { + $this->assertEquals('Hello, World!', helloWorld()); + } +} diff --git a/tests/success-with-user-output/expected_results.json b/tests/success-with-user-output/expected_results.json new file mode 100644 index 0000000..d7ab0f3 --- /dev/null +++ b/tests/success-with-user-output/expected_results.json @@ -0,0 +1 @@ +{"version":3,"status":"pass","tests":[{"name":"testHelloWorld","status":"pass","test_code":"$this->assertEquals('Hello, World!', helloWorld());\n","output":"Some 'user \u00fc\u00e2`|| output\ncontaining \\ various \"problematic\" and UTF-8 chars\nobject(stdClass)#79 (0) {\n}\n"}]} From 51da516ea7ea6a8097bd34c66b5c6ce12b2b3ac8 Mon Sep 17 00:00:00 2001 From: Michael Kramer Date: Sat, 27 Apr 2024 18:06:32 +0200 Subject: [PATCH 2/6] Handle output in case of test failure --- tests/fail-with-user-output/HelloWorld.php | 10 +++++ .../fail-with-user-output/HelloWorldTest.php | 38 +++++++++++++++++++ .../expected_results.json | 1 + 3 files changed, 49 insertions(+) create mode 100644 tests/fail-with-user-output/HelloWorld.php create mode 100644 tests/fail-with-user-output/HelloWorldTest.php create mode 100644 tests/fail-with-user-output/expected_results.json diff --git a/tests/fail-with-user-output/HelloWorld.php b/tests/fail-with-user-output/HelloWorld.php new file mode 100644 index 0000000..35eb1e6 --- /dev/null +++ b/tests/fail-with-user-output/HelloWorld.php @@ -0,0 +1,10 @@ +. + * + * To disable strict typing, comment out the directive below. + */ + +declare(strict_types=1); + +class HelloWorldTest extends PHPUnit\Framework\TestCase +{ + public static function setUpBeforeClass(): void + { + require_once 'HelloWorld.php'; + } + + public function testHelloWorld(): void + { + $this->assertEquals('Hello, World!', helloWorld()); + } +} diff --git a/tests/fail-with-user-output/expected_results.json b/tests/fail-with-user-output/expected_results.json new file mode 100644 index 0000000..7ad0a65 --- /dev/null +++ b/tests/fail-with-user-output/expected_results.json @@ -0,0 +1 @@ +{"version":3,"status":"fail","tests":[{"name":"testHelloWorld","status":"fail","test_code":"$this->assertEquals('Hello, World!', helloWorld());\n","output":"Some 'user \u00fc\u00e2`|| output\ncontaining \\ various \"problematic\" and UTF-8 chars\nobject(stdClass)#79 (0) {\n}\n","message":"HelloWorldTest::testHelloWorld\nFailed asserting that two strings are equal.\n--- Expected\n+++ Actual\n@@ @@\n-'Hello, World!'\n+'Goodbye, Mars!'\n\nHelloWorldTest.php:36"}]} From 3b2a24883b899074c15e40b7a26b9f235e43ee39 Mon Sep 17 00:00:00 2001 From: Michael Kramer Date: Sat, 27 Apr 2024 18:11:05 +0200 Subject: [PATCH 3/6] Handle output in case of test error --- tests/error-with-user-output/HelloWorld.php | 10 +++++ .../error-with-user-output/HelloWorldTest.php | 38 +++++++++++++++++++ .../expected_results.json | 1 + 3 files changed, 49 insertions(+) create mode 100644 tests/error-with-user-output/HelloWorld.php create mode 100644 tests/error-with-user-output/HelloWorldTest.php create mode 100644 tests/error-with-user-output/expected_results.json diff --git a/tests/error-with-user-output/HelloWorld.php b/tests/error-with-user-output/HelloWorld.php new file mode 100644 index 0000000..1af289c --- /dev/null +++ b/tests/error-with-user-output/HelloWorld.php @@ -0,0 +1,10 @@ +. + * + * To disable strict typing, comment out the directive below. + */ + +declare(strict_types=1); + +class HelloWorldTest extends PHPUnit\Framework\TestCase +{ + public static function setUpBeforeClass(): void + { + require_once 'HelloWorld.php'; + } + + public function testHelloWorld(): void + { + $this->assertEquals('Hello, World!', helloWorld()); + } +} diff --git a/tests/error-with-user-output/expected_results.json b/tests/error-with-user-output/expected_results.json new file mode 100644 index 0000000..5b817dc --- /dev/null +++ b/tests/error-with-user-output/expected_results.json @@ -0,0 +1 @@ +{"version":3,"status":"fail","tests":[{"name":"testHelloWorld","status":"error","test_code":"$this->assertEquals('Hello, World!', helloWorld());\n","output":"Some 'user \u00fc\u00e2`|| output\ncontaining \\ various \"problematic\" and UTF-8 chars\nobject(stdClass)#79 (0) {\n}\n","message":"HelloWorldTest::testHelloWorld\nBadFunctionCallException: Implement the helloWorld() function\n\nHelloWorld.php:9\nHelloWorldTest.php:36"}]} From c6f3dee6f8fc5eddec62e9f77b9c93a1d0961b1e Mon Sep 17 00:00:00 2001 From: Michael Kramer Date: Sat, 27 Apr 2024 18:11:46 +0200 Subject: [PATCH 4/6] Undo testing changes to run-tests-in-docker.sh --- bin/run-tests-in-docker.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bin/run-tests-in-docker.sh b/bin/run-tests-in-docker.sh index f67e05d..f88c0a0 100755 --- a/bin/run-tests-in-docker.sh +++ b/bin/run-tests-in-docker.sh @@ -16,7 +16,7 @@ set -e # Build the Docker image -# docker build --rm -t exercism/php-test-runner . +docker build --rm -t exercism/php-test-runner . # Run the Docker image using the settings mimicking the production environment docker run \ @@ -26,8 +26,6 @@ docker run \ --mount type=bind,src="${PWD}/tests",dst=/opt/test-runner/tests \ --mount type=tmpfs,dst=/tmp \ --volume "${PWD}/bin/run-tests.sh:/opt/test-runner/bin/run-tests.sh" \ - --volume "${PWD}/bin/run.sh:/opt/test-runner/bin/run.sh" \ - --volume "${PWD}/junit-handler:/opt/test-runner/junit-handler" \ --workdir /opt/test-runner \ --entrypoint /opt/test-runner/bin/run-tests.sh \ exercism/php-test-runner From 1d4e06831586b58d3d2b98defc3fb86cc65300c3 Mon Sep 17 00:00:00 2001 From: Michael Kramer Date: Mon, 29 Apr 2024 14:49:52 +0200 Subject: [PATCH 5/6] Add handling `\r` and test for \t and \n also --- junit-handler/src/TeamcityResult.php | 4 ++-- tests/error-with-user-output/HelloWorld.php | 2 +- tests/error-with-user-output/expected_results.json | 2 +- tests/fail-with-user-output/HelloWorld.php | 2 +- tests/fail-with-user-output/expected_results.json | 2 +- tests/success-with-user-output/HelloWorld.php | 2 +- tests/success-with-user-output/expected_results.json | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/junit-handler/src/TeamcityResult.php b/junit-handler/src/TeamcityResult.php index 6f5a94a..6334205 100644 --- a/junit-handler/src/TeamcityResult.php +++ b/junit-handler/src/TeamcityResult.php @@ -87,8 +87,8 @@ private function outputFromThisLine(string $line): string private function unescape(string $text): string { return \str_replace( - [ "|'", '|n', '||', ], - [ "'", "\n", '|', ], + [ "|'", '|n', '|r', '||', ], + [ "'", "\n", "\r", '|', ], $text, ); } diff --git a/tests/error-with-user-output/HelloWorld.php b/tests/error-with-user-output/HelloWorld.php index 1af289c..b5b87bb 100644 --- a/tests/error-with-user-output/HelloWorld.php +++ b/tests/error-with-user-output/HelloWorld.php @@ -2,7 +2,7 @@ function helloWorld() { - echo "Some 'user üâ`|| output" . PHP_EOL + echo "Some 'user üâ`|| \r\toutput\n" . 'containing \\ various "problematic" and UTF-8 chars' . PHP_EOL; var_dump(new stdClass()); diff --git a/tests/error-with-user-output/expected_results.json b/tests/error-with-user-output/expected_results.json index 5b817dc..fb3b546 100644 --- a/tests/error-with-user-output/expected_results.json +++ b/tests/error-with-user-output/expected_results.json @@ -1 +1 @@ -{"version":3,"status":"fail","tests":[{"name":"testHelloWorld","status":"error","test_code":"$this->assertEquals('Hello, World!', helloWorld());\n","output":"Some 'user \u00fc\u00e2`|| output\ncontaining \\ various \"problematic\" and UTF-8 chars\nobject(stdClass)#79 (0) {\n}\n","message":"HelloWorldTest::testHelloWorld\nBadFunctionCallException: Implement the helloWorld() function\n\nHelloWorld.php:9\nHelloWorldTest.php:36"}]} +{"version":3,"status":"fail","tests":[{"name":"testHelloWorld","status":"error","test_code":"$this->assertEquals('Hello, World!', helloWorld());\n","output":"Some 'user \u00fc\u00e2`|| \r\toutput\ncontaining \\ various \"problematic\" and UTF-8 chars\nobject(stdClass)#79 (0) {\n}\n","message":"HelloWorldTest::testHelloWorld\nBadFunctionCallException: Implement the helloWorld() function\n\nHelloWorld.php:9\nHelloWorldTest.php:36"}]} diff --git a/tests/fail-with-user-output/HelloWorld.php b/tests/fail-with-user-output/HelloWorld.php index 35eb1e6..718634c 100644 --- a/tests/fail-with-user-output/HelloWorld.php +++ b/tests/fail-with-user-output/HelloWorld.php @@ -2,7 +2,7 @@ function helloWorld() { - echo "Some 'user üâ`|| output" . PHP_EOL + echo "Some 'user üâ`|| \r\toutput\n" . 'containing \\ various "problematic" and UTF-8 chars' . PHP_EOL; var_dump(new stdClass()); diff --git a/tests/fail-with-user-output/expected_results.json b/tests/fail-with-user-output/expected_results.json index 7ad0a65..834c269 100644 --- a/tests/fail-with-user-output/expected_results.json +++ b/tests/fail-with-user-output/expected_results.json @@ -1 +1 @@ -{"version":3,"status":"fail","tests":[{"name":"testHelloWorld","status":"fail","test_code":"$this->assertEquals('Hello, World!', helloWorld());\n","output":"Some 'user \u00fc\u00e2`|| output\ncontaining \\ various \"problematic\" and UTF-8 chars\nobject(stdClass)#79 (0) {\n}\n","message":"HelloWorldTest::testHelloWorld\nFailed asserting that two strings are equal.\n--- Expected\n+++ Actual\n@@ @@\n-'Hello, World!'\n+'Goodbye, Mars!'\n\nHelloWorldTest.php:36"}]} +{"version":3,"status":"fail","tests":[{"name":"testHelloWorld","status":"fail","test_code":"$this->assertEquals('Hello, World!', helloWorld());\n","output":"Some 'user \u00fc\u00e2`|| \r\toutput\ncontaining \\ various \"problematic\" and UTF-8 chars\nobject(stdClass)#79 (0) {\n}\n","message":"HelloWorldTest::testHelloWorld\nFailed asserting that two strings are equal.\n--- Expected\n+++ Actual\n@@ @@\n-'Hello, World!'\n+'Goodbye, Mars!'\n\nHelloWorldTest.php:36"}]} diff --git a/tests/success-with-user-output/HelloWorld.php b/tests/success-with-user-output/HelloWorld.php index c62854c..b474e0d 100644 --- a/tests/success-with-user-output/HelloWorld.php +++ b/tests/success-with-user-output/HelloWorld.php @@ -2,7 +2,7 @@ function helloWorld() { - echo "Some 'user üâ`|| output" . PHP_EOL + echo "Some 'user üâ`|| \r\toutput\n" . 'containing \\ various "problematic" and UTF-8 chars' . PHP_EOL; var_dump(new stdClass()); diff --git a/tests/success-with-user-output/expected_results.json b/tests/success-with-user-output/expected_results.json index d7ab0f3..9dd8da7 100644 --- a/tests/success-with-user-output/expected_results.json +++ b/tests/success-with-user-output/expected_results.json @@ -1 +1 @@ -{"version":3,"status":"pass","tests":[{"name":"testHelloWorld","status":"pass","test_code":"$this->assertEquals('Hello, World!', helloWorld());\n","output":"Some 'user \u00fc\u00e2`|| output\ncontaining \\ various \"problematic\" and UTF-8 chars\nobject(stdClass)#79 (0) {\n}\n"}]} +{"version":3,"status":"pass","tests":[{"name":"testHelloWorld","status":"pass","test_code":"$this->assertEquals('Hello, World!', helloWorld());\n","output":"Some 'user \u00fc\u00e2`|| \r\toutput\ncontaining \\ various \"problematic\" and UTF-8 chars\nobject(stdClass)#79 (0) {\n}\n"}]} From 1c30a2edbab9b22f915020c9381dd3be0a4f8be9 Mon Sep 17 00:00:00 2001 From: Michael Kramer Date: Sat, 4 May 2024 10:14:16 +0200 Subject: [PATCH 6/6] Sync TeamCity escaping to PHPUnit --- junit-handler/src/TeamcityResult.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/junit-handler/src/TeamcityResult.php b/junit-handler/src/TeamcityResult.php index 6334205..bbb31f2 100644 --- a/junit-handler/src/TeamcityResult.php +++ b/junit-handler/src/TeamcityResult.php @@ -87,8 +87,10 @@ private function outputFromThisLine(string $line): string private function unescape(string $text): string { return \str_replace( - [ "|'", '|n', '|r', '||', ], - [ "'", "\n", "\r", '|', ], + // Keep this in sync with PHPUnit Teamcity escape() + // https://github.com/sebastianbergmann/phpunit/blob/main/src/Logging/TeamCity/TeamCityLogger.php#L331 + ['||', "|'", '|n', '|r', '|]', '|['], + ['|', "'", "\n", "\r", ']', '['], $text, ); }