Skip to content
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

python: remove arbitrary timeouts from tests #3059

Merged
merged 1 commit into from
Feb 6, 2022

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Feb 5, 2022

pytest-timeout already handles all deadlocks and is configurable with
--timeout option. With this change it is possible to disable timeout
with --timeout 0 to run tests on extremely slow connections.

#skip-changelog

@link2xt link2xt force-pushed the remove-arbitrary-timeouts branch 3 times, most recently from 51f5e9c to f2ecdf1 Compare February 5, 2022 20:17
@link2xt link2xt requested review from Hocuri and r10s February 5, 2022 20:43
@link2xt
Copy link
Collaborator Author

link2xt commented Feb 5, 2022

Made all the tests succeed on a network with 5 second ping.

Copy link
Collaborator

@Hocuri Hocuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

res = self.conn.idle_check(timeout=30)
if len(res) == 0:
raise TimeoutError
res = self.conn.idle_check()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this still raise a TimeoutError if len(res) == 0? Or will conn.idle_check() raise an error by itself in this case now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without timeout it should not return without a response. If it still does, we return empty res and outside code will restart idle_check if it wants a response.

@link2xt link2xt merged commit c4b0f77 into master Feb 6, 2022
@link2xt link2xt deleted the remove-arbitrary-timeouts branch February 6, 2022 12:49
pytest-timeout already handles all deadlocks and is configurable with
--timeout option. With this change it is possible to disable timeout
with --timeout 0 to run tests on extremely slow connections.
@Hocuri Hocuri mentioned this pull request Feb 7, 2022
@Hocuri Hocuri restored the remove-arbitrary-timeouts branch February 7, 2022 22:02
@Hocuri Hocuri deleted the remove-arbitrary-timeouts branch February 7, 2022 22:02
@Hocuri Hocuri restored the remove-arbitrary-timeouts branch February 7, 2022 22:02
@Hocuri Hocuri deleted the remove-arbitrary-timeouts branch February 7, 2022 22:03
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.

None yet

2 participants