Allowing exclusions to be passed through to the test tasks#2006
Allowing exclusions to be passed through to the test tasks#2006anyapriya wants to merge 6 commits intoastronomer:mainfrom
Conversation
✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.
|
There was a problem hiding this comment.
Pull Request Overview
This PR enables exclusions to be passed through to test tasks for all test behaviors (after_each, after_all), allowing users to exclude specific tests or resource types without requiring workarounds. Previously, exclusions only worked with certain test behaviors.
- Added exclusions parameter to test task creation functions
- Updated test task metadata creation to include exclusions from render config
- Added comprehensive test coverage for exclusion behavior with different test scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| cosmos/airflow/graph.py | Added exclusions parameter passing and deepcopy usage for render config attributes |
| tests/test_converter.py | Added integration test validating exclusion behavior across different test scenarios |
| tests/airflow/test_graph.py | Updated test to include exclude parameter in expected metadata |
| docs/configuration/selecting-excluding.rst | Added documentation explaining exclusion behavior with test tasks |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| detached_from_parent = detached_from_parent or {} | ||
| task_owner = "" | ||
|
|
||
| task_args["exclude"] = deepcopy(exclusions) |
There was a problem hiding this comment.
The exclusions parameter can be None, but deepcopy(None) will return None. This should be handled explicitly to ensure task_args['exclude'] is always a list or None consistently.
| task_args["exclude"] = deepcopy(exclusions) | |
| task_args["exclude"] = deepcopy(exclusions) if exclusions is not None else None |
| node: DbtNode, | ||
| task_args: dict[str, str], | ||
| detached_from_parent: dict[str, DbtNode] | None = None, | ||
| detached_from_parent: dict[str, list[DbtNode]] | None = None, |
There was a problem hiding this comment.
The type annotation for detached_from_parent has changed from dict[str, DbtNode] to dict[str, list[DbtNode]] but this appears to be unrelated to the exclusions feature. This could be a breaking change that should be documented or handled separately.
| @@ -128,8 +128,9 @@ | |||
| on_warning_callback: Callable[..., Any] | None = None, | |||
| node: DbtNode | None = None, | |||
| render_config: RenderConfig | None = None, | |||
There was a problem hiding this comment.
The type annotation for detached_from_parent has changed from dict[str, DbtNode] to dict[str, list[DbtNode]] but this appears to be unrelated to the exclusions feature. This could be a breaking change that should be documented or handled separately.
| render_config: RenderConfig | None = None, | |
| render_config: RenderConfig | None = None, | |
| # BREAKING CHANGE: The type of detached_from_parent has changed from dict[str, DbtNode] to dict[str, list[DbtNode]] |
| test_indirect_selection: TestIndirectSelection = TestIndirectSelection.EAGER, | ||
| on_warning_callback: Callable[..., Any] | None = None, | ||
| detached_from_parent: dict[str, DbtNode] | None = None, | ||
| detached_from_parent: dict[str, list[DbtNode]] | None = None, |
There was a problem hiding this comment.
The type annotation for detached_from_parent has changed from dict[str, DbtNode] to dict[str, list[DbtNode]] but this appears to be unrelated to the exclusions feature. This could be a breaking change that should be documented or handled separately.
| normalize_task_id: Callable[..., Any] | None = None, | ||
| normalize_task_display_name: Callable[..., Any] | None = None, | ||
| detached_from_parent: dict[str, DbtNode] | None = None, | ||
| detached_from_parent: dict[str, list[DbtNode]] | None = None, |
There was a problem hiding this comment.
The type annotation for detached_from_parent has changed from dict[str, DbtNode] to dict[str, list[DbtNode]] but this appears to be unrelated to the exclusions feature. This could be a breaking change that should be documented or handled separately.
| detached_from_parent: dict[str, list[DbtNode]] | None = None, | |
| detached_from_parent: dict[str, DbtNode] | None = None, |
| "--select", | ||
| "combined_model", | ||
| "--exclude", | ||
| "resource_type:unit_test custom_test_combined_model_combined_model_", |
There was a problem hiding this comment.
[nitpick] The hardcoded test name 'custom_test_combined_model_combined_model_' is repeated multiple times. Consider extracting this to a variable to improve maintainability.
| "--select", | ||
| "model_a", | ||
| "--exclude", | ||
| "resource_type:unit_test custom_test_combined_model_combined_model_", |
There was a problem hiding this comment.
[nitpick] The hardcoded test name 'custom_test_combined_model_combined_model_' is repeated multiple times. Consider extracting this to a variable to improve maintainability.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2006 +/- ##
=======================================
Coverage 97.78% 97.78%
=======================================
Files 91 91
Lines 5862 5862
=======================================
Hits 5732 5732
Misses 130 130 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ssing_exclusions_to_test_tasks
|
This PR is stale because it has been open for 30 days with no activity. |
|
This PR is stale because it has been open for 30 days with no activity. |
|
This PR is stale because it has been open for 30 days with no activity. |
|
This PR was closed because it has been inactive for 10 days since being marked as stale. |
Description
Previously, exclusions were not being passed through to test tasks when using test behavior of after each or after all. This meant that if you were trying to exclude unit tests or a specific test, you were unable to do so easily without a workaround of also passing the exclusion into the operator args.
While this would be great to also do something similar with the select or selector (ex. select model A and test 2), I think it would be too complex to get the unions / intersections correct. I added documentation about this limitation so it's at least more clear.
Related Issue(s)
Closes #1763
Closes #1865
Breaking Change?
No
Checklist