mirror of
https://gerrit.googlesource.com/git-repo
synced 2026-11-13 07:47:23 +00:00
`Project.UploadForReview()` unconditionally passed `--progress` to `git push`, forcing client and remote progress meters (`Counting objects`, `remote: Resolving deltas: 3%..100%`) even on non-TTY stderr or when `-q`/`--quiet` was passed. Meanwhile, `repo upload` ignored `opt.quiet` completely. Move `--progress` injection for `push` into `GitCommand` alongside `fetch` and `clone`, enabling it only when `sys.stderr.isatty()` and none of `--progress`, `--no-progress`, `--quiet`, or `-q` are set. Forward `opt.quiet` from `repo upload` to `Project.UploadForReview()` and pass `--no-progress` to `git push` rather than `--quiet` so Gerrit `remote: https://...` review URLs are still emitted and parsed by `_FindGerritUrls()`. Also suppress the non-error `[OK]` summary banner when `-q`/`--quiet` is set. Bug: 578114287 Change-Id: I88b7b055dcca0db80dae6962886a693fe717c741 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/649663 Tested-by: Gavin Mak <gavinmak@google.com> Commit-Queue: Gavin Mak <gavinmak@google.com> Reviewed-by: Brian Gan <brgan@google.com>
333 lines
12 KiB
Python
333 lines
12 KiB
Python
# Copyright (C) 2023 The Android Open Source Project
|
|
#
|
|
# Licensed under the Apache License, Version 2.0 (the "License");
|
|
# you may not use this file except in compliance with the License.
|
|
# You may obtain a copy of the License at
|
|
#
|
|
# http://www.apache.org/licenses/LICENSE-2.0
|
|
#
|
|
# Unless required by applicable law or agreed to in writing, software
|
|
# distributed under the License is distributed on an "AS IS" BASIS,
|
|
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
|
# See the License for the specific language governing permissions and
|
|
# limitations under the License.
|
|
|
|
"""Unittests for the subcmds/upload.py module."""
|
|
|
|
from typing import List, Optional
|
|
from unittest import mock
|
|
|
|
import pytest
|
|
|
|
import error
|
|
from subcmds import upload
|
|
|
|
|
|
class UnexpectedError(Exception):
|
|
"""An exception not expected by upload command."""
|
|
|
|
|
|
# A stub people list (reviewers, cc).
|
|
_STUB_PEOPLE = ([], [])
|
|
|
|
|
|
@pytest.fixture
|
|
def cmd() -> upload.Upload:
|
|
"""Fixture to provide an Upload command instance with mocked methods."""
|
|
cmd = upload.Upload()
|
|
with mock.patch.object(
|
|
cmd, "_AppendAutoList", return_value=None
|
|
), mock.patch.object(cmd, "git_event_log"):
|
|
yield cmd
|
|
|
|
|
|
def test_UploadAndReport_UploadError(cmd: upload.Upload) -> None:
|
|
"""Check UploadExitError raised when UploadError encountered."""
|
|
opt, _ = cmd.OptionParser.parse_args([])
|
|
with mock.patch.object(
|
|
cmd, "_UploadBranch", side_effect=error.UploadError("")
|
|
):
|
|
with pytest.raises(upload.UploadExitError):
|
|
cmd._UploadAndReport(opt, [mock.MagicMock()], _STUB_PEOPLE)
|
|
|
|
|
|
def test_UploadAndReport_GitError(cmd: upload.Upload) -> None:
|
|
"""Check UploadExitError raised when GitError encountered."""
|
|
opt, _ = cmd.OptionParser.parse_args([])
|
|
with mock.patch.object(
|
|
cmd, "_UploadBranch", side_effect=error.GitError("")
|
|
):
|
|
with pytest.raises(upload.UploadExitError):
|
|
cmd._UploadAndReport(opt, [mock.MagicMock()], _STUB_PEOPLE)
|
|
|
|
|
|
def test_UploadAndReport_UnhandledError(cmd: upload.Upload) -> None:
|
|
"""Check UnexpectedError passed through."""
|
|
opt, _ = cmd.OptionParser.parse_args([])
|
|
with mock.patch.object(cmd, "_UploadBranch", side_effect=UnexpectedError):
|
|
with pytest.raises(UnexpectedError):
|
|
cmd._UploadAndReport(opt, [mock.MagicMock()], _STUB_PEOPLE)
|
|
|
|
|
|
def test_GetMergeBranch_explicit_branch(cmd: upload.Upload) -> None:
|
|
"""Verify _GetMergeBranch reads branch.merge for explicit local_branch."""
|
|
mock_project = mock.MagicMock()
|
|
mock_branch = mock.MagicMock()
|
|
mock_branch.merge = "refs/heads/main"
|
|
mock_project.GetBranch.return_value = mock_branch
|
|
|
|
res = cmd._GetMergeBranch(mock_project, local_branch="feature")
|
|
assert res == "refs/heads/main"
|
|
mock_project.GetBranch.assert_called_once_with("feature")
|
|
|
|
|
|
def test_GetMergeBranch_current_branch(cmd: upload.Upload) -> None:
|
|
"""Verify _GetMergeBranch falls back to project.CurrentBranch."""
|
|
mock_project = mock.MagicMock()
|
|
mock_project.CurrentBranch = "auto-cbr"
|
|
mock_branch = mock.MagicMock()
|
|
mock_branch.merge = "refs/heads/upstream-main"
|
|
mock_project.GetBranch.return_value = mock_branch
|
|
|
|
res = cmd._GetMergeBranch(mock_project, local_branch=None)
|
|
assert res == "refs/heads/upstream-main"
|
|
mock_project.GetBranch.assert_called_once_with("auto-cbr")
|
|
|
|
|
|
def test_GetMergeBranch_none_when_no_branch(cmd: upload.Upload) -> None:
|
|
"""Verify _GetMergeBranch returns empty string when detached HEAD."""
|
|
mock_project = mock.MagicMock()
|
|
mock_project.CurrentBranch = None
|
|
|
|
res = cmd._GetMergeBranch(mock_project, local_branch=None)
|
|
assert res == ""
|
|
|
|
|
|
def test_GatherOne_returns_resolved_current_branch(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
"""Upload error reporting reuses the branch gathered by the worker."""
|
|
project = mock.MagicMock()
|
|
project.CurrentBranch = "topic"
|
|
branch = mock.sentinel.branch
|
|
project.GetUploadableBranch.return_value = branch
|
|
monkeypatch.setattr(
|
|
upload.Upload,
|
|
"get_parallel_context",
|
|
lambda: {"projects": [project]},
|
|
)
|
|
opt = mock.MagicMock(current_branch=True)
|
|
|
|
assert upload.Upload._GatherOne(opt, 0) == (0, [branch], "topic")
|
|
|
|
project.GetUploadableBranch.assert_called_once_with("topic")
|
|
|
|
|
|
def _create_mock_branch(
|
|
name: str = "main",
|
|
commits: Optional[List[str]] = None,
|
|
project_relpath: str = "project-a",
|
|
) -> mock.MagicMock:
|
|
"""Helper to construct a mock ReviewableBranch."""
|
|
branch = mock.MagicMock()
|
|
branch.name = name
|
|
branch.commits = commits if commits is not None else ["commit1"]
|
|
|
|
project = mock.MagicMock()
|
|
project.RelPath.return_value = project_relpath
|
|
branch.project = project
|
|
return branch
|
|
|
|
|
|
def test_MultipleBranches_yes_with_current_branch_flag_bypasses_editor(
|
|
cmd: upload.Upload,
|
|
) -> None:
|
|
"""_MultipleBranches with --yes and -c flag bypasses editor."""
|
|
opt, _ = cmd.OptionParser.parse_args(["-c", "-y"])
|
|
branch1 = _create_mock_branch("b1", project_relpath="p1")
|
|
branch2 = _create_mock_branch("b2", project_relpath="p2")
|
|
pending = [(branch1.project, [branch1]), (branch2.project, [branch2])]
|
|
|
|
with mock.patch.object(cmd, "_UploadAndReport") as mock_upload, mock.patch(
|
|
"editor.Editor.EditString"
|
|
) as mock_edit:
|
|
cmd._MultipleBranches(opt, pending, _STUB_PEOPLE)
|
|
mock_edit.assert_not_called()
|
|
mock_upload.assert_called_once_with(
|
|
opt, [branch1, branch2], _STUB_PEOPLE
|
|
)
|
|
|
|
|
|
def test_MultipleBranches_yes_with_cbr_flag_bypasses_editor(
|
|
cmd: upload.Upload,
|
|
) -> None:
|
|
"""_MultipleBranches with --yes and --cbr flag bypasses editor."""
|
|
opt, _ = cmd.OptionParser.parse_args(["--cbr", "--yes"])
|
|
branch1 = _create_mock_branch("b1", project_relpath="p1")
|
|
branch2 = _create_mock_branch("b2", project_relpath="p2")
|
|
pending = [(branch1.project, [branch1]), (branch2.project, [branch2])]
|
|
|
|
with mock.patch.object(cmd, "_UploadAndReport") as mock_upload, mock.patch(
|
|
"editor.Editor.EditString"
|
|
) as mock_edit:
|
|
cmd._MultipleBranches(opt, pending, _STUB_PEOPLE)
|
|
mock_edit.assert_not_called()
|
|
mock_upload.assert_called_once_with(
|
|
opt, [branch1, branch2], _STUB_PEOPLE
|
|
)
|
|
|
|
|
|
def test_MultipleBranches_yes_with_branch_flag_bypasses_editor(
|
|
cmd: upload.Upload,
|
|
) -> None:
|
|
"""_MultipleBranches with --yes and --br flag bypasses editor."""
|
|
opt, _ = cmd.OptionParser.parse_args(["--br", "feature", "-y"])
|
|
branch1 = _create_mock_branch("feature", project_relpath="p1")
|
|
branch2 = _create_mock_branch("feature", project_relpath="p2")
|
|
pending = [(branch1.project, [branch1]), (branch2.project, [branch2])]
|
|
|
|
with mock.patch.object(cmd, "_UploadAndReport") as mock_upload, mock.patch(
|
|
"editor.Editor.EditString"
|
|
) as mock_edit:
|
|
cmd._MultipleBranches(opt, pending, _STUB_PEOPLE)
|
|
mock_edit.assert_not_called()
|
|
mock_upload.assert_called_once_with(
|
|
opt, [branch1, branch2], _STUB_PEOPLE
|
|
)
|
|
|
|
|
|
def test_MultipleBranches_yes_with_branch_flag_empty_pending_dies(
|
|
cmd: upload.Upload,
|
|
) -> None:
|
|
"""_MultipleBranches with --yes and empty pending branches dies."""
|
|
opt, _ = cmd.OptionParser.parse_args(["--br", "feature", "-y"])
|
|
mock_project = mock.MagicMock()
|
|
pending = [(mock_project, [])]
|
|
|
|
with pytest.raises(
|
|
upload.UploadExitError, match="nothing ready for upload"
|
|
):
|
|
cmd._MultipleBranches(opt, pending, _STUB_PEOPLE)
|
|
|
|
|
|
def test_MultipleBranches_yes_without_branch_or_cbr_uses_editor(
|
|
cmd: upload.Upload,
|
|
) -> None:
|
|
"""_MultipleBranches with -y but no -c/--br falls back to editor."""
|
|
opt, _ = cmd.OptionParser.parse_args(["-y"])
|
|
branch1 = _create_mock_branch("b1", project_relpath="p1")
|
|
branch1.date = "2026-08-26"
|
|
mock_remote = mock.MagicMock()
|
|
branch1.project.dest_branch = None
|
|
branch1.project.revisionExpr = "refs/heads/main"
|
|
branch_config = mock.MagicMock()
|
|
branch_config.remote = mock_remote
|
|
branch1.project.GetBranch.return_value = branch_config
|
|
pending = [(branch1.project, [branch1])]
|
|
|
|
edited_script = (
|
|
"project p1/:\n"
|
|
" branch b1 ( 1 commit, 2026-08-26) to remote branch "
|
|
"refs/heads/main:\n"
|
|
)
|
|
with mock.patch.object(cmd, "_UploadAndReport") as mock_upload, mock.patch(
|
|
"editor.Editor.EditString", return_value=edited_script
|
|
) as mock_edit:
|
|
cmd._MultipleBranches(opt, pending, _STUB_PEOPLE)
|
|
mock_edit.assert_called_once()
|
|
mock_upload.assert_called_once_with(opt, [branch1], _STUB_PEOPLE)
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"argv, expected",
|
|
(
|
|
(["--fix", "--ignore-hooks"], "not be part of the upload"),
|
|
(["--fix", "--no-verify"], "--fix would have no effect"),
|
|
(
|
|
["--fix", "--ignore-hooks", "--no-verify"],
|
|
"--fix and --ignore-hooks are mutually exclusive",
|
|
),
|
|
),
|
|
)
|
|
def test_ValidateOptions_fix_conflicts(
|
|
cmd: upload.Upload,
|
|
capsys: pytest.CaptureFixture,
|
|
argv: List[str],
|
|
expected: str,
|
|
) -> None:
|
|
"""Check --fix is rejected with --ignore-hooks or --no-verify."""
|
|
opt, args = cmd.OptionParser.parse_args(argv)
|
|
with pytest.raises(SystemExit):
|
|
cmd.ValidateOptions(opt, args)
|
|
assert expected in capsys.readouterr().err
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"argv",
|
|
(
|
|
[],
|
|
["--fix"],
|
|
["--fix", "--verify"],
|
|
["--fix", "-y"],
|
|
["--ignore-hooks"],
|
|
["--no-verify"],
|
|
["--ignore-hooks", "--no-verify"],
|
|
),
|
|
)
|
|
def test_ValidateOptions_allowed(cmd: upload.Upload, argv: List[str]) -> None:
|
|
"""Check valid hook option combinations are accepted."""
|
|
opt, args = cmd.OptionParser.parse_args(argv)
|
|
cmd.ValidateOptions(opt, args)
|
|
|
|
|
|
@pytest.mark.parametrize("argv,expected_quiet", [([], False), (["-q"], True)])
|
|
def test_UploadBranch_forwards_quiet(
|
|
cmd: upload.Upload, argv: List[str], expected_quiet: bool
|
|
) -> None:
|
|
"""Check _UploadBranch forwards opt.quiet to branch.UploadForReview."""
|
|
opt, _ = cmd.OptionParser.parse_args(argv)
|
|
cmd.CommonValidateOptions(opt, [])
|
|
branch = mock.MagicMock()
|
|
branch.project.dest_branch = None
|
|
with mock.patch("git_superproject.UseSuperproject", return_value=False):
|
|
cmd._UploadBranch(opt, branch, _STUB_PEOPLE)
|
|
|
|
branch.UploadForReview.assert_called_once()
|
|
_, kwargs = branch.UploadForReview.call_args
|
|
assert kwargs["quiet"] is expected_quiet
|
|
|
|
|
|
def test_UploadAndReport_quiet_suppresses_ok_summary(
|
|
cmd: upload.Upload, capsys: pytest.CaptureFixture
|
|
) -> None:
|
|
"""Check -q suppresses non-error [OK] summary output."""
|
|
opt, _ = cmd.OptionParser.parse_args(["-q"])
|
|
cmd.CommonValidateOptions(opt, [])
|
|
branch = mock.MagicMock()
|
|
branch.name = "topic"
|
|
branch.project.RelPath.return_value = "proj"
|
|
with mock.patch.object(cmd, "_UploadBranch"):
|
|
cmd._UploadAndReport(opt, [branch], _STUB_PEOPLE)
|
|
|
|
assert capsys.readouterr().err == ""
|
|
|
|
|
|
def test_UploadAndReport_quiet_still_prints_errors(
|
|
cmd: upload.Upload, capsys: pytest.CaptureFixture
|
|
) -> None:
|
|
"""Check -q still prints [FAILED] error summary when upload fails."""
|
|
opt, _ = cmd.OptionParser.parse_args(["-q"])
|
|
cmd.CommonValidateOptions(opt, [])
|
|
branch = mock.MagicMock()
|
|
branch.name = "topic"
|
|
branch.project.RelPath.return_value = "proj"
|
|
with mock.patch.object(
|
|
cmd, "_UploadBranch", side_effect=error.UploadError("boom")
|
|
):
|
|
with pytest.raises(upload.UploadExitError):
|
|
cmd._UploadAndReport(opt, [branch], _STUB_PEOPLE)
|
|
|
|
err = capsys.readouterr().err
|
|
assert "[FAILED]" in err
|
|
assert "[OK ]" not in err
|