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

Support timeout trigger skip instead of fail #153

Closed

Conversation

yasirroni
Copy link

This is an initial PR for support timeout that trigger skip instead of fail. If this concept is accepted, I'm willing to add pytest and doc. Thank you.

Support timeout trigger skip instead of fail
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

Why skip?

@yasirroni
Copy link
Author

yasirroni commented Oct 13, 2023

The rationale is because I use pytest with --nbdime that run notebooks and pytest at the same time. Fails due to timeout in this directory containing various notebooks example is deemed as not crucial. Instead of not testing those notebooks at all, I would be rather knowing if those notebooks did fails due to timeout and not other error. Ideally, I should try and catch timeout, but changing how nbdime and pytest works seems too complicated.

Currently, what is possible in nbdime is to allow notebook to fail (I already use it), but sadly timeout error is not fail in the cell but on the side of pytest (can't be catch by nbdime as far I know).

Thus, I need pytest that allow timeout error.

@flub
Copy link
Member

flub commented Oct 23, 2023

Thanks for giving more rationale. I'm currently leaning against this I'm afraid. The current philosophy of pytest-timeout is that timeouts are fatal and need to be debugged. You seem to use it as some kind of "i don't care enough about the results to wait longer" if I understand correctly (I might not have understood correctly!), which I'm not really keen to add more features to pytest-timeout for.

@yasirroni
Copy link
Author

You seem to use it as some kind of "i don't care enough about the results to wait longer" if I understand correctly (I might not have understood correctly!), which I'm not really keen to add more features to pytest-timeout for.

It's more like that "let skip that for now". Skip is different from success, it works like a warning.
_
Without skip support, I'm in a position of "skip the whole example notebook" or "workflow will always fail due to single example require 24 hours of simulation running".

Or, I need to change the notebook itself, like comments all cells below the long run cell and tell user to uncomment those cells to run the example.

@RonnyPfannschmidt
Copy link
Member

From my pov having it xfail would be a stretch, but Skip ain't acceptable

Even then, xfail should not available as global configuration by default, but rather as context manager

@flub flub deleted the branch pytest-dev:master February 8, 2024 19:08
@flub flub closed this Feb 8, 2024
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

3 participants