You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/21 19:28:31 UTC
[GitHub] [airflow] potiuk opened a new issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
potiuk opened a new issue #20453:
URL: https://github.com/apache/airflow/issues/20453
### Body
As explained in https://github.com/apache/airflow/pull/20428#issuecomment-998714275 we seem to have number (not big) of tests that use "has_calls" rather than "assert_has_calls".
:scream: :scream: :scream: :scream: :scream: :scream: :scream: :scream: :scream:
What "has_calls" does is acually calling "has_calls" method on the mock :) . Which make them no-assertion tests:
:scream: :scream: :scream: :scream: :scream: :scream: :scream: :scream: :scream:
The list of those tests:
- [ ] ./tests/providers/google/common/hooks/test_base_google.py: mock_check_output.has_calls(
- [ ] ./tests/providers/google/common/hooks/test_base_google.py: mock_check_output.has_calls(
- [ ] ./tests/providers/google/cloud/transfers/test_sheets_to_gcs.py: mock_sheet_hook.return_value.get_values.has_calls(calls)
- [ ] ./tests/providers/google/cloud/transfers/test_sheets_to_gcs.py: mock_upload_data.has_calls(calls)
- [ ] ./tests/providers/google/cloud/hooks/test_bigquery.py: mock_poll_job_complete.has_calls(mock.call(running_job_id), mock.call(running_job_id))
- [ ] ./tests/providers/google/cloud/hooks/test_bigquery.py: mock_schema.has_calls([mock.call(x, "") for x in ["field_1", "field_2"]])
- [ ] ./tests/providers/google/cloud/hooks/test_bigquery.py: assert mock_insert.has_calls(
- [ ] ./tests/providers/google/cloud/hooks/test_pubsub.py: publish_method.has_calls(calls)
- [ ] ./tests/providers/google/cloud/hooks/test_cloud_memorystore.py: mock_get_conn.return_value.get_instance.has_calls(
- [ ] ./tests/providers/google/cloud/hooks/test_cloud_memorystore.py: mock_get_conn.return_value.get_instance.has_calls(
- [ ] ./tests/providers/google/cloud/hooks/test_cloud_memorystore.py: mock_get_conn.return_value.get_instance.has_calls(
- [ ] ./tests/providers/google/cloud/hooks/test_dataproc.py: mock_get_job.has_calls(calls)
- [ ] ./tests/providers/google/cloud/hooks/test_dataproc.py: mock_get_job.has_calls(calls)
- [ ] ./tests/providers/google/suite/operators/test_sheets.py: mock_xcom.has_calls(calls)
- [ ] ./tests/providers/http/operators/test_http.py: mock_info.has_calls(calls)
- [ ] ./tests/providers/airbyte/hooks/test_airbyte.py: assert mock_get_job.has_calls(calls)
- [ ] ./tests/providers/airbyte/hooks/test_airbyte.py: assert mock_get_job.has_calls(calls)
- [ ] ./tests/providers/airbyte/hooks/test_airbyte.py: assert mock_get_job.has_calls(calls)
- [ ] ./tests/providers/airbyte/hooks/test_airbyte.py: assert mock_get_job.has_calls(calls)
- [ ] ./tests/providers/airbyte/hooks/test_airbyte.py: assert mock_get_job.has_calls(calls)
We should fix those tests and likelly add pre-commit to ban this.
Thanks to @jobegrabber for noticing it!
### Committer
- [X] I acknowledge that I am a maintainer/committer of the Apache Airflow project.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] harishkrao commented on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
harishkrao commented on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1001723526
@potiuk just created a PR, can you please review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1008649769
Have you tried to run `breeze` to run the tests? https://github.com/apache/airflow/blob/main/BREEZE.rst
Airflow has docker-compose based development environment that is the mirror of what is used in CI. Essentially it should be as easy as:
```
./breeze
```
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):
```
pytest test/providers/google/...
```
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 ``airflow db reset`` and it should reset the db ``airflow db init`` will fill the db with default connections.
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):
```
airflow breeze --backend postgres --db-reset
```
or
```
airflow breeze --backend mysql --db-reset
```
or even:
```
airflow breeze --backend postgres --python 3.9 --db-reset
```
If you need to run tests with a different python version.
I hope it will be helpful
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] harishkrao commented on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
harishkrao commented on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1053618372
> > 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 `has_calls` - it might be more efficient
Good idea, thank you @potiuk! I will do that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-999620264
Feel free!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] harishkrao commented on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
harishkrao commented on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1078615559
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.
```
./tests/providers/airbyte/hooks/test_airbyte.py: assert mock_get_job.has_calls(calls)
./tests/providers/airbyte/hooks/test_airbyte.py: assert mock_get_job.has_calls(calls)
./tests/providers/airbyte/hooks/test_airbyte.py: assert mock_get_job.has_calls(calls)
./tests/providers/airbyte/hooks/test_airbyte.py: assert mock_get_job.has_calls(calls)
./tests/providers/airbyte/hooks/test_airbyte.py: assert mock_get_job.has_calls(calls)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] harishkrao commented on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
harishkrao commented on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1009625245
@potiuk That worked, thank you very much!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] harishkrao commented on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
harishkrao commented on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-999368922
Can I take this? @potiuk
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] harishkrao commented on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
harishkrao commented on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1053275745
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 `has_calls` listed above and then thinking of the best possible way to convert it to an assert-able test.
I will keep working on it and will submit a PR once I have covered all the tests listed above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1008649769
Have you tried to run `breeze` to run the tests? https://github.com/apache/airflow/blob/main/BREEZE.rst
Airflow has docker-compose based development environment that is the mirror of what is used in CI. Essentially it should be as easy as:
```
./breeze
```
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):
```
pytest test/providers/google/...
```
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 ``airflow db reset`` and it should reset the db, while ``airflow db init`` will fill the db with default connections.
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):
```
./breeze --backend postgres --db-reset
```
or
```
./breeze --backend mysql --db-reset
```
or even:
```
./breeze --backend postgres --python 3.9 --db-reset
```
If you need to run tests with a different python version.
I hope it will be helpful
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] harishkrao edited a comment on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
harishkrao edited a comment on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1008540709
@mik-laj / @potiuk - I am trying to run pytest on [google bigquery](https://github.com/apache/airflow/blob/main/tests/providers/google/cloud/hooks/test_bigquery.py) and I am running into an exception defined [here](https://github.com/apache/airflow/blob/main/airflow/models/connection.py#L397). 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] harishkrao commented on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
harishkrao commented on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1008540709
@mik-laj / @potiuk - I am trying to run pytest on [google bigquery](https://github.com/apache/airflow/blob/main/tests/providers/google/cloud/hooks/test_bigquery.py) and I am running into an exception defined [here](https://github.com/apache/airflow/blob/main/airflow/models/connection.py#L397). 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] harishkrao commented on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
harishkrao commented on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1009568661
@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!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1053526987
> 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 `has_calls` - it might be more efficient
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on issue #20453: The "has_calls" is used in place of "assert_has_calls" in a few places
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #20453:
URL: https://github.com/apache/airflow/issues/20453#issuecomment-1008649769
Have you tried to run `breeze` to run the tests? https://github.com/apache/airflow/blob/main/BREEZE.rst
Airflow has docker-compose based development environment that is the mirror of what is used in CI. Essentially it should be as easy as:
```
./breeze
```
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):
```
pytest test/providers/google/...
```
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 ``airflow db reset`` and it should reset the db, while ``airflow db init`` will fill the db with default connections.
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):
```
airflow breeze --backend postgres --db-reset
```
or
```
airflow breeze --backend mysql --db-reset
```
or even:
```
airflow breeze --backend postgres --python 3.9 --db-reset
```
If you need to run tests with a different python version.
I hope it will be helpful
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org