From f704e628ee2c1777ddfadc272f3d188530555c4e Mon Sep 17 00:00:00 2001 From: Michael Kramer Date: Sun, 14 Apr 2024 14:08:48 +0200 Subject: [PATCH 1/6] Fix timeout PHPUnit 10 with assertArrayNotHasKey() --- .../practice/robot-name/RobotNameTest.php | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/exercises/practice/robot-name/RobotNameTest.php b/exercises/practice/robot-name/RobotNameTest.php index ae360996..c30071b9 100644 --- a/exercises/practice/robot-name/RobotNameTest.php +++ b/exercises/practice/robot-name/RobotNameTest.php @@ -73,18 +73,34 @@ public function testResetName(): void $this->assertMatchesRegularExpression('/\w{2}\d{3}/', $name2); } + /** + * PHPUnit ^10.5 has an issue with + * $this->assertArrayNotHasKey($name, $names, sprintf... + * that leads to test timeouts. This is fixed in PHPUnit ^11. + * Revert workaround + * $this->assertFalse(isset($names[$name]), sprintf... + * when upgrading. + */ public function testNameArentRecycled(): void { $names = []; for ($i = 0; $i < 10000; $i++) { $name = $this->robot->getName(); - $this->assertArrayNotHasKey($name, $names, sprintf('Name %s reissued after Reset.', $name)); + $this->assertFalse(isset($names[$name]), sprintf('Name %s reissued after Reset.', $name)); $names[$name] = true; $this->robot->reset(); } } + /** + * PHPUnit ^10.5 has an issue with + * $this->assertArrayNotHasKey($name, $names, sprintf... + * that leads to test timeouts. This is fixed in PHPUnit ^11. + * Revert workaround + * $this->assertFalse(isset($names[$name]), sprintf... + * when upgrading. + */ // This test is optional. public function testNameUniquenessManyRobots(): void { @@ -92,7 +108,7 @@ public function testNameUniquenessManyRobots(): void for ($i = 0; $i < 10000; $i++) { $name = (new Robot())->getName(); - $this->assertArrayNotHasKey($name, $names, sprintf('Name %s reissued after %d robots', $name, $i)); + $this->assertFalse(isset($names[$name]), sprintf('Name %s reissued after %d robots', $name, $i)); $names[$name] = true; } } From 5dc73bdd5cda025cd9de30fb4ee4ca24be5ac016 Mon Sep 17 00:00:00 2001 From: Michael Kramer Date: Sun, 14 Apr 2024 14:17:15 +0200 Subject: [PATCH 2/6] Catch performance issues with max time per test --- bin/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/test.sh b/bin/test.sh index 16380de4..568ea951 100755 --- a/bin/test.sh +++ b/bin/test.sh @@ -58,7 +58,7 @@ function test { cp "${exercise_dir}/.meta/${example_file}" "${outdir}/${exercise_file}.${file_ext}" fi - eval "${PHPUNIT_BIN}" --no-configuration "${outdir}/${test_file}" + eval "${PHPUNIT_BIN}" --default-time-limit 7 --enforce-time-limit --fail-on-risky --no-configuration "${outdir}/${test_file}" } function installed { From 7cca089c085d24f4b7fb8f33d20a7e8c6dfdaca4 Mon Sep 17 00:00:00 2001 From: Michael Kramer Date: Sun, 14 Apr 2024 15:00:12 +0200 Subject: [PATCH 3/6] Raise max timeout to 20 seconds --- bin/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/test.sh b/bin/test.sh index 568ea951..565282e5 100755 --- a/bin/test.sh +++ b/bin/test.sh @@ -58,7 +58,7 @@ function test { cp "${exercise_dir}/.meta/${example_file}" "${outdir}/${exercise_file}.${file_ext}" fi - eval "${PHPUNIT_BIN}" --default-time-limit 7 --enforce-time-limit --fail-on-risky --no-configuration "${outdir}/${test_file}" + eval "${PHPUNIT_BIN}" --default-time-limit 20 --enforce-time-limit --fail-on-risky --no-configuration "${outdir}/${test_file}" } function installed { From f2b740196c3a0e7fdc2d9dc7bd90576564234625 Mon Sep 17 00:00:00 2001 From: Michael Kramer Date: Mon, 15 Apr 2024 08:55:01 +0200 Subject: [PATCH 4/6] Recalculate and document per-test CI limit --- bin/test.sh | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/bin/test.sh b/bin/test.sh index 565282e5..d371483a 100755 --- a/bin/test.sh +++ b/bin/test.sh @@ -58,7 +58,23 @@ function test { cp "${exercise_dir}/.meta/${example_file}" "${outdir}/${exercise_file}.${file_ext}" fi - eval "${PHPUNIT_BIN}" --default-time-limit 20 --enforce-time-limit --fail-on-risky --no-configuration "${outdir}/${test_file}" + # The time-limit `18` is based on an approximation of the performance. + # Online Test Runner is appr. 3x faster than GitHub CI on Ubuntu. + # + # The total runtime limit of the Online Test Runner is 20 seconds including + # Docker container launch and processing the results. That leaves appr. 18 + # seconds for all tests of an exercise. But that's not the `18` here. + # + # The longest running exercise tests in CI are in `alphametics`. It has 3 slow + # test cases (9 letters and 10 letters) that require appr. 15 secs each in CI + # and appr. 5 seconds each in the Online Test Runner. + # + # Putting that together gives a max. of 6 seconds for each of the slow test + # cases in the Online Test Runner before hitting the timeout in production + # -> use 3x 6secs = 18 seconds as a CI limit to ensure tests can actually + # be run in production. + # See: https://forum.exercism.org/t/test-tracks-for-the-20-seconds-limit-on-test-runners/10536/8 + eval "${PHPUNIT_BIN}" --default-time-limit 18 --enforce-time-limit --fail-on-risky --no-configuration "${outdir}/${test_file}" } function installed { From 4ac0a5b4ca4b2eb2c587644891e15afd3dd5a39f Mon Sep 17 00:00:00 2001 From: Michael Kramer Date: Sun, 21 Apr 2024 09:00:50 +0200 Subject: [PATCH 5/6] Use per exercise process timeout --- bin/test.sh | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/bin/test.sh b/bin/test.sh index d371483a..a79c4479 100755 --- a/bin/test.sh +++ b/bin/test.sh @@ -58,23 +58,17 @@ function test { cp "${exercise_dir}/.meta/${example_file}" "${outdir}/${exercise_file}.${file_ext}" fi - # The time-limit `18` is based on an approximation of the performance. + # The time-limit `54` is based on an approximation of the performance. # Online Test Runner is appr. 3x faster than GitHub CI on Ubuntu. # # The total runtime limit of the Online Test Runner is 20 seconds including # Docker container launch and processing the results. That leaves appr. 18 - # seconds for all tests of an exercise. But that's not the `18` here. + # seconds for all tests of an exercise. # - # The longest running exercise tests in CI are in `alphametics`. It has 3 slow - # test cases (9 letters and 10 letters) that require appr. 15 secs each in CI - # and appr. 5 seconds each in the Online Test Runner. - # - # Putting that together gives a max. of 6 seconds for each of the slow test - # cases in the Online Test Runner before hitting the timeout in production - # -> use 3x 6secs = 18 seconds as a CI limit to ensure tests can actually - # be run in production. + # Putting that together gives a max. of 18 seconds x3 = 54 seconds for any + # test class in CI. # See: https://forum.exercism.org/t/test-tracks-for-the-20-seconds-limit-on-test-runners/10536/8 - eval "${PHPUNIT_BIN}" --default-time-limit 18 --enforce-time-limit --fail-on-risky --no-configuration "${outdir}/${test_file}" + timeout 54s "${PHPUNIT_BIN}" --no-configuration "${outdir}/${test_file}" } function installed { From 87b3fd99d784418cc4241ac949f8282d08d5550a Mon Sep 17 00:00:00 2001 From: mk-mxp <55182845+mk-mxp@users.noreply.github.com> Date: Sun, 21 Apr 2024 15:08:18 +0200 Subject: [PATCH 6/6] Improved comment wording Co-authored-by: homersimpsons --- bin/test.sh | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/bin/test.sh b/bin/test.sh index a79c4479..b54292c7 100755 --- a/bin/test.sh +++ b/bin/test.sh @@ -58,16 +58,17 @@ function test { cp "${exercise_dir}/.meta/${example_file}" "${outdir}/${exercise_file}.${file_ext}" fi - # The time-limit `54` is based on an approximation of the performance. - # Online Test Runner is appr. 3x faster than GitHub CI on Ubuntu. + # `54s` timeout is an approximation to ensure the tests will not timeont in Exercism Test Runner. # - # The total runtime limit of the Online Test Runner is 20 seconds including - # Docker container launch and processing the results. That leaves appr. 18 - # seconds for all tests of an exercise. + # 1. Exercism Test Runner is around 3 times faster than GitHub CI on Ubuntu. + # See: https://forum.exercism.org/t/test-tracks-for-the-20-seconds-limit-on-test-runners/10536/8 + # 2. Exercism Test Runner should complete in 20s and involves: + # - Starting Docker container (~1s) + # - Running the test suite + # - Processing the results (~1s) # - # Putting that together gives a max. of 18 seconds x3 = 54 seconds for any - # test class in CI. - # See: https://forum.exercism.org/t/test-tracks-for-the-20-seconds-limit-on-test-runners/10536/8 + # This gives 18s maximum for the test suite to run in the Exercism Test Runner. + # Hence the test suite should complete in less than 18s x 3 = 54s in GitHub CI on Ubuntu. timeout 54s "${PHPUNIT_BIN}" --no-configuration "${outdir}/${test_file}" }