Fix bug in git trace2 event Write() function when no config present.

See https://bugs.chromium.org/p/gerrit/issues/detail?id=13706#c9

Added additional unit tests for Write() for additional test coverage.

Testing:
- Unit tests
- Verified repo works with:
  - Valid trace2.eventtarget
  - Invalid trace2.eventtarget

Bug: https://crbug.com/gerrit/13706
Tested-by: Ian Kasprzak <iankaz@google.com>
Change-Id: I6b027cb2399bd03e453a132ad82e022a1f48476e
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/292762
Reviewed-by: Mike Frysinger <vapier@google.com>

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']
+      path = self._GetEventTargetPath()
-      # TODO(https://crbug.com/gerrit/13706): Use GitConfig when it supports
+
-      # system git config variables.
+    # If no logging path is specified, exit.
-      p = GitCommand(None, cmd, capture_stdout=True, capture_stderr=True,
+    if path is None:
-                     bare=True)
+      return None
-      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)
 
     if isinstance(path, str):
       # Get absolute path.

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
+  def test_write_with_filename(self):
-  # Write() where:
+    """Test Write() with a path to a file exits with None."""
-  # - path=None (using git config call)
+    self.assertIsNone(self._event_log_module.Write(path='path/to/file'))
-  # - path=<Non-String type> (raises TypeError)
+
-  # - path=<Non-Directory> (should return None)
+  def test_write_with_git_config(self):
-  # - tempfile.NamedTemporaryFile errors with FileExistsError (should return  None)
+    """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__':