[MLv2] Keep order-by in sync when replacing breakouts #31165
Conversation
|
No failed tests 🎉 |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #31165 +/- ##
==========================================
- Coverage 72.56% 72.56% -0.01%
==========================================
Files 2910 2910
Lines 102117 102167 +50
Branches 12811 12816 +5
==========================================
+ Hits 74102 74137 +35
- Misses 22301 22311 +10
- Partials 5714 5719 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
| q4 (lib/replace-clause q3 (first (lib/breakouts q3)) month)] | ||
| (is (= :day (:temporal-unit (second (last (first (lib/order-bys q3))))))) | ||
| (is (= :month (:temporal-unit (second (last (first (lib/order-bys q4))))))))) | ||
| (testing "Binning should keep in order-by in sync" |
There was a problem hiding this comment.
It's a little hard to read big 110 line tests like this and even harder to debug them when you break something, you could have broken this out into 5 separate deftests since they're basically completely separate. I think splitting big tests out into a series of smaller ones is preferrable when it's easy to do, but since the 5 top-level testings don't share anything it would have been relatively easy
Fixes #30980
In MLv1 breakout that were ordered by the user were kept in sync when changing the bucket or binning on a breakout. Order by was also removed if the field changed or if the breakout was removed.
This maintains the behavior.