Skip to content

Fix timeout tests #4969

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

Fix timeout tests #4969

wants to merge 1 commit into from

Conversation

nikic
Copy link
Member

@nikic nikic commented Dec 5, 2019

After taking a more detailed look at our commonly failing timeout tests... turns out that most of them are useless as written and don't test what they're supposed to.

This PR has a couple of changes:

  • Tests for timeout in while/for/foreach should just have the loop as an infinite loop. Calling into something like busy_wait means that we just end up always testing whatever busy_wait does.
  • Tests for timeouts in calls need to be based on something like sleep, otherwise we'd have to introduce a loop, and we'd end up testing timeout of the looping structure instead. Using sleep only works on Windows, because that's the only system where sleep counts towards the timeout. As such, many of those tests are now Windows only.
  • Removed some tests where I don't see a good way to test what they're supposed to test. E.g. how can we test a timeout in eval() specifically?

@nikic
Copy link
Member Author

nikic commented Dec 5, 2019

The failure in the shutdown function tests is legitimate-ish. The timeout happens during the actual call of the shutdown function, so we'd need a timeout check as part of call_user_function. I'll probably mark these tests as XFAIL for now and do a separate PR for that.

@php-pulls php-pulls closed this in e760d94 Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant