[WIP] Fixes for pytest-8 failures related to warnings #9040
Conversation
Add irrelevant warnings triggered by different parts of tests to ignored warning list when using `pytest.warns()`, in order to fix compatibility with pytest-8. In previous versions of pytest, the call to `pytest.warns()` would consume all warnings, including these not matched by the clause. Starting with pytest-8, remaining warnings are reemitted and therefore trigger errors. To preserve compatibility with both pytest versions, just ignore them rather than asserting for both.
Replace the deprecated `PYDANTIC_ERRORS_OMIT_URL` with `PYDANTIC_ERRORS_INCLUDE_URL` in `tests/conftest.py`, to fix deprecation warning from pydantic-core. This warning was incidentally ignored by pytest-7 but it is now triggered by pytest-8.
Modify the `parse_raw()` and `parse_file()` to ignore deprecation warnings from their implementation details. The methods themselves are deprecated, and that should be the only deprecation warning interesting to the user. This also fixes test failures with pytest-8 that no longer ignores warnings that did not match `pytest.warns()`.
CodSpeed Performance ReportMerging #9040 will not alter performanceComparing Summary
|
Add a warning filter to test_json_schema to resolve the test failure with pytest-8. The test currently uses parametrization to assert for a single call emitting two separate warnings, since `pytest.warns()` used to consume all non-matched warnings before. However, with pytest-8 it reemits the remaining warnings and therefore causes test to fail due to the `error` filter. When pydantic requires pytest >= 8, we can instead switch to using two `pytest.warns()`.
|
Once this is implemented, can we bump the pytest version to v8 in |
| allow_pickle=allow_pickle, | ||
| ) | ||
| return cls.parse_obj(obj) | ||
| with warnings.catch_warnings(): |
There was a problem hiding this comment.
I definitely don't think we want to add these in pydantic/main.py. What warnings are being raised?
There was a problem hiding this comment.
A warning about load_file() (called below) being deprecated, possibly more.
There was a problem hiding this comment.
Thanks for the pointer to the commit messages - that's helpful. We should instead not use deprecated methods instead of catching the warnings :)
There was a problem hiding this comment.
I don't think there's a point in rewriting a method that's deprecated itself, at this point.
There was a problem hiding this comment.
Well I mean we should use whatever the new api is...
There was a problem hiding this comment.
Oh. Do you mean we should remove tests for deprecated API then?
There was a problem hiding this comment.
Yeah, we should remove the catching of the warnings and write better code so that we don't have to catch warnings. I haven't done much research into what new calls we should make instead, though.
| with warnings.catch_warnings(): | ||
| # we need to explicitly ignore the other warning in pytest-8 | ||
| # TODO: rewrite it to use two nested pytest.warns() when pytest-7 is no longer supported | ||
| warnings.simplefilter("ignore") |
There was a problem hiding this comment.
Same question here, what warnings are being raised?
There was a problem hiding this comment.
It's in the commit message. When we're matching "Default value .* is not JSON serializable", it's emitting "Cannot generate a JsonSchema", and the other way around. I wasn't able to find a way to successfully match both simultaneously in pytest-7.
There was a problem hiding this comment.
What about something like this:
import warnings
import pytest
def test_multiple_warnings():
with pytest.warns((UserWarning, RuntimeWarning)) as record:
warnings.warn("This is a user warning", UserWarning)
warnings.warn("This is a runtime warning", RuntimeWarning)
assert len(record) == 2 # check that two warnings were raised
assert isinstance(record[0].message, UserWarning) # check the type of the first warning
assert str(record[0].message) == "This is a user warning" # check the first warning message
assert isinstance(record[1].message, RuntimeWarning) # check the type of the second warning
assert str(record[1].message) == "This is a runtime warning" # check the second warning messageObviously this would be specialized to the case for the warnings raised above
Change Summary
This is a work-in-progress of my attempts to fix pytest-8 test failures. I'm opening it early to facilitate discussion whether my approach is correct, particularly whether it's a good idea to ignore implementation-detail warnings from inside
parse_raw()andparse_file().Related issue number
fix #9025
Checklist