-
-
Notifications
You must be signed in to change notification settings - Fork 138
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
Test timeout issues #683
Test timeout issues #683
Conversation
This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested. If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.
For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping |
@homersimpsons Sorry, I thought I had figured that out right. But I somehow haven't remembered things correctly... |
@homersimpsons Another attempt using measured data now. I put the data into the forum, and a longish comment into the test script. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your investigation. I'm proposing a new wording for the comment about the timeout. But I still approve the pull request as-is.
Co-authored-by: homersimpsons <[email protected]>
Co-authored-by: homersimpsons <[email protected]>
PHPUnit 10.5
assertArrayNotHasKey()
PHPUnit
^10.3
has an issue withassertArrayNotHasKey()
. Only this (and the oppositeassertArrayHasKey()
) are affected. The issue is not listed in PHPUnit's issue list / mentioned in Pull Requests, but the issue is gone in11.0.0
.This contains a workaround for this and enables upgrading to PHPUnit 10.5. I replaced that assertion with a functionally equivalent one, but that prints some misleading output in addition to the helpful message. So it should be reverted when upgrading to PHPUnit 11.
CI limit for test runtime
I also added a time limit of 18 (lowest value possible on my machine with equal performance to Online Test Runner) x 3 (to compensate lower CI performance) = 54 seconds for each exercise test class using
timeout
command. This should prevent running into performance degradation issues in the future.As discussed in the forum there is no recommendation or guiding data for such a limit. Only guesses about "test runner is expected to be slower than GitHub CI". But the production limit of 20 seconds exists and is enforced. So I took a guess based on my measurements.
Part of #652