Skip to content

test(airflow): isolate yield_pipeline_status tests from SQLAlchemy registry pollution#28995

Merged
pmbrull merged 2 commits into
mainfrom
fix/airflow-dagrun-mock-registry-isolation
Jun 12, 2026
Merged

test(airflow): isolate yield_pipeline_status tests from SQLAlchemy registry pollution#28995
pmbrull merged 2 commits into
mainfrom
fix/airflow-dagrun-mock-registry-isolation

Conversation

@IceS2

@IceS2 IceS2 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Problem

tests/unit/airflow/test_airflow_metadata.py::TestYieldPipelineStatus::{test_uses_start_date_when_logical_date_is_none, test_skips_run_when_both_dates_are_none} intermittently fail in CI (e.g. on #28977) with:

dag_run = MagicMock(spec=DagRun)
...
sqlalchemy.exc.InvalidRequestError: One or more mappers failed to initialize -
can't proceed with initialization of other mappers.
Triggering mapper: 'Mapper[DagWarning(dag_warning)]'.
Original exception was: 'NullType' object has no attribute 'length'

Root cause

Building MagicMock(spec=DagRun) makes unittest.mock walk dir(DagRun). DagRun's association proxies resolve through orm.class_mapper(), which calls _check_configure()_configure_registries({registry}, cascade=True) — i.e. configuring the entire shared SQLAlchemy mapper registry just to build the mock.

If any other test scheduled on the same pytest -n auto worker has already left a mapper in a failed state (here airflow's DagWarning mapper, marked _configure_failed), that cascade re-raises and these two tests fail — even though the failure has nothing to do with the date-fallback logic they verify.

Because xdist work distribution shifts with the overall test corpus, the two tests fail on some PRs (#28977) and pass on others (#28991) despite identical base — which reads as flakiness. The tests pass in isolation and when their file is run alone.

Fix

Build the fake DagRun with types.SimpleNamespace carrying exactly the attributes yield_pipeline_status reads (run_id, dag_id, state, logical_date, start_date). It never introspects DagRun, so it cannot trigger mapper configuration, and — being a real object — still raises AttributeError on unexpected attribute access, preserving the typo-safety spec= gave.

Test-only change; the 16 tests in the file pass unchanged.

…gistry pollution

TestYieldPipelineStatus built its fake DagRun with MagicMock(spec=DagRun).
Building a spec'd mock makes unittest.mock walk dir(DagRun); DagRun's
association proxies resolve through orm.class_mapper(), which forces
SQLAlchemy to configure the entire shared mapper registry (cascade=True).

If any other test on the same xdist worker has left a mapper in a failed
state (e.g. airflow's DagWarning mapper failing to initialize), that
cascade re-raises InvalidRequestError and these two date-fallback tests
fail for reasons unrelated to what they verify. Because xdist work
distribution shifts with the test corpus, the failure surfaces on some
PRs and not others, which reads as flakiness.

Use a SimpleNamespace carrying exactly the attributes yield_pipeline_status
reads. It never introspects DagRun, so it cannot trigger mapper
configuration, and it still raises AttributeError on unexpected access.
@IceS2 IceS2 requested a review from a team as a code owner June 12, 2026 08:50
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

🟡 Playwright Results — all passed (13 flaky)

✅ 4291 passed · ❌ 0 failed · 🟡 13 flaky · ⏭️ 88 skipped

Shard Passed Failed Flaky Skipped
✅ Shard 1 301 0 0 4
🟡 Shard 2 811 0 1 9
🟡 Shard 3 809 0 3 8
🟡 Shard 4 848 0 2 12
🟡 Shard 5 733 0 1 47
🟡 Shard 6 789 0 6 8
🟡 13 flaky test(s) (passed on retry)
  • Features/DataQuality/ColumnLevelTests.spec.ts › Column Values To Not Match Regex (shard 2, 1 retry)
  • Features/IncidentManager.spec.ts › Resolving incident & re-run pipeline (shard 3, 1 retry)
  • Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
  • Features/Table.spec.ts › Table pagination with sorting should works (shard 3, 1 retry)
  • Flow/PersonaFlow.spec.ts › Set default persona for team should work properly (shard 4, 1 retry)
  • Pages/CustomProperties.spec.ts › Hyperlink (shard 4, 1 retry)
  • Pages/ExplorePageRightPanel_KnowledgeCenter.spec.ts › Should remove user owner for knowledgeCenter (shard 5, 2 retries)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage service type filter selection (shard 6, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: searchIndex (shard 6, 1 retry)
  • Pages/Lineage/PlatformLineage.spec.ts › Verify domain platform view (shard 6, 1 retry)
  • Pages/LogsViewer.spec.ts › Logs page shows breadcrumb, summary, and log viewer or empty state after opening from bundle suite pipeline tab (shard 6, 1 retry)
  • Pages/Users.spec.ts › Admin soft & hard delete and restore user (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@pmbrull pmbrull merged commit 0c4233a into main Jun 12, 2026
50 of 51 checks passed
@pmbrull pmbrull deleted the fix/airflow-dagrun-mock-registry-isolation branch June 12, 2026 13:53
@gitar-bot

gitar-bot Bot commented Jun 12, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Replaces MagicMock with SimpleNamespace in yield_pipeline_status tests to prevent SQLAlchemy mapper registry pollution. This change eliminates intermittent CI failures caused by cross-test dependency on registry configuration.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ingestion safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants