results: allow fixups on warnings; hooks: alint: pass warning=True for exit code 6 Modify ProjectResults.fixups to yield results even if they are non-blocking warnings, as long as they have a fixup command. Update alint hook to map exit code 6 (findings with fixes) to 77 (non-blocking warning). This allows alint findings to be reported as warnings while still offering the fixup command to the user. Bug: 494210418 Test: rh/results_unittest.py Test: rh/hooks_unittest.py Change-Id: Ibb395c8a618f8b75138152141fb92adf45544733 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repohooks/+/566241 Reviewed-by: Mike Frysinger <vapier@google.com> Tested-by: Marty Heavey <mheavey@google.com> Commit-Queue: Marty Heavey <mheavey@google.com>
diff --git a/rh/hooks.py b/rh/hooks.py index faea03d..3ab137d 100644 --- a/rh/hooks.py +++ b/rh/hooks.py
@@ -1309,9 +1309,16 @@ else None ) + # If alint returns 6, it means there are findings with fixes, but it + # should be non-blocking (warning). return [ rh.results.HookCommandResult( - "alint", project, commit, result, fixup_cmd=fixup_cmd + "alint", + project, + commit, + result, + fixup_cmd=fixup_cmd, + warning=result.returncode == 6, ) ]
diff --git a/rh/hooks_unittest.py b/rh/hooks_unittest.py index 0ab5c4e..60a9d53 100755 --- a/rh/hooks_unittest.py +++ b/rh/hooks_unittest.py
@@ -1219,7 +1219,7 @@ self.assertEqual( ret[0].fixup_cmd, ["alint", "fix", "--no_amend", "--commit", commit] ) - self.assertFalse(ret[0].is_warning()) + self.assertTrue(ret[0].is_warning()) self.assertEqual(ret[0].result.returncode, 6) # Test warning without fix.
diff --git a/rh/results.py b/rh/results.py index f1d1688..0084377 100644 --- a/rh/results.py +++ b/rh/results.py
@@ -61,7 +61,7 @@ def __bool__(self): """Whether this result is an error.""" - return bool(self.error) and not self._warning + return bool(self.error) and not self.is_warning() def is_warning(self): """Whether this result is a non-fatal warning.""" @@ -71,13 +71,23 @@ class HookCommandResult(HookResult): """A single hook result based on a CompletedProcess.""" - def __init__(self, hook, project, commit, result, files=(), fixup_cmd=None): + def __init__( + self, + hook, + project, + commit, + result, + warning: bool = False, + files=(), + fixup_cmd: Optional[List[str]] = None, + ): HookResult.__init__( self, hook, project, commit, result.stderr if result.stderr else result.stdout, + warning=warning, files=files, fixup_cmd=fixup_cmd, ) @@ -85,11 +95,11 @@ def __bool__(self): """Whether this result is an error.""" - return self.result.returncode not in (None, 0, 77) + return not self.is_warning() and self.result.returncode not in (None, 0) def is_warning(self): """Whether this result is a non-fatal warning.""" - return self.result.returncode == 77 + return self._warning or self.result.returncode == 77 class ProjectResults(NamedTuple): @@ -113,7 +123,9 @@ @property def fixups(self): """Yield results that have a fixup available.""" - yield from (x for x in self.results if x and x.fixup_cmd) + yield from ( + x for x in self.results if (x or x.is_warning()) and x.fixup_cmd + ) def __bool__(self): """Whether there are any errors in this set of results."""
diff --git a/rh/results_unittest.py b/rh/results_unittest.py index 9699e47..146747c 100755 --- a/rh/results_unittest.py +++ b/rh/results_unittest.py
@@ -79,6 +79,13 @@ self.assertFalse(result) self.assertTrue(result.is_warning()) + # A warning via kwarg. + result = rh.results.HookCommandResult( + "hook", "project", "HEAD", COMPLETED_PROCESS_FAIL, warning=True + ) + self.assertFalse(result) + self.assertTrue(result.is_warning()) + class ProjectResultsTests(unittest.TestCase): """Verify behavior of ProjectResults object.""" @@ -120,6 +127,35 @@ self.assertEqual(len(result1.results), 1) self.assertEqual(len(result2.results), 0) + def test_fixups(self): + """Check fixups handling.""" + result = rh.results.ProjectResults("project", "workdir", []) + + # A warning with a fixup. + warn_fix = rh.results.HookCommandResult( + "hook", "project", "HEAD", COMPLETED_PROCESS_WARN, fixup_cmd=["fix"] + ) + # An error with a fixup. + err_fix = rh.results.HookCommandResult( + "hook", "project", "HEAD", COMPLETED_PROCESS_FAIL, fixup_cmd=["fix"] + ) + # A warning WITHOUT a fixup. + warn_no_fix = rh.results.HookCommandResult( + "hook", "project", "HEAD", COMPLETED_PROCESS_WARN + ) + # An error WITHOUT a fixup. + err_no_fix = rh.results.HookCommandResult( + "hook", "project", "HEAD", COMPLETED_PROCESS_FAIL + ) + + result.add_results([warn_fix, err_fix, warn_no_fix, err_no_fix]) + + fixups = list(result.fixups) + self.assertIn(warn_fix, fixups) + self.assertIn(err_fix, fixups) + self.assertNotIn(warn_no_fix, fixups) + self.assertNotIn(err_no_fix, fixups) + if __name__ == "__main__": unittest.main()