Skip to content

Comments

Add pre-commit hook to prevent test-only imports in production source#61713

Merged
shahar1 merged 1 commit intoapache:mainfrom
shahar1:prevent-tests-common-runtime-imports
Feb 10, 2026
Merged

Add pre-commit hook to prevent test-only imports in production source#61713
shahar1 merged 1 commit intoapache:mainfrom
shahar1:prevent-tests-common-runtime-imports

Conversation

@shahar1
Copy link
Contributor

@shahar1 shahar1 commented Feb 10, 2026

related: #60662

Adds a pre-commit hook that prevents test-only module imports from leaking into production source code (*/src/). This addresses the class of bugs demonstrated by #61673, where a tests_common import in the kubernetes provider caused a ModuleNotFoundError at runtime.

The hook detects two categories of forbidden imports:

  1. tests_common.* — the apache-airflow-devel-common package is dev-only.
  2. *.tests.* / tests.* — any import whose module path contains a .tests. component (e.g. from providers.cncf.kubernetes.tests.foo import bar). Test directories are not shipped in package wheels.

Note (S.E.): the latter should be a relatively low risk because of how the current namespaces work. It could be somewhat helpful in case you messed up the imports while working locally, but such cases could be indicated quickly. I could remove if it's an overkill, the tests_common use case is more prone.

Strictness (see comment on the PR regarding necessity):
All imports from these modules are forbidden, including those guarded by if TYPE_CHECKING:.

This PR also fixes an existing violation in BashOperator (standard provider) by moving ArgNotSet compatibility logic to version_compat.py.

The hook is AST-based (similar to check_airflow_imports_in_shared.py) and runs on all src/ files.


Was generative AI tooling used to co-author this PR?
  • Yes (GitHub Copilot)

Generated-by: GitHub Copilot following the guidelines

@boring-cyborg boring-cyborg bot added area:dev-tools backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch labels Feb 10, 2026
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

It seems we haven't added corresponding entry in .pre-commit-config.yaml.

@shahar1 shahar1 marked this pull request as draft February 10, 2026 09:08
@shahar1 shahar1 force-pushed the prevent-tests-common-runtime-imports branch from 68ae9df to d6b639c Compare February 10, 2026 09:13
@shahar1 shahar1 changed the title Add pre-commit hook to prevent runtime tests_common imports Add pre-commit hook to prevent test-only imports in production source Feb 10, 2026
@shahar1
Copy link
Contributor Author

shahar1 commented Feb 10, 2026

It seems we haven't added corresponding entry in .pre-commit-config.yaml.

Yeah, just figured that I've missed it :)
Also, I'm expanding it to imports from tests in general (not only test_common).

@shahar1 shahar1 force-pushed the prevent-tests-common-runtime-imports branch 2 times, most recently from 60286cc to 3fec785 Compare February 10, 2026 09:34
@shahar1 shahar1 marked this pull request as ready for review February 10, 2026 09:38
@shahar1 shahar1 requested a review from jason810496 February 10, 2026 09:45
@shahar1 shahar1 force-pushed the prevent-tests-common-runtime-imports branch 2 times, most recently from 081aeca to aa5a9d8 Compare February 10, 2026 10:40
@shahar1 shahar1 marked this pull request as draft February 10, 2026 11:26
@shahar1 shahar1 force-pushed the prevent-tests-common-runtime-imports branch from aa5a9d8 to 816730d Compare February 10, 2026 11:44
@shahar1 shahar1 force-pushed the prevent-tests-common-runtime-imports branch from 816730d to 4580241 Compare February 10, 2026 11:48
@shahar1 shahar1 marked this pull request as ready for review February 10, 2026 11:53
@shahar1 shahar1 merged commit 6983599 into apache:main Feb 10, 2026
296 of 300 checks passed
@github-actions
Copy link

Backport failed to create: v3-1-test. View the failure log Run details

Status Branch Result
v3-1-test Commit Link

You can attempt to backport this manually by running:

cherry_picker 6983599 v3-1-test

This should apply the commit to the v3-1-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

If you don't have cherry-picker installed, see the installation guide.

@jscheffl
Copy link
Contributor

Cool!

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

Labels

area:dev-tools backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants