potiuk commented on code in PR #70574:
URL: https://github.com/apache/airflow/pull/70574#discussion_r3681899747
##########
providers/microsoft/azure/tests/unit/microsoft/azure/transfers/test_gcs_to_wasb.py:
##########
@@ -49,14 +49,17 @@ def test_init_defaults(self):
assert op.create_container is False
@mock.patch("airflow.providers.google.__version__", "10.2.0")
- def test_match_glob_requires_recent_google_provider(self):
+
@mock.patch("airflow.providers.microsoft.azure.transfers.gcs_to_wasb.WasbHook")
+
@mock.patch("airflow.providers.microsoft.azure.transfers.gcs_to_wasb.GCSHook")
+ def test_match_glob_requires_recent_google_provider(self, mock_gcs_hook,
mock_wasb_hook):
+ op = GCSToAzureBlobStorageOperator(
+ task_id=TASK_ID,
+ gcs_bucket=GCS_BUCKET,
+ container_name=CONTAINER,
+ match_glob="**/*.csv",
+ )
with pytest.raises(ValueError, match="match_glob"):
- GCSToAzureBlobStorageOperator(
- task_id=TASK_ID,
- gcs_bucket=GCS_BUCKET,
- container_name=CONTAINER,
- match_glob="**/*.csv",
- )
+ op.execute(context=None)
Review Comment:
`context=None` works only because the raise happens before anything touches
the context. It's a bit fragile as documentation of intent — if the guard later
moves below something that reads `context`, this fails with an unrelated
`AttributeError` rather than the assertion you meant.
`mock.MagicMock()` (as #70529 uses for the same situation) or `{}` would be
more robust. Also worth adding
`mock_gcs_hook.return_value.list.assert_not_called()` — #70723's equivalent
test does that, and it's what actually proves the operator bailed out before
doing any work rather than just raising somewhere.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
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]
