fix(versioning): narrow UUIDs in restore tests - #42654
Conversation
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #42654 +/- ##
=======================================
Coverage 65.44% 65.44%
=======================================
Files 2810 2810
Lines 159362 159364 +2
Branches 36372 36372
=======================================
+ Hits 104301 104303 +2
Misses 53019 53019
Partials 2042 2042
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:
|
705ae03 to
ca87645
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes MyPy arg-type errors in the version-restore integration tests by narrowing nullable SQLAlchemy UUID fields to non-null UUIDs before calling derive_version_uuid, aligning the tests with the helper’s strict UUID contract.
Changes:
- Add explicit non-null assertions for entity UUIDs in chart, dashboard, and dataset version-restore tests.
- Reuse the narrowed UUID variables when deriving deterministic
version_uuidvalues.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/integration_tests/charts/version_restore_tests.py | Assert chart UUID is non-null before passing to derive_version_uuid; reuse narrowed UUID variable. |
| tests/integration_tests/dashboards/version_restore_tests.py | Assert dashboard UUID is non-null before passing to derive_version_uuid; reuse narrowed UUID variable in multiple tests. |
| tests/integration_tests/datasets/version_restore_tests.py | Assert dataset UUID is non-null before passing to derive_version_uuid across restore/listing tests. |
Suppressed comments (1)
tests/integration_tests/dashboards/version_restore_tests.py:298
target_txcomes from.scalar()and can beNoneif no version rows are found. PassingNoneintoderive_version_uuidwill compute a UUID for a non-existent version and fail later in a less obvious way. Add an explicitassert target_tx is not Nonebefore deriving to make failures clearer (and keep the call contract tight).
)
target_uuid = str(derive_version_uuid(entity_uuid, target_tx))
| ) | ||
| target_uuid = str(derive_version_uuid(dashboard.uuid, target_tx)) | ||
| target_uuid = str(derive_version_uuid(entity_uuid, target_tx)) |
| ) | ||
| target_uuid = str(derive_version_uuid(chart.uuid, first_tx)) | ||
| target_uuid = str(derive_version_uuid(entity_uuid, first_tx)) |
Code Review Agent Run #ad41f5Actionable 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 AI Code Review powered by |
|
Thanks @sadpandajoe, this should help unblocking the CI! |
SUMMARY
Narrow the nullable SQLAlchemy UUID attributes in the chart, dashboard, and
dataset version-restore integration tests before passing them to
derive_version_uuid.The scheduled pre-commit workflow on master reported 14 MyPy
arg-typeerrorsafter #42469 introduced these tests. The model fields are typed as
UUID | None,while the helper correctly requires
UUID. Explicit non-null assertions at thetest setup boundaries retain the runtime test behavior and satisfy the stricter
contract.
Observed failure: https://github.com/apache/superset/actions/runs/30611617987
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable; test-only typing fix.
TESTING INSTRUCTIONS
pre-commit run mypy --files tests/integration_tests/charts/version_restore_tests.py tests/integration_tests/dashboards/version_restore_tests.py tests/integration_tests/datasets/version_restore_tests.pypre-commit run --files tests/integration_tests/charts/version_restore_tests.py tests/integration_tests/dashboards/version_restore_tests.py tests/integration_tests/datasets/version_restore_tests.pypre-commit run --all-fileswas run. The affected MyPy hook passed; unrelated local baseline/environment failures remained in docs/frontend tooling and repository-wide linting.pytest-covplugin importscoverage.results.display_covered, which is absent from the installedcoveragepackage.ADDITIONAL INFORMATION