diff --git a/subcmds/upload.py b/subcmds/upload.py index 2215f9e84..8b5ae96e2 100644 --- a/subcmds/upload.py +++ b/subcmds/upload.py @@ -742,6 +742,22 @@ Gerrit Code Review: https://www.gerritcodereview.com/ avail = project.GetUploadableBranches(opt.branch) return (project_idx, avail, cbr) + def ValidateOptions(self, opt: optparse.Values, args: List[str]) -> None: + if opt.fix: + if opt.ignore_hooks: + self.OptionParser.error( + "--fix and --ignore-hooks are mutually exclusive options: " + "with --ignore-hooks, the original commits are uploaded, " + "so fixups would only modify the working tree and would " + "not be part of the upload" + ) + if opt.bypass_hooks: + self.OptionParser.error( + "--fix and --no-verify are mutually exclusive options: " + "--no-verify skips the pre-upload hook, so --fix would " + "have no effect" + ) + def Execute(self, opt, args): projects = self.GetProjects( args, all_manifests=not opt.this_manifest_only diff --git a/tests/test_subcmds_upload.py b/tests/test_subcmds_upload.py index 53666fe20..980520baf 100644 --- a/tests/test_subcmds_upload.py +++ b/tests/test_subcmds_upload.py @@ -233,3 +233,45 @@ def test_MultipleBranches_yes_without_branch_or_cbr_uses_editor( 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)