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

Switch to Pytest's terminal reporter #162

Merged
merged 4 commits into from
Jan 23, 2024
Merged

Conversation

s0undt3ch
Copy link
Contributor

Additionally update pre-commit hook versions

Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>
Copy link
Member

@flub flub left a 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.

yield
finally:
if is_timeout and settings.func_only is False:
hooks.pytest_timeout_cancel_timer(item=item)
Copy link
Member

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?

Copy link
Member

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

Copy link
Contributor Author

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?

Copy link
Member

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

test_pytest_timeout.py Outdated Show resolved Hide resolved
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")
Copy link
Member

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?

Copy link
Contributor Author

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>
@s0undt3ch
Copy link
Contributor Author

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.

Done.

Copy link
Member

@flub flub left a 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.

@flub flub merged commit 9a024f5 into pytest-dev:master Jan 23, 2024
12 checks passed
@s0undt3ch
Copy link
Contributor Author

Thanks! I guess maybe it's time to figure out how to do another release sometime soon.

I'd love that!
I have a PR waiting for it 😁

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