Copilot commented on code in PR #64879:
URL: https://github.com/apache/airflow/pull/64879#discussion_r3107908020
##########
providers/git/tests/unit/git/bundles/test_git.py:
##########
@@ -920,6 +932,111 @@ def
test_clone_bare_repo_invalid_repository_error_retry_fails(
# Verify Repo was called twice (failed attempt + failed retry)
assert mock_repo_class.call_count == 2
+ @mock.patch("airflow.providers.git.bundles.git.GitHook")
+ def test_fetch_submodules_uses_custom_environment_when_git_ssh_command_set(
+ self, mock_githook_class
+ ):
+ """Non-empty hook.env is passed to custom_environment; submodule runs
inside that context."""
+ expected_ssh_cmd = (
+ "ssh -i /path/key -o IdentitiesOnly=yes -o
UserKnownHostsFile=/path/known_hosts"
+ )
+ mock_hook = mock_githook_class.return_value
+ mock_hook.repo_url = "[email protected]:apache/airflow.git"
+ mock_hook.env = {"GIT_SSH_COMMAND": expected_ssh_cmd}
+
+ mock_git = mock.create_autospec(Git, instance=True)
+ ssh_ctx = mock.create_autospec(_GitCustomEnvContextManager,
instance=True)
+ mock_git.custom_environment.return_value = ssh_ctx
+ mock_repo = mock.MagicMock(spec=Repo)
+ mock_repo.git = mock_git
+
+ bundle = GitDagBundle(
+ name="test",
+ git_conn_id="git_default",
+ tracking_ref="main",
+ version="123456",
+ submodules=True,
+ )
+ bundle.repo = mock_repo
+
+ bundle._fetch_submodules()
+
+
mock_git.custom_environment.assert_called_once_with(GIT_SSH_COMMAND=expected_ssh_cmd)
+
+ parent = mock.Mock()
+ parent.attach_mock(mock_git.custom_environment, "custom_environment")
+ parent.attach_mock(ssh_ctx.__enter__, "enter")
+ parent.attach_mock(mock_git.submodule, "submodule")
+ parent.attach_mock(ssh_ctx.__exit__, "exit")
+
+ assert parent.mock_calls == [
+ mock.call.custom_environment(GIT_SSH_COMMAND=expected_ssh_cmd),
+ mock.call.enter(),
+ mock.call.submodule("sync", "--recursive"),
+ mock.call.submodule("update", "--init", "--recursive", "--jobs",
"1"),
+ mock.call.exit(None, None, None),
+ ]
Review Comment:
Add a blank line between these two test methods to match the surrounding
formatting (one blank line between methods inside the test class). As-is, the
next `@mock.patch` decorator starts immediately after the previous assertion
block.
```suggestion
]
```
##########
providers/git/tests/unit/git/bundles/test_git.py:
##########
@@ -920,6 +932,111 @@ def
test_clone_bare_repo_invalid_repository_error_retry_fails(
# Verify Repo was called twice (failed attempt + failed retry)
assert mock_repo_class.call_count == 2
+ @mock.patch("airflow.providers.git.bundles.git.GitHook")
+ def test_fetch_submodules_uses_custom_environment_when_git_ssh_command_set(
+ self, mock_githook_class
+ ):
+ """Non-empty hook.env is passed to custom_environment; submodule runs
inside that context."""
+ expected_ssh_cmd = (
+ "ssh -i /path/key -o IdentitiesOnly=yes -o
UserKnownHostsFile=/path/known_hosts"
+ )
+ mock_hook = mock_githook_class.return_value
+ mock_hook.repo_url = "[email protected]:apache/airflow.git"
+ mock_hook.env = {"GIT_SSH_COMMAND": expected_ssh_cmd}
+
+ mock_git = mock.create_autospec(Git, instance=True)
+ ssh_ctx = mock.create_autospec(_GitCustomEnvContextManager,
instance=True)
+ mock_git.custom_environment.return_value = ssh_ctx
+ mock_repo = mock.MagicMock(spec=Repo)
+ mock_repo.git = mock_git
+
+ bundle = GitDagBundle(
+ name="test",
+ git_conn_id="git_default",
+ tracking_ref="main",
+ version="123456",
+ submodules=True,
+ )
+ bundle.repo = mock_repo
+
+ bundle._fetch_submodules()
+
+
mock_git.custom_environment.assert_called_once_with(GIT_SSH_COMMAND=expected_ssh_cmd)
+
+ parent = mock.Mock()
+ parent.attach_mock(mock_git.custom_environment, "custom_environment")
+ parent.attach_mock(ssh_ctx.__enter__, "enter")
+ parent.attach_mock(mock_git.submodule, "submodule")
+ parent.attach_mock(ssh_ctx.__exit__, "exit")
+
+ assert parent.mock_calls == [
+ mock.call.custom_environment(GIT_SSH_COMMAND=expected_ssh_cmd),
+ mock.call.enter(),
+ mock.call.submodule("sync", "--recursive"),
+ mock.call.submodule("update", "--init", "--recursive", "--jobs",
"1"),
+ mock.call.exit(None, None, None),
+ ]
Review Comment:
`parent.attach_mock(...)` is done after `bundle._fetch_submodules()` has
already run, so `parent.mock_calls` will not include the earlier calls (calls
are only propagated to the parent at call time, when the child is already
attached). This makes the call-order assertion unreliable/likely to fail.
Attach the mocks to `parent` before invoking `_fetch_submodules()` (or assert
ordering using the individual mocks’ `mock_calls`/`method_calls` instead).
--
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]
