jonathangosling commented on code in PR #71675:
URL: https://github.com/apache/airflow/pull/71675#discussion_r3792574378
##########
providers/google/tests/unit/google/cloud/transfers/test_gcs_to_bigquery.py:
##########
@@ -2124,6 +2131,110 @@ def test_schema_fields_is_templated(self):
assert operator.schema_fields == SCHEMA_FIELDS
+ @pytest.mark.parametrize(
+ ("operator_kwargs", "expects_warning"),
+ [
+ pytest.param({"ignore_unknown_values": True}, True,
id="autodetect_defaults_to_true"),
+ pytest.param({"ignore_unknown_values": True, "autodetect": True},
True, id="autodetect_true"),
+ pytest.param({"ignore_unknown_values": True, "autodetect": None},
False, id="autodetect_none"),
+ pytest.param(
+ {"ignore_unknown_values": True, "schema_fields":
SCHEMA_FIELDS},
+ False,
+ id="schema_fields_supplied",
+ ),
+ pytest.param({"ignore_unknown_values": False}, False,
id="ignore_unknown_values_off"),
+ pytest.param(
+ {"ignore_unknown_values": True, "extra_config": {"autodetect":
None}},
+ False,
+ id="autodetect_cleared_by_extra_config",
+ ),
+ pytest.param(
+ {"ignore_unknown_values": True, "extra_config": {"schema":
{"fields": SCHEMA_FIELDS}}},
+ False,
+ id="schema_supplied_by_extra_config",
+ ),
+ pytest.param(
+ {"ignore_unknown_values": False, "extra_config":
{"ignoreUnknownValues": True}},
+ True,
+ id="ignore_unknown_values_set_by_extra_config",
+ ),
+ ],
+ )
+ @mock.patch(GCS_TO_BQ_PATH.format("BigQueryHook"))
+ def test_ignore_unknown_values_no_op_warning(self, bq_hook,
operator_kwargs, expects_warning):
+ bq_hook.return_value.insert_job.side_effect = [
+ MagicMock(job_id=REAL_JOB_ID, error_result=False),
+ REAL_JOB_ID,
+ ]
+ bq_hook.return_value.generate_job_id.return_value = REAL_JOB_ID
+ bq_hook.return_value.split_tablename.return_value = (PROJECT_ID,
DATASET, TABLE)
+
+ operator = GCSToBigQueryOperator(
+ task_id=TASK_ID,
+ bucket=TEST_BUCKET,
+ source_objects=TEST_SOURCE_OBJECTS,
+ destination_project_dataset_table=TEST_EXPLICIT_DEST,
+ write_disposition=WRITE_DISPOSITION,
+ project_id=JOB_PROJECT_ID,
+ **operator_kwargs,
+ )
+
+ with mock.patch.object(operator.log, "warning") as mock_warning:
+ operator.execute(context=MagicMock())
+
+ assert
mock_warning.call_args_list.count(call(IGNORE_UNKNOWN_VALUES_WARNING)) == (
+ 1 if expects_warning else 0
+ )
Review Comment:
Yes, good suggestion. I've updated to use `assert_called_once()` /
`assert_not_called()`. On protecting the message, rather than pinning the full
text, one test (`test_ignore_unknown_values_no_op_warning_names_the_fix`) now
asserts only that the warning names `autodetect=None`, since that's the
actionable part.
One thing worth flagging - these assertions don't discriminate which warning
fired, so they'll break if an unrelated warning is added. Filtering
on/asserting against a stable part of the message (i.e.
`ignore_unknown_values` or `autodetect=None`), rather than the full message,
could avoid that. Let me know if you think it's worth it.
--
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]
