potiuk commented on pull request #20428: URL: https://github.com/apache/airflow/pull/20428#issuecomment-999033755
> For some reason, `has_calls` is used instead of `assert_has_calls`. I couldn't find any documentation of `has_calls`, ~but it seems to do the same thing as `assert_has_calls`, just that it returns a boolean instead of doing an assertion on its own.~ (**EDIT**: I'm wrong, it just seems to create a new mock ad-hoc..)This means the affected tests don't actually assert anything, making them pass although they shouldn't. UPS! > > There are a couple of other places where `has_calls` is used: > > ``` > (airflow-env) ➜ airflow git:(main) ✗ egrep -ir '[^_]has_calls' . > ./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) AH COOL. We should fix them. > I don't think I have the time to fix all these in a short time frame, I haven't looked at these parts at all.. No worries, I will create an issue for that and we will fix it. Also might make sense to ban using "has_calls" in our tests in this case! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
