test(sqla): add the jinja orderby calculated column to the session#42274
Conversation
#41870 added test_jinja_calculated_column_in_order_by, which builds a TableColumn(table=table) and relies on backref cascading to keep it in table.columns. #42222 (merged the same day, after #41870's CI ran) set cascade_backrefs=False on that relationship, so the unadded column is lost on expiry and the orderby falls back to the bare column name, failing the test on every DB variant on master. Register the column with the session explicitly, matching the sibling test_calculated_column_in_order_by_base_engine_spec that #42222 itself updated the same way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Code Review Agent Run #b12679Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #42274 +/- ##
==========================================
+ Coverage 56.62% 65.19% +8.56%
==========================================
Files 2775 2775
Lines 156816 156816
Branches 35852 35852
==========================================
+ Hits 88798 102235 +13437
+ Misses 67186 52613 -14573
- Partials 832 1968 +1136
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…42274) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…spec Also retriggers CI so the merge ref picks up the master fix from #42274 (an empty commit does not trigger the path-gated workflows). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…spec Also retriggers CI so the merge ref picks up the master fix from #42274 (an empty commit does not trigger the path-gated workflows). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…spec Also retriggers CI so the merge ref picks up the master fix from #42274 (an empty commit does not trigger the path-gated workflows). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SUMMARY
Master CI is red on every Python-Integration DB variant since today:
test_jinja_calculated_column_in_order_byfails withassert "'male'" in ' max(gender_cc_jinja) ASC;'.Two PRs that were each green collided:
TableColumn(table=table)and relied on backref cascading to keep the column intable.columns.cascade_backrefs=Falseon the TableColumn↔SqlaTable relationship, so the never-added column is lost when the collection is refreshed, and the orderby falls back to the bare column name.The fix is the same one-liner #42222 applied to the sibling
test_calculated_column_in_order_by_base_engine_spec: register the column with the session explicitly viadb.session.add(column).BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (test-only change)
TESTING INSTRUCTIONS
pytest tests/integration_tests/db_engine_specs/base_engine_spec_tests.py -k order_by— both order-by calculated-column tests pass; on current master the jinja one fails.ADDITIONAL INFORMATION
🤖 Generated with Claude Code