Skip to content

Improve test usleep_basic by inlining its output #4703

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

Closed
wants to merge 1 commit into from
Closed

Improve test usleep_basic by inlining its output #4703

wants to merge 1 commit into from

Conversation

villfa
Copy link
Contributor

@villfa villfa commented Sep 14, 2019

Here the current diff when usleep_basic.phpt fails:

========DIFF========
003+ TEST FAILED
003- TEST PASSED
========DONE========
FAIL Test usleep() function [ext/standard/tests/general_functions/usleep_basic.phpt]

This PR makes it more informative by adding the actual time spent sleeping:

========DIFF========
002+ TEST FAILED: Thread slept for 970171.21315002 micro-seconds
002- TEST PASSED: Thread slept for %f micro-seconds
========DONE========
FAIL Test usleep() function [ext/standard/tests/general_functions/usleep_basic.phpt]

@nikic
Copy link
Member

nikic commented Sep 18, 2019

From https://dev.azure.com/phpazuredevops/PHP/_build/results?buildId=2692&view=ms.vss-test-web.build-test-results-tab&runId=55304&resultId=112010&paneView=debug:

002+ TEST FAILED: Thread slept for 933841.94374084 micro-seconds
002- TEST PASSED: Thread slept for %f micro-seconds

That sleep is shorter by >60ms, that's a pretty big diff. We've had a lot of issues with sleep tests on macos. I don't think the problem is on our side, my best guess is that there are some clock synchronization issues when migrating between CPU cores.

@nikic
Copy link
Member

nikic commented Sep 18, 2019

The other theory would be that usleep() gets interrupted. I added some debugging code to check for this: 96840a2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants