Allowing for correct name in select clause in the case of detached tests#1680
Conversation
dosubot
Bot
added
the
size:XS
✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.
|
dosubot
Bot
added
area:selector
|
Noticing on the integration tests that the following tests (from tests/test_converter.py) are now failing like they were expecting the number at the end...
I don't want to blindly update them if that's the intended functionality, so I'm going to wait for input. |
tatiana
left a comment
tatiana
left a comment
There was a problem hiding this comment.
Hi @anyapriya, thank you for reporting / proposing a fix for the issue.
Please, could you please add a test-case so we can further understand the problem - and make sure that once the fix in place, we don't have regressions?
Also, it would be great if the CI checks can be green.
|
@tatiana I left a comment above yours. There's currently a test that's assuming the ".{id}" will be included when selecting this test, so it seems more intenteional rather than a miss. Before I modify that test, I wanted to check if that's intended functionality, or if the ".{id}" is a mistake, and make sure that my change won't cause issues. Any idea? Happy to add in the test once I get confirmation. |
…st ID in it and thus dbt fails to find the test
|
I took a look at the original PR and didn't see a clear reason to keep the ".{id}" so I did modify the tests (though I've been struggling to get the integration tests to run locally so haven't verified that they pass now, apologies). |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1680 +/- ##
=======================================
Coverage 97.42% 97.43%
=======================================
Files 80 80
Lines 5019 5021 +2
=======================================
+ Hits 4890 4892 +2
Misses 129 129 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
The change looks correct and needed to me @anyapriya . I tested on my end that without this fix, and see that the expected test is NOT selected and run. And this fix accurately selects and runs the test. Thanks a lot for helping us in reporting and fixing this issue, much appreciated!
Also, for next time, if you would like to run the integration tests locally, the Cosmos doc https://astronomer.github.io/astronomer-cosmos/contributing.html should help. It also has the section for running integration tests and for it to work, you'll also have to ensure that you run the previous steps in the guide. Let us know if you face any challenges after following the guide wrt to running the integration tests, and we will work on any missing sections in the doc.
I will wait for a while before merging the PR -- for my peers if they would like to review the PR as well, but we will definitely get this in the upcoming Cosmos 1.10 release scheduled for next week! :) Thanks again!
|
@pankajkoti Thanks for the info, but that's the guide I've been using and still running into issues. I'll see if I can put together the specific problems later. |
tatiana
left a comment
tatiana
left a comment
There was a problem hiding this comment.
Looks great, thank you @anyapriya !
Features * Airflow 3 support * Support running ``dbt deps`` incrementally to pre-defined ``dbt_packages`` by @tatiana in #1668 and #1670 * Add ``DuckDB`` profile mapping by @prithvijitguha and @pankajastro in #1553 * Implement DBT exposure selector by ghjklw #1717 Bug Fixes * Fix ``test_indirect_selection`` flag to be propagated in case of ``TestBehavior.BUILD`` by @corsettigyg in #1663 * Fix ``select`` clause in the case of detached tests by @anyapriya in #1680 * Operator argument fixes by @johnhoran in #1648 Airflow 3 Support * Support rendering DbtDag in Airflow 3 by @tatiana and @ashb in #1657 * Refactor Rendered Task Instance Fields (RTIF) handling for Airflow 2.x and 3.x by @pankajkoti in #1661 * Run cosmos operator in Airflow 3 by @pankajastro in #1642 * Fix ``python_virtualenv.prepare_env`` top-level import for Airflow 3 by @pankajkoti in #1678 * Fix Variable not found issue in Airflow 3 by @tatiana in #1684 * Disable CosmosPlugin on Airflow 3 setup by @pankajkoti in #1692, #1698 * Use ``schedule`` param in example DAGs instead of the 2.10 deprecated and 3.0 removed ``schedule_interval`` by @pankajkoti in #1701 * Ensure ``virtualenv_dir`` path exists by @pankajkoti in #1724 * Support emitting Assets with Airflow 3 by @tatiana in #1713 * Add docs on Airflow 3 compatibility by @pankajkoti and @tatiana in #1731 * Introduce, test and document asset/dataset breaking change by @tatiana in #1672 * Improve dataset/asset driven scheduling documentation by @tatiana in #1729 Enhancements * Allow multiple callbacks by @corsettigyg #1693 * Refactor kubernetes warning callback handling by @canbekley in #1681 Documentation * Add documentation related to ``copy_dbt_packages`` by @tatiana in #1671 * Make wording and command consistent in the contributing doc by @pankajkoti in #1697 * Add MonteCarlo callback example for importing dbt artifacts by @corsettigyg #1695 * Change async feature to be non-experimental by @tatiana in #1732 Others * Add sample ``dbt_packages`` to validate incremental ``dbt deps`` by @tatiana in #1669 * Add kubernetes execution mode example in Airflow 3 by @pankajastro in #1667 * Check only major version until Airflow 3 stable release by @pankajastro in #1665 * Install Airflow from main branch by @pankajastro in #1660 * Add dev tool for Airflow 3 by @pankajastro and @tatiana in #1627 * Improve Airflow 3 tooling by @pankajastro in #1656 * Skip associating ``openlineage_events_completes`` to ``ti`` in Airflow 3 by @pankajkoti in #1662 * Add .gitignore file for the scripts/airflow3 directory by @pankajkoti in #1658 * Remove ``original_jaffle_shop`` dbt project by @pankajkoti in #1676 * Fix or ignore type check error by @pankajastro in #1687 * Run virtualenv example with Airflow 3 tooling by @pankajastro in #1686 * Enable running setup/teardown tasks with Async execution DAG with Airflow 3 tooling by @pankajastro in #1696 * Enable integration tests for the DuckDB adapter by @pankajastro in #1699 * Add Airflow 3 tests matrix entries in CI by @pankajkoti in #1646 * Use a different way to get tasks count for asserting test_perf_dag by @pankajkoti in #1714 * Reinstall Airflow 3 dependency on ``pydantic>=2.11`` for dbt adapter versions 1.6 & 1.9 by @pankajkoti in #1715 * Fix outdated ``echo`` in Airflow 3 tooling script #1700 * Add files not needed for git tracking to .gitignore by @pankajkoti in #1723 * Use latest minor versions for dbt adapters to get in compatibility fixes by @pankajkoti in #1719 * Fix Airflow 3 tests raising generate_run_id() takes 0 positional arguments by @tatiana in #1725 * Fix dataset tests failing in Airflow 3 by @tatiana in #1716 * Enable example DAGs to run in CI that were disabled in PR #1646 by @pankajkoti in #1726 * Pre-commit updates: #1666, #1653, #1641, #1682, #1720 Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com> Co-authored-by: Pankaj Singh <98807258+pankajastro@users.noreply.github.com> --------- Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com>
3 participants
Description
The select wasn't taking into account the ".{id}" appended to detached schema tests like some other functions were, so the tests weren't actually running.
Log before fix:
Log after fix:
Related Issue(s)
Closes #1679
Breaking Change?
N/A
Checklist