Add tests for BaseConfigKeys iteration - #71844
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
|
There was a problem hiding this comment.
Pull request overview
Adds a missing unit test module in the Amazon provider to cover BaseConfigKeys.__iter__, and updates the project-structure “overlooked tests” allowlist accordingly. This supports the ongoing provider test-module coverage effort tracked in #35442 without changing runtime behavior.
Changes:
- Added a dedicated unit test validating
BaseConfigKeysiteration returns the subclass’ non-dunder attribute values. - Removed the corresponding path from
OVERLOOKED_TESTSnow that the module has a real test file.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| providers/amazon/tests/unit/amazon/aws/executors/utils/test_base_config_keys.py | New unit test covering BaseConfigKeys.__iter__ behavior. |
| airflow-core/tests/unit/always/test_project_structure.py | Removes the new test file from the OVERLOOKED_TESTS exclusion list. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
SameerMesiah97
left a comment
There was a problem hiding this comment.
I know that this PR was opened in response to Issue #35442 but that is almost 3 years old and may not be relevant anymore.
Also, I am struggling to see the value of this new test. It verifies the implementation of BaseConfigKeys.__iter__ using an artificial subclass, but what user-facing or internal contract relies on iteration returning these values?
Maybe you could see if there are genuine gaps that require test coverage in BaseConfigKeys instead of adding a test for its own sake?
Add a dedicated unit test for
BaseConfigKeys.__iter__and remove the matchingOVERLOOKED_TESTSentry. This follows the provider test-module coverage work tracked in #35442.No runtime behavior changes.
related: #35442
Testing
breeze run pytest providers/amazon/tests/unit/amazon/aws/executors/utils/test_base_config_keys.py airflow-core/tests/unit/always/test_project_structure.py::TestProjectStructure::test_providers_modules_should_have_tests -xvs(2 passed)breeze run mypy providers/amazon/tests/unit/amazon/aws/executors/utils/test_base_config_keys.pyprek run --from-ref upstream/main --stage pre-commitbreeze testing providers-tests --test-type "Providers[amazon]"(4737 passed,9 skipped)breeze testing core-tests --test-type Always(2020 passed,66 skipped,1 xfailed; one unrelated Azure Batch SDK import failure)Was generative AI tooling used to co-author this PR?
Generated-by: Codex (GPT-5.6 Sol) following the guidelines
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.Drafted-by: Codex (GPT-5.6 Sol); reviewed by @lbbc-lh before posting