-
Notifications
You must be signed in to change notification settings - Fork 18
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
Tests timeout is too short, especially on slow architectures #18
Comments
60s is a standard that I have copied from other places. I have no problem going up if needed for non amd64 architecture. What about using 60s for i386/amd64 and going up for anything else. As an alternative, we can have a OUNIT_MAX_TIMEOUT_MINUTES=10 so that you can set it yourself depending on the architecture. |
I think timeouts are a necessary evil for some tests, but I try to avoid them if possible. Some questions:
If we're in the second case then I would chose some long timeout on all architectures, like 10 or 20 minutes. If we're in the third case then we shouldn't have a timeout at all. If it's the first case then of course we need to keep the timeout and investigate why the test fails on s390x. |
This was an arbitrary choice, nothing will fail if we change the delay. We are in the second case. The real thing that I wanted to avoid is to have tests that never ends and an overall testing step that takes hours in a CI. |
CHANGES: ### Changed - Minimal OCaml version is now 4.04. ### Fixed - Make colored output and JUnit features more prominent in the documentation. (Closes: gildor478/ounit#13, gildor478/ounit#12) - Increase default timeouts, so that they work as well for slow architecture like s390x. The fastest timeout is now 20s (immediate test) and the longest is 1h (huge test). (Closes: gildor478/ounit#18)
On s390x in particular:
TBH 60s is simply too short on these architectures. Best to make it 10 minutes, unless the 60s rule is there for a good reason (like the test must never take longer than 60s).
The text was updated successfully, but these errors were encountered: