-
Notifications
You must be signed in to change notification settings - Fork 64
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
Switch to Pytest's terminal reporter #162
Conversation
Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>
d0afc0e
to
ca86ac4
Compare
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.
Thanks for the updates! Looks good mostly, I'd like to request two things though:
- Keep the timeouts in the tests to 1s
- Add changelog entries for all the changes.
pytest_timeout.py
Outdated
yield | ||
finally: | ||
if is_timeout and settings.func_only is False: | ||
hooks.pytest_timeout_cancel_timer(item=item) |
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.
this seems like quite an important bugfix. could you update the changelog as well?
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.
With old style hook wrappers it's incorrect to add try/finally
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.
Oh?!
Does the code after the yield
always run then?
Got a link to something I could read? I have several plugins which might need to be updated...
What's the consequence?
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.
For legacy hook wrappers one needs to check the exception attribute if one needs to handle them
Moder hookwrappers Work like expected
def test_not_main_thread(testdir): | ||
testdir.makepyfile( | ||
def test_not_main_thread(pytester): | ||
pytest.skip("The 'pytest_timeout.timeout_setup' function no longer exists") |
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'm confused by this, why is there still a test for it then?
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.
This was broken in dd9d6080763c559de1ff5b15deb9c27559dd3431 and the test was broad enough that it kept passing.
Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>
Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>
Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>
Done. |
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.
Thanks! I guess maybe it's time to figure out how to do another release sometime soon.
I'd love that! |
Additionally update pre-commit hook versions