DM-42605: list chained collection children in search order #947
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #947 +/- ##
=======================================
Coverage 88.39% 88.39%
=======================================
Files 303 303
Lines 39082 39082
Branches 8237 8237
=======================================
Hits 34547 34547
Misses 3341 3341
Partials 1194 1194 ☔ View full report in Codecov by Sentry. |
| ("", "", "run1"), | ||
| ("", "", "tag1"), | ||
| ("", "", "chain2"), |
There was a problem hiding this comment.
It's a bit odd that this test didn't catch the bug; how was the value of expected chosen?
There was a problem hiding this comment.
I assume it's a classic case of:
- Developer knows they're breaking the test for good reason.
- Developer looks at the test output is after their change.
- Developer thinks about whether that it reasonable and adopts it (but is subject to confirmation bias).
I suppose it's good that we were at least trying to test this before, even if we failed to do it correctly.
There was a problem hiding this comment.
I would say this is a classic case of the problem definition being simple enough that you can work out the correct output in your head... 😬
kfindeisen
force-pushed
the
tickets/DM-42605
branch
from
624744f
to
d910dec
Compare
| ("", "", "run1"), | ||
| ("", "", "tag1"), | ||
| ("", "", "chain2"), |
There was a problem hiding this comment.
I assume it's a classic case of:
- Developer knows they're breaking the test for good reason.
- Developer looks at the test output is after their change.
- Developer thinks about whether that it reasonable and adopts it (but is subject to confirmation bias).
I suppose it's good that we were at least trying to test this before, even if we failed to do it correctly.
The TABLE mode of butler query-collections, unlike the default TREE, listed child collections in alphabetical order. This has now been changed to match TREE's behavior. No changes were needed for FLATTEN, which ignores chains anyway.
kfindeisen
force-pushed
the
tickets/DM-42605
branch
from
d910dec
to
1007d69
Compare
This PR fixes a bug where the
TABLEmode ofbutler query-collectionslisted children in alphabetical order, obscuring the exact definition of a chained collection.Checklist
doc/changes