-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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: Order-insensitive unit test equality assertion for expected/actual with multiple nulls #10202
Conversation
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10202 +/- ##
==========================================
+ Coverage 88.63% 88.68% +0.05%
==========================================
Files 180 180
Lines 22434 22449 +15
==========================================
+ Hits 19885 19910 +25
+ Misses 2549 2539 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
# Sort expected and actual inputs prior to creating daff diff to ensure order insensitivity | ||
# https://github.com/paulfitz/daff/issues/200 | ||
expected_daff_table = daff.PythonTableView(list_rows_from_table(expected, sort=True)) | ||
actual_daff_table = daff.PythonTableView(list_rows_from_table(actual, sort=True)) | ||
|
||
flags = daff.CompareFlags() |
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.
Moved flags
initialization higher up as they should also be used to create alignment
. This doesn't fix the issue, but it makes sense to use the same flags
for alignment
and diff
, assuming flags.order
worked perfectly :)
resolves #10167
Problem
TableDiff.hasDifference unexpectedly returns True when comparing two input tables that have multiple identical rows with containing None, even if CompareFlags.orderis set toFalse`.
Solution
None
, even if CompareFlags.order is False paulfitz/daff#200flags.ordered
as a backstop + in the hope that we can remove the custom sorting if daff has a fixChecklist