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

Fix test failures with pytest-8.0.0 due to pytest.warns() starting to work inside pytest.raises() #8678

Merged
merged 2 commits into from
Feb 6, 2024

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Jan 30, 2024

Change Summary

Fix the test suite problems that cause test failures with pytest-8.0.0, due to pytest.warns() being fixed to work inside pytest.raises().

I've specifically:

  1. Removed pytest.warns() when no warning could be produced because of invalid function call raising TypeError.
  2. Moved the Pydantic V1 deprecation warning earlier in the function to ensure that it is emitted even if the function raises an exception. If you prefer, I can change that to remove the pytest.warns() assertion instead.

This doesn't fix all the test failures reported in the linked bug, though. test_validator_bad_fields_throws_configerror fails due to changed exception representation, and I don't know how to clearly handle both versions.

Additionally, test_root_validator_allow_reuse_same_field fails with the default -Werror (I didn't include that in the bug), since pytest.warns() passes the warning through. However, adding a second pytest.warns() to check for that breaks the test with older pytest versions, where the warning is captured by the other pytest.warns().

Related issue number

Fix #8674

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @hramezani

Fix `tests/test_validators.py::test_use_no_fields` not to check
for a warning that can't be emitted because calling the decorator
fails with a `TypeError`.  Starting with pytest 8.0.0 (due to
pytest-dev/pytest#9036 fix), `pytest.warns()` assertions are enforced
even if an exception is raised and consumed by `pytest.raises()`.

Bug pydantic#8674
Emit Pydantic V1 style `@validator` deprecation warnings before raising
`PydanticUserError` about incorrect use.  This is one of the possible
solutions to test failures introduced with pytest 8.0.0, as now
`pytest.warns()` assertions are enforced even if an exception raised
and caught by `pytest.raises()` (this is pytest-dev/pytest#9036).

The alternate possibility would be to remove `pytest.warns()` from
respective test functions but I think it's reasonable to emit these
warnings, to warn the users that they ought to consider upgrading
the code to Pydantic V2 API rather than fixing the immediate issue.

Bug pydantic#8674
Copy link

codspeed-hq bot commented Jan 30, 2024

CodSpeed Performance Report

Merging #8678 will not alter performance

Comparing mgorny:pytest-8 (e43edaa) with main (c1dff15)

Summary

✅ 10 untouched benchmarks

@mgorny
Copy link
Contributor Author

mgorny commented Jan 30, 2024

please review

@sydney-runkle
Copy link
Member

Marking this as fixing the issue that you've filed, and we can open a new one when the time comes!

Copy link
Member

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

@mgorny,

Thanks for investigating these test failures + helping us to resolve some of the issues. Your changes look good. I'm ok with removing that warns check, given that we check for the same warning in other tests.

@sydney-runkle sydney-runkle merged commit 825a692 into pydantic:main Feb 6, 2024
53 of 55 checks passed
@mgorny mgorny deleted the pytest-8 branch February 7, 2024 06:35
@mgorny
Copy link
Contributor Author

mgorny commented Feb 7, 2024

Thanks!

Marking this as fixing the issue that you've filed, and we can open a new one when the time comes!

Should I open another issue for the remaining test failures?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test regressions with pytest 8.0.0
3 participants