diff --git a/error.py b/error.py index 3cf34d54..ed4a90b7 100644 --- a/error.py +++ b/error.py @@ -12,8 +12,51 @@ # See the License for the specific language governing permissions and # limitations under the License. +from typing import List -class ManifestParseError(Exception): + +class BaseRepoError(Exception): + """All repo specific exceptions derive from BaseRepoError.""" + + +class RepoError(BaseRepoError): + """Exceptions thrown inside repo that can be handled.""" + + def __init__(self, *args, project: str = None) -> None: + super().__init__(*args) + self.project = project + + +class RepoExitError(BaseRepoError): + """Exception thrown that result in termination of repo program. + - Should only be handled in main.py + """ + + def __init__( + self, + *args, + exit_code: int = 1, + aggregate_errors: List[Exception] = None, + **kwargs, + ) -> None: + super().__init__(*args, **kwargs) + self.exit_code = exit_code + self.aggregate_errors = aggregate_errors + + +class RepoUnhandledExceptionError(RepoExitError): + """Exception that maintains error as reason for program exit.""" + + def __init__( + self, + error: BaseException, + **kwargs, + ) -> None: + super().__init__(error, **kwargs) + self.error = error + + +class ManifestParseError(RepoExitError): """Failed to parse the manifest file.""" @@ -25,11 +68,11 @@ class ManifestInvalidPathError(ManifestParseError): """A path used in or is incorrect.""" -class NoManifestException(Exception): +class NoManifestException(RepoExitError): """The required manifest does not exist.""" - def __init__(self, path, reason): - super().__init__(path, reason) + def __init__(self, path, reason, **kwargs): + super().__init__(path, reason, **kwargs) self.path = path self.reason = reason @@ -37,55 +80,64 @@ class NoManifestException(Exception): return self.reason -class EditorError(Exception): +class EditorError(RepoError): """Unspecified error from the user's text editor.""" - def __init__(self, reason): - super().__init__(reason) + def __init__(self, reason, **kwargs): + super().__init__(reason, **kwargs) self.reason = reason def __str__(self): return self.reason -class GitError(Exception): - """Unspecified internal error from git.""" +class GitError(RepoError): + """Unspecified git related error.""" - def __init__(self, command): - super().__init__(command) - self.command = command + def __init__(self, message, command_args=None, **kwargs): + super().__init__(message, **kwargs) + self.message = message + self.command_args = command_args def __str__(self): - return self.command + return self.message -class UploadError(Exception): +class UploadError(RepoError): """A bundle upload to Gerrit did not succeed.""" - def __init__(self, reason): - super().__init__(reason) + def __init__(self, reason, **kwargs): + super().__init__(reason, **kwargs) self.reason = reason def __str__(self): return self.reason -class DownloadError(Exception): +class DownloadError(RepoExitError): """Cannot download a repository.""" - def __init__(self, reason): - super().__init__(reason) + def __init__(self, reason, **kwargs): + super().__init__(reason, **kwargs) self.reason = reason def __str__(self): return self.reason -class NoSuchProjectError(Exception): +class SyncError(RepoExitError): + """Cannot sync repo.""" + + +class UpdateManifestError(RepoExitError): + """Cannot update manifest.""" + + +class NoSuchProjectError(RepoExitError): """A specified project does not exist in the work tree.""" - def __init__(self, name=None): - super().__init__(name) + def __init__(self, name=None, **kwargs): + super().__init__(**kwargs) self.name = name def __str__(self): @@ -94,11 +146,11 @@ class NoSuchProjectError(Exception): return self.name -class InvalidProjectGroupsError(Exception): +class InvalidProjectGroupsError(RepoExitError): """A specified project is not suitable for the specified groups""" - def __init__(self, name=None): - super().__init__(name) + def __init__(self, name=None, **kwargs): + super().__init__(**kwargs) self.name = name def __str__(self): @@ -107,7 +159,7 @@ class InvalidProjectGroupsError(Exception): return self.name -class RepoChangedException(Exception): +class RepoChangedException(BaseRepoError): """Thrown if 'repo sync' results in repo updating its internal repo or manifest repositories. In this special case we must use exec to re-execute repo with the new code and manifest. @@ -118,7 +170,7 @@ class RepoChangedException(Exception): self.extra_args = extra_args or [] -class HookError(Exception): +class HookError(RepoError): """Thrown if a 'repo-hook' could not be run. The common case is that the file wasn't present when we tried to run it. diff --git a/git_command.py b/git_command.py index c7245ade..588a64fd 100644 --- a/git_command.py +++ b/git_command.py @@ -40,6 +40,10 @@ GIT_DIR = "GIT_DIR" LAST_GITDIR = None LAST_CWD = None +DEFAULT_GIT_FAIL_MESSAGE = "git command failure" +# Common line length limit +GIT_ERROR_STDOUT_LINES = 1 +GIT_ERROR_STDERR_LINES = 1 class _GitCall(object): @@ -237,6 +241,7 @@ class GitCommand(object): cwd=None, gitdir=None, objdir=None, + verify_command=False, ): if project: if not cwd: @@ -244,6 +249,10 @@ class GitCommand(object): if not gitdir: gitdir = project.gitdir + self.project = project + self.cmdv = cmdv + self.verify_command = verify_command + # Git on Windows wants its paths only using / for reliability. if platform_utils.isWindows(): if objdir: @@ -332,7 +341,11 @@ class GitCommand(object): stderr=stderr, ) except Exception as e: - raise GitError("%s: %s" % (command[1], e)) + raise GitCommandError( + message="%s: %s" % (command[1], e), + project=project.name if project else None, + command_args=cmdv, + ) if ssh_proxy: ssh_proxy.add_client(p) @@ -366,4 +379,61 @@ class GitCommand(object): return env def Wait(self): - return self.rc + if not self.verify_command or self.rc == 0: + return self.rc + + stdout = ( + "\n".join(self.stdout.split("\n")[:GIT_ERROR_STDOUT_LINES]) + if self.stdout + else None + ) + + stderr = ( + "\n".join(self.stderr.split("\n")[:GIT_ERROR_STDERR_LINES]) + if self.stderr + else None + ) + project = self.project.name if self.project else None + raise GitCommandError( + project=project, + command_args=self.cmdv, + git_rc=self.rc, + git_stdout=stdout, + git_stderr=stderr, + ) + + +class GitCommandError(GitError): + """ + Error raised from a failed git command. + Note that GitError can refer to any Git related error (e.g. branch not + specified for project.py 'UploadForReview'), while GitCommandError is + raised exclusively from non-zero exit codes returned from git commands. + """ + + def __init__( + self, + message: str = DEFAULT_GIT_FAIL_MESSAGE, + git_rc: int = None, + git_stdout: str = None, + git_stderr: str = None, + **kwargs, + ): + super().__init__( + message, + **kwargs, + ) + self.git_rc = git_rc + self.git_stdout = git_stdout + self.git_stderr = git_stderr + + def __str__(self): + args = "[]" if not self.command_args else " ".join(self.command_args) + error_type = type(self).__name__ + return f"""{error_type}: {self.message} + Project: {self.project} + Args: {args} + Stdout: +{self.git_stdout} + Stderr: +{self.git_stderr}""" diff --git a/subcmds/__init__.py b/subcmds/__init__.py index 4e41afc0..0754f708 100644 --- a/subcmds/__init__.py +++ b/subcmds/__init__.py @@ -16,6 +16,7 @@ import os # A mapping of the subcommand name to the class that implements it. all_commands = {} +all_modules = [] my_dir = os.path.dirname(__file__) for py in os.listdir(my_dir): @@ -42,6 +43,7 @@ for py in os.listdir(my_dir): name = name.replace("_", "-") cmd.NAME = name all_commands[name] = cmd + all_modules.append(mod) # Add 'branch' as an alias for 'branches'. all_commands["branch"] = all_commands["branches"] diff --git a/tests/test_error.py b/tests/test_error.py index 784e2d57..2733ab8c 100644 --- a/tests/test_error.py +++ b/tests/test_error.py @@ -19,6 +19,15 @@ import pickle import unittest import error +import project +import git_command +from subcmds import all_modules + +imports = all_modules + [ + error, + project, + git_command, +] class PickleTests(unittest.TestCase): @@ -26,10 +35,11 @@ class PickleTests(unittest.TestCase): def getExceptions(self): """Return all our custom exceptions.""" - for name in dir(error): - cls = getattr(error, name) - if isinstance(cls, type) and issubclass(cls, Exception): - yield cls + for entry in imports: + for name in dir(entry): + cls = getattr(entry, name) + if isinstance(cls, type) and issubclass(cls, Exception): + yield cls def testExceptionLookup(self): """Make sure our introspection logic works.""" diff --git a/tests/test_git_command.py b/tests/test_git_command.py index c4c3a4c5..1e8beabc 100644 --- a/tests/test_git_command.py +++ b/tests/test_git_command.py @@ -16,6 +16,7 @@ import re import os +import subprocess import unittest try: @@ -65,6 +66,56 @@ class GitCommandTest(unittest.TestCase): ) +class GitCommandWaitTest(unittest.TestCase): + """Tests the GitCommand class .Wait()""" + + def setUp(self): + class MockPopen(object): + rc = 0 + + def communicate( + self, input: str = None, timeout: float = None + ) -> [str, str]: + """Mock communicate fn.""" + return ["", ""] + + def wait(self, timeout=None): + return self.rc + + self.popen = popen = MockPopen() + + def popen_mock(*args, **kwargs): + return popen + + def realpath_mock(val): + return val + + mock.patch.object(subprocess, "Popen", side_effect=popen_mock).start() + + mock.patch.object( + os.path, "realpath", side_effect=realpath_mock + ).start() + + def tearDown(self): + mock.patch.stopall() + + def test_raises_when_verify_non_zero_result(self): + self.popen.rc = 1 + r = git_command.GitCommand(None, ["status"], verify_command=True) + with self.assertRaises(git_command.GitCommandError): + r.Wait() + + def test_returns_when_no_verify_non_zero_result(self): + self.popen.rc = 1 + r = git_command.GitCommand(None, ["status"], verify_command=False) + self.assertEqual(1, r.Wait()) + + def test_default_returns_non_zero_result(self): + self.popen.rc = 1 + r = git_command.GitCommand(None, ["status"]) + self.assertEqual(1, r.Wait()) + + class GitCallUnitTest(unittest.TestCase): """Tests the _GitCall class (via git_command.git)."""