-
-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(testing): pytest.approx correctly take account Mapping keys order to compare them
#13815
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
base: main
Are you sure you want to change the base?
Conversation
|
Hello @bluetech, can I have a review here ? It should be fast. |
ad30e6c to
99d108b
Compare
|
Hello @nicoddemus, thanks for your review for #13818 Here is a similar contribution to take a look to when you have the time |
…paring mappings with different keys
…er to compare them
…apping keys order
| for approx_key, approx_value in approx_side_as_map.items(): | ||
| other_value = other_side.get(approx_key, None) |
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.
Let's access the key directly, because the previous checks already ensure both dicts have the same set of keys.
| for approx_key, approx_value in approx_side_as_map.items(): | |
| other_value = other_side.get(approx_key, None) | |
| for approx_key, approx_value in approx_side_as_map.items(): | |
| other_value = other_side[approx_key] |
| def test_approx_on_unordered_mapping_with_mismatch(self) -> None: | ||
| """https://github.com/pytest-dev/pytest/pull/12445""" | ||
| expected = {"a": 1, "c": 3} | ||
| actual = {"c": 5, "a": 1} | ||
|
|
||
| with pytest.raises( | ||
| AssertionError, | ||
| match="Mismatched elements: 1 / 2:\n Max absolute difference: 2\n", | ||
| ): | ||
| assert expected == approx(actual) |
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.
Given the output message is central to the issue, please rewrite this test using pytester and verifying the output itself.
Also, let's use a few more keys in both dictionaries to cover more ground. I suggest:
expected = {"a": 1, "b": 2, "c": 3, "d": 4}
actual = {"d": 4, "c": 5, "a": 8}
Hello,
This PR fixes an issue with
pytest.approxwhere the error message incorrectly reported all elements as mismatched when comparing mappings with different key orders, even when only some values differed.The original code paired values by position rather than by key. This caused incorrect mismatch reporting when dictionary keys were in different orders.
This closes #12444.
Checklist
changelogfolder, named<ISSUE NUMBER>.<TYPE>.rst.