diff --git a/git_trace2_event_log.py b/git_trace2_event_log.py index 4a8e0347..dfbded15 100644 --- a/git_trace2_event_log.py +++ b/git_trace2_event_log.py @@ -132,6 +132,30 @@ class EventLog(object): exit_event['code'] = result self._log.append(exit_event) + def _GetEventTargetPath(self): + """Get the 'trace2.eventtarget' path from git configuration. + + Returns: + path: git config's 'trace2.eventtarget' path if it exists, or None + """ + path = None + cmd = ['config', '--get', 'trace2.eventtarget'] + # TODO(https://crbug.com/gerrit/13706): Use GitConfig when it supports + # system git config variables. + p = GitCommand(None, cmd, capture_stdout=True, capture_stderr=True, + bare=True) + retval = p.Wait() + if retval == 0: + # Strip trailing carriage-return in path. + path = p.stdout.rstrip('\n') + elif retval != 1: + # `git config --get` is documented to produce an exit status of `1` if + # the requested variable is not present in the configuration. Report any + # other return value as an error. + print("repo: error: 'git config --get' call failed with return code: %r, stderr: %r" % ( + retval, p.stderr), file=sys.stderr) + return path + def Write(self, path=None): """Writes the log out to a file. @@ -150,21 +174,11 @@ class EventLog(object): log_path = None # If no logging path is specified, get the path from 'trace2.eventtarget'. if path is None: - cmd = ['config', '--get', 'trace2.eventtarget'] - # TODO(https://crbug.com/gerrit/13706): Use GitConfig when it supports - # system git config variables. - p = GitCommand(None, cmd, capture_stdout=True, capture_stderr=True, - bare=True) - retval = p.Wait() - if retval == 0: - # Strip trailing carriage-return in path. - path = p.stdout.rstrip('\n') - elif retval != 1: - # `git config --get` is documented to produce an exit status of `1` if - # the requested variable is not present in the configuration. Report any - # other return value as an error. - print("repo: error: 'git config --get' call failed with return code: %r, stderr: %r" % ( - retval, p.stderr), file=sys.stderr) + path = self._GetEventTargetPath() + + # If no logging path is specified, exit. + if path is None: + return None if isinstance(path, str): # Get absolute path. diff --git a/tests/test_git_trace2_event_log.py b/tests/test_git_trace2_event_log.py index 3905630f..686802ea 100644 --- a/tests/test_git_trace2_event_log.py +++ b/tests/test_git_trace2_event_log.py @@ -15,8 +15,10 @@ """Unittests for the git_trace2_event_log.py module.""" import json +import os import tempfile import unittest +from unittest import mock import git_trace2_event_log @@ -157,12 +159,27 @@ class EventLogTestCase(unittest.TestCase): self.assertIn('code', exit_event) self.assertEqual(exit_event['code'], 2) - # TODO(https://crbug.com/gerrit/13706): Add additional test coverage for - # Write() where: - # - path=None (using git config call) - # - path= (raises TypeError) - # - path= (should return None) - # - tempfile.NamedTemporaryFile errors with FileExistsError (should return None) + def test_write_with_filename(self): + """Test Write() with a path to a file exits with None.""" + self.assertIsNone(self._event_log_module.Write(path='path/to/file')) + + def test_write_with_git_config(self): + """Test Write() uses the git config path when 'git config' call succeeds.""" + with tempfile.TemporaryDirectory(prefix='event_log_tests') as tempdir: + with mock.patch.object(self._event_log_module, + '_GetEventTargetPath', return_value=tempdir): + self.assertEqual(os.path.dirname(self._event_log_module.Write()), tempdir) + + def test_write_no_git_config(self): + """Test Write() with no git config variable present exits with None.""" + with mock.patch.object(self._event_log_module, + '_GetEventTargetPath', return_value=None): + self.assertIsNone(self._event_log_module.Write()) + + def test_write_non_string(self): + """Test Write() with non-string type for |path| throws TypeError.""" + with self.assertRaises(TypeError): + self._event_log_module.Write(path=1234) if __name__ == '__main__':