mirror of
https://github.com/apache/superset.git
synced 2026-09-06 07:21:35 +00:00
Claude Code
6ec8ffecb2
`generate_dashboard()` re-fetches with eager-loaded relationships via
`subqueryload(Dashboard.slices).subqueryload(Slice.editors)` (and
similarly for `.tags`, `Dashboard.editors`, `Dashboard.tags`), using a
`Dashboard` class resolved through a deferred `from
superset.models.dashboard import Dashboard` import inside the function --
deferred specifically so `@patch("superset.models.dashboard.Dashboard")`
in tests can substitute it.
`test_generate_dashboard_refetches_via_dao` and
`test_generate_dashboard_restricted_user_redacts_chart_datasource_name`
both patch the whole `Dashboard` class to control its constructor
(`mock_dashboard_cls.return_value = dashboard`), which leaves
`Dashboard.slices`/`.editors`/`.tags` as unconfigured auto-mocks. Passing
those to `subqueryload()` now raises `sqlalchemy.exc.ArgumentError:
Wildcard token cannot be followed by another entity` -- `ArgumentError`
subclasses `SQLAlchemyError`, which `generate_dashboard`'s re-fetch
already catches and treats as "re-fetch failed, return minimal
response", so both tests silently got the fallback response instead of
the real one they were asserting against.
Confirmed via a side-by-side instrumented run against SQLAlchemy 1.4.54:
the exact same mock chain there raised no error at all -- 1.4 didn't
eagerly validate `subqueryload()`'s argument type, so an unconfigured
MagicMock silently worked. This is a genuine SQLAlchemy 2.0 strictness
change in loader-option construction, but it doesn't affect real
production code (where `Dashboard` is never mocked and `.slices` is
always a real `InstrumentedAttribute`) -- it only breaks these two
tests' mocks, which were never fully specified to begin with.
Capture the real `Dashboard` class in a module-level `_RealDashboard`
import (before any per-test `@patch` can shadow it) and copy its
`slices`/`editors`/`tags` relationship attributes onto the mocked class
in `_setup_generate_dashboard_mocks`, so `subqueryload()` sees genuine
mapped attributes while `Dashboard(...)` construction stays mocked. Ran
the full 64-test file afterward, including the sibling
`..._refetch_sqlalchemy_error_rollback` test (which forces a real
`SQLAlchemyError` via `find_by_id.side_effect` and still correctly hits
the fallback path) -- no regressions.