Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes pytest compatibility issues by ensuring that test functions do not inadvertently return a value, which can cause test failures in pytest 8.4.0. The changes remove the non-None return in an edge-case within _test_array_recode and update the corresponding test functions.
- Remove returning True for the dtype.char == "S" edge case.
- Refactor test functions to call _test_array_recode without returning its value.
| # should ever do this, but we do need it for input. That's why we can't test this right now, because we | ||
| # can't do it full circle. | ||
| return True | ||
| return |
There was a problem hiding this comment.
Returning None here avoids sending an unintended value back to the caller, in line with pytest 8.4.0 expectations. Ensure this behavior is intentional for the edge case when dtype.char equals 'S'.
| @conftest.override_sparsify("partial") | ||
| def test_array_recode_sparsify_partial(array_with_index_and_table): | ||
| return _test_array_recode(array_with_index_and_table) | ||
| _test_array_recode(array_with_index_and_table) |
There was a problem hiding this comment.
Removing the return value here is appropriate since _test_array_recode already asserts internally. This change prevents unnecessary return propagation that could lead to test failures.
| @conftest.override_sparsify("none") | ||
| def test_array_recode_sparsify_none(array_with_index_and_table): | ||
| return _test_array_recode(array_with_index_and_table) | ||
| _test_array_recode(array_with_index_and_table) |
There was a problem hiding this comment.
Similarly, updating this test function to not return the value from _test_array_recode ensures consistency with pytest 8.4.0 requirements. The assertion within _test_array_recode is sufficient to validate the test outcome.
solatis
left a comment
solatis
left a comment
There was a problem hiding this comment.
Make sure to test it builds everywhere before merging.
|
other than flaky |
Since pytesty 8.4.0 tests will fail, instead of raising a warning, if they return any value other than None.
https://docs.pytest.org/en/stable/changelog.html#pytest-8-4-0-2025-06-02
Issue was identified in
tests/test_convert.pytests.To fix:
dtype.char == "S"edge case