Skip to content

Fix airflow-core tests that silently required the full provider set - #71637

Open
coleheflin wants to merge 4 commits into
apache:mainfrom
coleheflin:move-core-provider-tests
Open

coleheflin wants to merge 4 commits into
apache:mainfrom
coleheflin:move-core-provider-tests

Conversation

@coleheflin

@coleheflin coleheflin commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor

What

  • Fixes 3 tests_common files (test_utils/logs.py, api_fastapi.py, providers.py) that imported airflow_shared.secrets_masker/module_loading — a private, never-published distribution only resolvable in a full local workspace checkout — instead of the vendored airflow._shared.* copy airflow-core bakes into its own installed package (same pattern already used elsewhere, e.g. this file's own pytest_plugin.py).
  • Adds a skip_unless_all_providers_installed marker (plus a couple of targeted pytest.importorskips) to 11 tests in test_providers_manager.py/test_configuration.py that assumed the full ~100+ provider set is always installed.
  • Links the existing TODO in airflow-core/pyproject.toml (about eventually dropping its own hardcoded provider dev-dependencies) to a new tracking issue, Drop airflow-core's own dev-group dependency on 6 providers #71641, since it had none.

Why

addresses (partially): #60770

uv sync --project airflow-core (a scoped, core-only sync) succeeds, but the test suite didn't run clean: collection crashed with ModuleNotFoundError: No module named 'airflow_shared' (it only worked before as a side effect of an unrelated full workspace sync), and 11 more tests failed on full-provider-set assumptions. Fixing both gets pytest airflow-core/tests/unit -m "not db_test" passing cleanly in that scoped environment.

(An earlier version fixed the import crash by adding those private packages as explicit devel-common dependencies. That broke the Compat 3.1.8/3.2.2/3.3.1 CI jobs instead — they deliberately strip local editable packages before installing a released apache-airflow wheel, so the new dependency reintroduced the same error there. Using the vendored airflow._shared.* path avoids the dependency entirely and survives that environment too.)

This only partially addresses #60770. Its title ("move remaining provider tests from airflow-core") implies a bigger migration: airflow-core's own dev group still hardcodes 6 providers (amazon, celery, cncf-kubernetes, fab, git, ftp) so tests depending on them still pass — nothing was moved or removed here. Untangling which tests genuinely need which of those 6, and moving vs. guarding each, is a much larger multi-PR effort, so it's tracked separately in #71641 rather than folded into this PR. Leaving #60770 open for a maintainer to decide whether this is enough to close it.

Testing performed

  • Reproduced both failure modes via uv sync --project airflow-core + pytest; after the fix, collection is clean (0 errors) and the non-db unit suite passes fully (0 failed, was 11 failed) — with no new dependency added.
  • Restored a full uv sync and reran the same files to confirm all tests still pass for real (marker correctly no-ops when all providers are present).
  • Reproduced the CI regression locally (scoped shared/observability venv, and a compat-style env with airflow-core but no standalone shared packages) to verify the intermediate approach failed and the final one passes.
  • Confirmed on CI: all 5 Compat jobs pass (including the two that broke on the intermediate approach), Shared observability tests passes, no failures anywhere.
  • Rebased on latest main; prek run --from-ref upstream/main --stage pre-commit (ruff, mypy) passes.

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Sonnet 5)

Generated-by: Claude Code (Sonnet 5) following the guidelines

@coleheflin
coleheflin force-pushed the move-core-provider-tests branch from 94846a3 to e7cb86a Compare August 17, 2026 23:48
@coleheflin
coleheflin marked this pull request as ready for review August 17, 2026 23:49
@github-actions

github-actions Bot commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

uv.lock on main just moved via #72921 ("[main] Upgrade important CI environment"), commit 937dfe2 and this PR currently conflicts.

Quickest fix:

git fetch upstream main && git rebase upstream/main
rm uv.lock && uv lock
git add uv.lock && git rebase --continue
git push --force-with-lease

Automated nudge — ignore if you're not ready to rebase. This comment is updated in place on future uv.lock bumps.

@shahar1 shahar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please resolve conflicts

A scoped `uv sync --project airflow-core` installs only the six
providers airflow-core's own dev group declares (amazon, celery,
cncf-kubernetes, fab, git, ftp), not the ~100+ providers a full
workspace sync brings in. Several airflow-core tests assumed the full
set was always present and failed rather than skipped when it wasn't,
so a minimal core-only environment couldn't run the test suite clean.

devel-common's tests_common also imported airflow_shared.secrets_masker
and airflow_shared.module_loading directly without declaring those
shared distributions as dependencies, relying on them being installed
as a side effect of a full workspace sync.
importlib.util.find_spec("airflow.providers.google") imports the
parent airflow package as a side effect, which broke shared/observability's
own isolated test suite (it doesn't install airflow-core, so importing
airflow.configuration failed on a missing jsonschema dependency).
Checking installed distribution metadata instead avoids the import.
…pendency

The previous fix added apache-airflow-shared-module-loading and
apache-airflow-shared-secrets-masker as devel-common dependencies, but
those distributions are marked "Private: Do Not Upload" and never
published — they only resolve via the local workspace. CI's provider
compat-testing jobs (Compat 3.1.8/3.2.2/3.3.1) deliberately uninstall
all such local editable packages before installing a real released
apache-airflow wheel, so the dependency broke virtually every provider
test in those jobs with the same ModuleNotFoundError this PR set out
to fix.

airflow-core already vendors this exact code into its own installed
package (airflow._shared.*, baked in at build time — see its sdist
force-include config) specifically so it's available regardless of
how airflow-core was installed. Importing from there instead — the
same pattern already used elsewhere in this codebase, including
tests_common's own pytest_plugin.py — needs no extra dependency and
survives the compat-testing environment.
The TODO already acknowledged this should go away eventually, but had
no tracking issue. Filed one covering the remaining migration scope
(dropping the 6 hardcoded providers once tests no longer need them
unconditionally), since it's a larger, separate piece of work from
the fixes in this PR.
@coleheflin
coleheflin force-pushed the move-core-provider-tests branch from b3640f9 to 49bf1d0 Compare September 14, 2026 02:47
@coleheflin

Copy link
Copy Markdown
Contributor Author

Rebased onto the latest main and resolved the conflicts; the PR is now mergeable.

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because the author has not responded to a request for more information. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions Bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Oct 4, 2026

This branch has not been deployed

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

Labels

pending-response stale Stale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants