Copilot commented on code in PR #65543:
URL: https://github.com/apache/airflow/pull/65543#discussion_r3112355970
##########
airflow-core/tests/unit/dag_processing/test_manager.py:
##########
@@ -1543,6 +1543,80 @@ def test_callback_queue(self, mock_get_logger,
configure_testing_dag_bundle):
assert dag1_path not in manager._callback_to_execute
assert dag2_path not in manager._callback_to_execute
+ @mock.patch("airflow.dag_processing.manager.DagBundlesManager")
+ def test_resolve_callback_bundle(self, mock_bundle_manager):
+ manager = DagFileProcessorManager(max_runs=1)
+ bundle = MagicMock()
+ mock_bundle_manager.return_value.get_bundle.return_value = bundle
+
+ request = DagCallbackRequest(
+ filepath="file1.py",
+ dag_id="dag1",
+ run_id="run1",
+ is_failure_callback=False,
+ bundle_name="testing",
+ bundle_version="some_commit_hash",
+ msg=None,
+ )
+
+ assert manager.resolve_callback_bundle(request) is bundle
+ mock_bundle_manager.return_value.get_bundle.assert_called_once_with(
+ name="testing", version="some_commit_hash"
+ )
+
+ @mock.patch("airflow.dag_processing.manager.DagBundlesManager")
+ def
test_resolve_callback_bundle_returns_none_for_unconfigured_bundle(self,
mock_bundle_manager):
+ manager = DagFileProcessorManager(max_runs=1)
+ mock_bundle_manager.return_value.get_bundle.side_effect =
ValueError("missing bundle")
+
+ request = DagCallbackRequest(
+ filepath="file1.py",
+ dag_id="dag1",
+ run_id="run1",
+ is_failure_callback=False,
+ bundle_name="testing",
+ bundle_version="some_commit_hash",
+ msg=None,
+ )
+
+ assert manager.resolve_callback_bundle(request) is None
+
+ def test_initialize_callback_bundle_skips_unversioned_callback(self):
+ manager = DagFileProcessorManager(max_runs=1)
+ bundle = MagicMock()
+ bundle.supports_versioning = True
+
+ request = DagCallbackRequest(
+ filepath="file1.py",
+ dag_id="dag1",
+ run_id="run1",
+ is_failure_callback=False,
+ bundle_name="testing",
+ bundle_version=None,
+ msg=None,
+ )
+
+ assert manager.initialize_callback_bundle(request, bundle) is True
+ bundle.initialize.assert_not_called()
+
+ def test_initialize_callback_bundle_skips_non_versioned_bundle(self):
+ manager = DagFileProcessorManager(max_runs=1)
+ bundle = MagicMock()
+ bundle.supports_versioning = False
Review Comment:
New tests create `MagicMock()` instances without `spec`/`autospec`, which
can hide attribute errors and make tests less meaningful. Prefer
`MagicMock(spec=...)` so the mock reflects the real bundle interface used by
`initialize_callback_bundle`.
##########
airflow-core/tests/unit/dag_processing/test_manager.py:
##########
@@ -1543,6 +1543,80 @@ def test_callback_queue(self, mock_get_logger,
configure_testing_dag_bundle):
assert dag1_path not in manager._callback_to_execute
assert dag2_path not in manager._callback_to_execute
+ @mock.patch("airflow.dag_processing.manager.DagBundlesManager")
+ def test_resolve_callback_bundle(self, mock_bundle_manager):
+ manager = DagFileProcessorManager(max_runs=1)
+ bundle = MagicMock()
+ mock_bundle_manager.return_value.get_bundle.return_value = bundle
Review Comment:
New tests create `MagicMock()` instances without `spec`/`autospec`, which
can hide attribute errors and make tests less meaningful. Prefer
`MagicMock(spec=...)` (or use `autospec=True` on patch) so the mocks match the
real bundle interface.
##########
airflow-core/tests/unit/dag_processing/test_manager.py:
##########
@@ -1543,6 +1543,80 @@ def test_callback_queue(self, mock_get_logger,
configure_testing_dag_bundle):
assert dag1_path not in manager._callback_to_execute
assert dag2_path not in manager._callback_to_execute
+ @mock.patch("airflow.dag_processing.manager.DagBundlesManager")
+ def test_resolve_callback_bundle(self, mock_bundle_manager):
+ manager = DagFileProcessorManager(max_runs=1)
+ bundle = MagicMock()
+ mock_bundle_manager.return_value.get_bundle.return_value = bundle
+
+ request = DagCallbackRequest(
+ filepath="file1.py",
+ dag_id="dag1",
+ run_id="run1",
+ is_failure_callback=False,
+ bundle_name="testing",
+ bundle_version="some_commit_hash",
+ msg=None,
+ )
+
+ assert manager.resolve_callback_bundle(request) is bundle
+ mock_bundle_manager.return_value.get_bundle.assert_called_once_with(
+ name="testing", version="some_commit_hash"
+ )
+
+ @mock.patch("airflow.dag_processing.manager.DagBundlesManager")
+ def
test_resolve_callback_bundle_returns_none_for_unconfigured_bundle(self,
mock_bundle_manager):
+ manager = DagFileProcessorManager(max_runs=1)
+ mock_bundle_manager.return_value.get_bundle.side_effect =
ValueError("missing bundle")
+
+ request = DagCallbackRequest(
+ filepath="file1.py",
+ dag_id="dag1",
+ run_id="run1",
+ is_failure_callback=False,
+ bundle_name="testing",
+ bundle_version="some_commit_hash",
+ msg=None,
+ )
+
+ assert manager.resolve_callback_bundle(request) is None
+
+ def test_initialize_callback_bundle_skips_unversioned_callback(self):
+ manager = DagFileProcessorManager(max_runs=1)
+ bundle = MagicMock()
+ bundle.supports_versioning = True
Review Comment:
New tests create `MagicMock()` instances without `spec`/`autospec`, which
can hide attribute errors and make tests less meaningful. Prefer
`MagicMock(spec=...)` so the mock matches `BaseDagBundle` (or the concrete
bundle type) and fails fast on invalid attribute access.
##########
airflow-core/tests/unit/dag_processing/test_manager.py:
##########
@@ -1565,6 +1639,25 @@ def test_add_callback_initializes_versioned_bundle(self,
mock_bundle_manager):
bundle.initialize.assert_called_once()
+ def
test_initialize_callback_bundle_returns_false_when_initialization_fails(self):
+ manager = DagFileProcessorManager(max_runs=1)
+ bundle = MagicMock()
+ bundle.supports_versioning = True
+ bundle.initialize.side_effect = Exception("clone failed")
+
Review Comment:
New tests create `MagicMock()` instances without `spec`/`autospec`, which
can hide attribute errors and make tests less meaningful. Prefer
`MagicMock(spec=...)` (or `create_autospec`) so `initialize_callback_bundle`
interactions are validated against the real bundle API.
--
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]
