-
Notifications
You must be signed in to change notification settings - Fork 897
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: make test timeouts configurable #1238
Conversation
Signed-off-by: Michael Dawson <mdawson@devrus.com>
Shouldn't this be documented somewhere? |
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.
I'd recommend we increase the default to 10s.
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.
Looks good to me with suggestions applied. Will probably solve some flakiness in our own CI.
I'm not sure we should document this.. but if we want to |
How would people know it's there without randomly stumbling upon it in code then? |
A timeout error would point to that block of code. |
Co-authored-by: Matteo Collina <matteo.collina@gmail.com>
Co-authored-by: Matteo Collina <matteo.collina@gmail.com>
@mcollina thanks, accepted your suggestions. From the output I'll confirm it was pretty easy to see that the failures were a timeout issue and where the timeouts where. |
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.
lgtm
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
We saw some timeouts in the tests when we were running to validate in Red Hat containers (for example: https://catalog.redhat.com/software/containers/ubi8/nodejs-12/5d3fff015a13461f5fb8635a).
Our investigation so far points to it being loading/resource issue on the machines we are running versus anything in the tests.
We'd like to be able to extended timeouts in the tests, this PR adds a simple way to do that through the environment.