test(datasets): regression coverage for #16141 (export with same table name, different schemas)#40123
Conversation
…e name, different schemas) Issue #16141 (Aug 2021) reported that the dataset export API returned only one file when exporting two datasets that shared a table name but lived in different schemas (e.g. prod.users + dev.users). The current ExportDatasetsCommand disambiguates the export filename with the dataset id (`<table_name>_<id>.yaml`), so the collision should no longer occur — but no test pinned the behavior. This is a TDD-first PR: the test asserts the *fixed* behavior. If CI green, the bug is provably gone and #16141 can be closed; if CI red, the test documents exactly what still needs to be fixed for whoever picks up the work. Closes #16141 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Code Review Agent Run #3d5e37Actionable 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 #40123 +/- ##
==========================================
+ Coverage 64.15% 64.16% +0.01%
==========================================
Files 2590 2590
Lines 138104 138087 -17
Branches 32039 32039
==========================================
+ Hits 88599 88608 +9
+ Misses 47984 47954 -30
- Partials 1521 1525 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds a regression test for issue #16141, which previously caused dataset exports to collide when two datasets shared a table_name but had different schemas. The current ExportDatasetsCommand filenames already include the dataset id (<table_name>_<id>.yaml), so the test should now pass and pin the fix.
Changes:
- Adds a single unit test that creates two
SqlaTablerows with identicaltable_namebut distinct schemas and verifies the exports produce distinct paths and preserve each schema in the YAML payload.
sadpandajoe
left a comment
There was a problem hiding this comment.
Left one question but not blocking so approving.
|
Yes, using tests/unit_tests/datasets/commands/export_test.py |
…edback) @sadpandajoe and the bito reviewer both flagged that the schema extraction was hand-splitting the YAML string. yaml.safe_load is both clearer and more robust to formatting changes (key order, spacing, multi-line values). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Bito Automatic Review Skipped – PR Already Merged |
…e table name, different schemas) (apache#40123) Co-authored-by: Superset Dev <dev@superset.apache.org> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
SUMMARY
TDD-first regression coverage for #16141 (Aug 2021): exporting two datasets with the same
table_namebut different schemas (e.g.prod.users+dev.users) historically returned only one file because the export filename did not disambiguate the pair.The current
ExportDatasetsCommandalready produces<table_name>_<id>.yaml, so the original collision should no longer occur — but no test pinned the behavior, leaving the bug technically open and at risk of silent regression.This PR is a small experiment in a test-PR-first workflow: open a failing-or-passing test that encodes the bug's contract before doing any code changes. If CI passes, the bug is provably fixed and #16141 can close. If CI fails, the test documents exactly what still needs work for whoever picks up the fix.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — test-only.
TESTING INSTRUCTIONS
The test creates two
SqlaTablerows with identicaltable_name="users"and distinct schemas (prod,dev), runsExportDatasetsCommand._exporton each, and asserts:schema:field — neither is silently overwritten.ADDITIONAL INFORMATION