Copilot commented on code in PR #64879:
URL: https://github.com/apache/airflow/pull/64879#discussion_r3123879043
##########
providers/celery/tests/unit/celery/executors/test_celery_executor.py:
##########
@@ -430,13 +434,22 @@ def test_send_workloads_to_celery_hang(register_signals):
executor = celery_executor.CeleryExecutor()
workload = MockWorkload()
- workload_tuples_to_send = [(None, None, None, workload) for _ in range(26)]
+ # ``WorkloadInCelery`` is (key, workload, queue, team_name); the 4th field
must be a
+ # str | None team name, not the workload (regression after team /
ExecutorConf support).
+ workload_tuples_to_send = [(None, workload, None, None) for _ in range(26)]
for _ in range(250):
# This loop can hang on Linux if celery_executor does something wrong
with
# multiprocessing.
results = executor._send_workloads_to_celery(workload_tuples_to_send)
- assert results == [(None, None, 1) for _ in workload_tuples_to_send]
+ assert len(results) == len(workload_tuples_to_send)
+ for key, sent_args, result in results:
+ assert key is None
+ if AIRFLOW_V_3_0_PLUS:
+ assert sent_args == (workload.model_dump_json(),)
+ else:
+ assert sent_args == [workload]
+ assert not isinstance(result,
celery_executor_utils.ExceptionWithTraceback)
Review Comment:
For the Airflow <3.0 branch (`AIRFLOW_V_3_0_PLUS` is False),
`send_workload_to_executor` ultimately calls Celery `apply_async` with
JSON-only serialization (`accept_content = ["json"]`). In that case, sending a
`MockWorkload` object as the workload/command payload (`(None, workload, None,
None)`) would not be JSON-serializable and would likely yield an
`ExceptionWithTraceback` result if this test is ever run on 2.x. Consider
making the non-3.0 path use a simple JSON-serializable command payload (e.g. a
list of strings) and adjust the assertion accordingly (or skip this test for
<3.0 if it’s not intended to run there).
##########
providers/git/tests/unit/git/bundles/test_git.py:
##########
@@ -46,6 +46,32 @@ def bundle_temp_dir(tmp_path):
GIT_DEFAULT_BRANCH = "main"
+
+class _GitCustomEnvContextManager:
+ """Minimal context manager surface for Git.custom_environment() (autospec
target)."""
+
+ def __enter__(self) -> None:
+ pass
+
+ def __exit__(self, exc_type, exc_val, exc_tb) -> None:
+ pass
+
+
+# ``_fetch_submodules`` only uses ``repo.git.custom_environment`` and
``repo.git.submodule``.
+# Use ``types.SimpleNamespace`` for ``bundle.repo`` / ``repo.git`` so ``.git``
/ ``.submodule``
+# are plain instance attributes — **not** MagicMock children (nested
``Repo``/``Git`` mocks
+# in unittest can lack ``submodule`` on lowest-dep / CI runs).
+
+
+def _repo_and_git_stubs_for_submodule_tests(ssh_ctx_cm):
+ """Return (repo_stub, git_cmd_stub) for ``GitDagBundle._fetch_submodules``
unit tests."""
+ git_cmd = types.SimpleNamespace(
+ custom_environment=mock.MagicMock(return_value=ssh_ctx_cm),
+ submodule=mock.MagicMock(),
+ )
Review Comment:
`_repo_and_git_stubs_for_submodule_tests` creates `MagicMock()` objects
without `spec`/`autospec` (`custom_environment` and `submodule`). In Airflow
tests we try to keep mocks spec’d so typos / API drift are caught. Consider
switching these to `create_autospec(...)` (e.g. autospec a small callable for
`submodule`, and a small callable returning the provided context manager for
`custom_environment`) or at least `Mock(spec_set=...)` for the minimal surface
used here.
--
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]
