-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add integration test to run example DAGs #114
Conversation
"snowflake_animal_adoption_example", | ||
], | ||
) | ||
def test_example_dag(session, dag_id): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were planning to run this in a periodic manner like a once a day or so, but this looks like part of CI? is this fast enough for CI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@utkarsharma2 yes this runs fairly quickly. I'm gonna attempt to split up the CI soon which should speed up everything considerably.
|
||
@pytest.mark.parametrize( | ||
"dag_id", | ||
[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth listing all the dags in the examples folder (e.g. glob.glob("../../example_dags/*.py
) instead of having a hard-coded list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we might want to pick and choose instead of running all dags?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe all the DAGs in our examples folders should be functional - and my understanding is that this is the way we have to make sure they'll be working. My concern with having a hard-coded list here is that we may forget to update this test when new example DAGs are added - and they may stop working if we don't have an automatic way of validating them.
If we want to run specific tests for local development, we can always do that with pytest -k
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tatiana I have added all DAGs to the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tatiana I tried to make it grab all DAGs from the dagbag which would've allowed this to automatically pick up new dags but for some reason that caused the CI to infinitely loop. I suppose we can just make sure that any new dags get added to the list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a shame. Please, let me know if you'd like to pair on this at some point - with a rubber duck, we may be able to find a way that works.
It is not a blocker, though. You did great work which deserves to be in our main branch! 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it seems you solved it in this other PR:
https://github.com/astro-projects/astro/pull/121/files#diff-957bc078ac97ff310bb3e871f049db5ae29b4b7e48aa6337878088d02c7ef44dR17-R18 🎉
Thank you!
example_dags/demo_parse_directory/ingest_models/adoption_center_1.sql
Outdated
Show resolved
Hide resolved
4d503f7
to
16e44dc
Compare
16e44dc
to
7f2fa00
Compare
* add integration tests around example dags * Create an integration test that ensures that all example DAGs are runnable * get save file tests to pass * get save file tests to pass * render example now works purely within postgres * fix tests * pin deps
Resolves #100