-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
The "has_calls" is used in place of "assert_has_calls" in a few places #20453
Comments
Can I take this? @potiuk |
Feel free! |
@potiuk just created a PR, can you please review? |
@mik-laj / @potiuk - I am trying to run pytest on google bigquery and I am running into an exception defined here. I believe I need some kind of basic GCP/bigquery connection setup, but I am unable to find documentation about it. Trying to debug on my side but since you have extensive knowledge of the code base, I wanted to check with you to see if I am missing some basic config before running the tests. Update: I tried creating a connection on the Airflow UI and set environment variables locally before running pytest, but to no avail so far. |
Have you tried to run Airflow has docker-compose based development environment that is the mirror of what is used in CI. Essentially it should be as easy as:
This should drop you (after a while of getting images and starting docker compose) into ./breeze shell (which is bash inside docler-compose airlfow image). Then immediately you should be able to run (you can auto-complete the tests with TAB):
and it should run the tests. The first time it will initialize the db and create all the necessary connections, Also while you are in the breeze shell you can run By default it will use sqlite database, but you can also do (--db-reset is optional - the first ime you run it, the database should be "fresh", --db-reset is needed if you need to reset the DB from scratch):
or
or even:
If you need to run tests with a different python version. I hope it will be helpful |
@potiuk Thank you for taking the time to respond. I tried some of the steps you mentioned above before posting here. I will give a fresh try. I had a suspicion that it might have something to do with my setup. Really appreciate spending time to reply! |
@potiuk That worked, thank you very much! |
I am working on this issue and wanted to post this message to let you all know of the progress. This issue touches multiple providers and multiple test files within each provider. I am currently working on each of the I will keep working on it and will submit a PR once I have covered all the tests listed above. |
Or simply provide a series of PRs handling smaller subset of the |
Good idea, thank you @potiuk! I will do that. |
Of the open issues, only the airbyte provider issues remain to be fixed. I am working on them and will submit a separate PR soon.
|
Cool ! Fantastic that you are following this one up! |
airbyte tests are fixed in #22951 . There are still below instances on main. On fixing the assert calls it seems the tests are already broken.
|
I am checking on these tests. |
Hi @tirkarthi, I was planning on fixing these |
I guess there are still tests with
|
@potiuk, with the latest PR from @tirkarthi, there are no more pending items on this issue. We can close this issue. |
Body
As explained in #20428 (comment) we seem to have number (not big) of tests that use "has_calls" rather than "assert_has_calls".
😱 😱 😱 😱 😱 😱 😱 😱 😱
What "has_calls" does is acually calling "has_calls" method on the mock :) . Which make them no-assertion tests:
😱 😱 😱 😱 😱 😱 😱 😱 😱
The list of those tests:
We should fix those tests and likelly add pre-commit to ban this.
Thanks to @jobegrabber for noticing it!
Committer
The text was updated successfully, but these errors were encountered: