Skip to content

Add tests for common compat standard utilities - #72487

Open
dhkim1920 wants to merge 2 commits into
apache:mainfrom
dhkim1920:test-common-compat-standard-utils
Open

Add tests for common compat standard utilities#72487
dhkim1920 wants to merge 2 commits into
apache:mainfrom
dhkim1920:test-common-compat-standard-utils

Conversation

@dhkim1920

Copy link
Copy Markdown
Contributor

Add coverage for the compatibility imports exposed by common.compat.standard.utils.

The tests verify every name in __all__ resolves through the compatibility import map and that unknown attributes retain the expected AttributeError contract. This removes the corresponding entry from OVERLOOKED_TESTS.

related: #35442


Was generative AI tooling used to co-author this PR?
  • Yes — Codex (GPT-5)

Generated-by: Codex (GPT-5) following the guidelines


Drafted-by: Codex (GPT-5); reviewed by @dhkim1920 before posting

@fat-catTW

Copy link
Copy Markdown
Contributor

Thanks for the contribution!

I think this test should use an independent expected list instead of parametrizing from utils.__all__.

Right now utils.__all__ is derived from _IMPORT_MAP, so the test only checks the exports that the implementation currently advertises. If an expected compat export like prepare_virtualenv, SkipMixin, or one of the XCom constants were accidentally removed from _IMPORT_MAP, it would also disappear from __all__, and this test would still pass without noticing.

Could we make the expected public exports explicit in the test, then assert both set(utils.__all__) and that each expected name resolves successfully? That would make the test protect the actual compatibility contract before removing this file from OVERLOOKED_TESTS.

Ensure the compatibility import surface stays covered as supported Airflow versions evolve.
Detect accidental removal of public compatibility exports even when the advertised export list changes with the import map.
@dhkim1920
dhkim1920 force-pushed the test-common-compat-standard-utils branch from 04943eb to 9081496 Compare September 5, 2026 11:53
@dhkim1920

dhkim1920 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

@fat-catTW
Thanks for the thoughtful review. You're right that deriving the test cases from __all__ could miss accidental removals. I've updated the test to define the expected exports explicitly, assert that __all__ matches them, and verify that each export resolves successfully.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants