Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL #816

Merged
merged 4 commits into from
Jan 26, 2024

Conversation

tatiana
Copy link
Collaborator

@tatiana tatiana commented Jan 25, 2024

Before, when using TestBehavior.AFTER_ALL, it did not consider the select and exclude settings defined in the rendering configuration. This was an error since it would run all the dbt project tests, even if they were outside of the scope of the Cosmos-defined DAG.

Now we take into account select, selector and exclude when using this test behaviour.

Closes: #643

@tatiana tatiana requested a review from a team as a code owner January 25, 2024 11:53
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jan 25, 2024
Copy link

netlify bot commented Jan 25, 2024

Deploy Preview for sunny-pastelito-5ecb04 canceled.

Name Link
🔨 Latest commit 617338d
🔍 Latest deploy log https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/65b261310e483b0008a8de1c

@dosubot dosubot bot added area:selector Related to selector, like DAG selector, DBT selector, etc dbt:test Primarily related to dbt test command or functionality execution:local Related to Local execution environment parsing:custom Related to custom parsing, like custom DAG parsing, custom DBT parsing, etc labels Jan 25, 2024
@tatiana tatiana changed the title Fix forwarding selectors to AFTERALL test Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL Jan 25, 2024
Copy link

codecov bot commented Jan 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9c090a4) 94.76% compared to head (617338d) 94.81%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #816      +/-   ##
==========================================
+ Coverage   94.76%   94.81%   +0.04%     
==========================================
  Files          55       55              
  Lines        2447     2470      +23     
==========================================
+ Hits         2319     2342      +23     
  Misses        128      128              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@jlaneve jlaneve left a comment

Choose a reason for hiding this comment

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

Nice! Great catch and great fix

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 25, 2024
@tatiana tatiana merged commit 3040887 into main Jan 26, 2024
42 checks passed
@tatiana tatiana deleted the fix-643 branch January 26, 2024 22:44
tatiana added a commit that referenced this pull request Jan 26, 2024
…ALL` (#816)

Before, when using `TestBehavior.AFTER_ALL`, it did not consider the
`select` and `exclude` settings defined in the rendering configuration.
This was an error since it would run all the dbt project tests, even if
they were outside of the scope of the Cosmos-defined DAG.

Now we take into account `select`, `selector` and `exclude` when using
this test behaviour.

Closes: #643
(cherry picked from commit 3040887)
tatiana added a commit that referenced this pull request Jan 26, 2024
Bug fixes

* Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803
* Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805
* Fix custom selector when test node has no depends_on values by @tatiana in #814
* Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL (#816)

Others

* Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in #812
* pre-commit updates in #799, #807
tatiana added a commit that referenced this pull request Jan 26, 2024
Bug fixes

* Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803
* Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805
* Fix custom selector when test node has no depends_on values by @tatiana in #814
* Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL (#816)

Others

* Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in #812
* pre-commit updates in #799, #807
tatiana added a commit that referenced this pull request Jan 26, 2024
Bug fixes

* Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803
* Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805
* Fix custom selector when test node has no depends_on values by @tatiana in #814
* Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL by @tatiana in #816

Others

* Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in #812
* pre-commit updates in #799, #807
@tatiana tatiana mentioned this pull request Jan 26, 2024
tatiana added a commit that referenced this pull request Jan 26, 2024
**Bug fixes**

* Fix: ensure ``DbtGraph.update_node_dependency`` is called for all load
methods by @jbandoro in #803
* Fix: ensure operator ``execute`` method is consistent across all
execution base subclasses by @jbandoro in #805
* Fix custom selector when ``test`` node has no ``depends_on`` values by
@tatiana in #814
* Fix forwarding selectors to test task when using
``TestBehavior.AFTER_ALL`` by @tatiana in #816

**Others**

* Docs: Remove incorrect docstring from ``DbtLocalBaseOperator`` by
@jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and
@tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in
#812
* pre-commit updates in #799, #807
tatiana added a commit that referenced this pull request Jan 26, 2024
Bug fixes

* Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803
* Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805
* Fix custom selector when test node has no depends_on values by @tatiana in #814
* Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL by @tatiana in #816

Others

* Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in #812
* pre-commit updates in #799, #807
@tatiana tatiana mentioned this pull request Jan 27, 2024
tatiana added a commit that referenced this pull request Jan 27, 2024
**Bug fixes**

* Fix: ensure ``DbtGraph.update_node_dependency`` is called for all load
methods by @jbandoro in #803
* Fix: ensure operator ``execute`` method is consistent across all
execution base subclasses by @jbandoro in #805
* Fix custom selector when ``test`` node has no ``depends_on`` values by
@tatiana in #814
* Fix forwarding selectors to test task when using
``TestBehavior.AFTER_ALL`` by @tatiana in #816

**Others**

* Docs: Remove incorrect docstring from ``DbtLocalBaseOperator`` by
@jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and
@tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in
#812
* pre-commit updates in #799, #807

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Justin Bandoro <79104794+jbandoro@users.noreply.github.com>
Co-authored-by: Jakob Aron Hvitnov <141235900+jakob-hvitnov-telia@users.noreply.github.com>
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this pull request Jul 14, 2024
…ALL` (astronomer#816)

Before, when using `TestBehavior.AFTER_ALL`, it did not consider the
`select` and `exclude` settings defined in the rendering configuration.
This was an error since it would run all the dbt project tests, even if
they were outside of the scope of the Cosmos-defined DAG.

Now we take into account `select`, `selector` and `exclude` when using
this test behaviour.

Closes: astronomer#643
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this pull request Jul 14, 2024
**Bug fixes**

* Fix: ensure ``DbtGraph.update_node_dependency`` is called for all load
methods by @jbandoro in astronomer#803
* Fix: ensure operator ``execute`` method is consistent across all
execution base subclasses by @jbandoro in astronomer#805
* Fix custom selector when ``test`` node has no ``depends_on`` values by
@tatiana in astronomer#814
* Fix forwarding selectors to test task when using
``TestBehavior.AFTER_ALL`` by @tatiana in astronomer#816

**Others**

* Docs: Remove incorrect docstring from ``DbtLocalBaseOperator`` by
@jakob-hvitnov-telia in astronomer#797
* Add more logs to troubleshoot custom selector by @tatiana in astronomer#809
* Fix OpenLineage integration documentation by @tatiana in astronomer#810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and
@tatiana in astronomer#806
* Use Airflow constraint file for test environment setup by @jbandoro in
astronomer#812
* pre-commit updates in astronomer#799, astronomer#807

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Justin Bandoro <79104794+jbandoro@users.noreply.github.com>
Co-authored-by: Jakob Aron Hvitnov <141235900+jakob-hvitnov-telia@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:selector Related to selector, like DAG selector, DBT selector, etc dbt:test Primarily related to dbt test command or functionality execution:local Related to Local execution environment lgtm This PR has been approved by a maintainer parsing:custom Related to custom parsing, like custom DAG parsing, custom DBT parsing, etc size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After all test behaviour with select and exclude
2 participants