Add unit tests for PRESUBMIT.py

Add a unit test program, and mocks, for PRESUBMIT.py. Additionally
add unit tests for two newly added presubmit checks which landed
in http://review.skia.org/668416.

These tests also identified a bug in _CheckReleaseNotesForPublicAPI.
That fix is also in this change.

Bug: skia:14276
Change-Id: I90d86e4d5216067484340e85e31f5a33a3eeba3a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/680017
Reviewed-by: Ravi Mistry <[email protected]>
Commit-Queue: Chris Mumford <[email protected]>
Reviewed-by: Kevin Lubick <[email protected]>

PRESUBMIT.py

@@ -613,7 +613,7 @@ def _CheckReleaseNotesForPublicAPI(input_api, output_api):
     # We only care about files that end in .h and are under the top-level
     # include dir, but not include/private.
     if (file_ext == '.h' and
-        os.path.dirname(file_path) == 'include' and
+        file_path.split(os.path.sep)[0] == 'include' and
         'private' not in file_path):
       public_api_changed = True
     elif os.path.dirname(file_path) == RELEASE_NOTES_DIR:

PRESUBMIT_test.py

@@ -0,0 +1,89 @@
+#!/usr/bin/env python3
+# Copyright 2023 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import io
+import os.path
+import subprocess
+import textwrap
+import unittest
+
+import PRESUBMIT
+
+from PRESUBMIT_test_mocks import MockFile, MockAffectedFile
+from PRESUBMIT_test_mocks import MockInputApi, MockOutputApi
+
+
+class ReleaseNotesTest(unittest.TestCase):
+    def testNoEditTopReleaseNotesNoWarning(self):
+        mock_input_api = MockInputApi()
+        mock_input_api.files = [
+            MockFile('README.chromium', ''),
+        ]
+
+        mock_output_api = MockOutputApi()
+        results = PRESUBMIT._CheckTopReleaseNotesChanged(
+            mock_input_api, mock_output_api)
+
+        self.assertEqual(0, len(results))
+
+    def testUpdateTopReleaseNotesIssuesWarning(self):
+        mock_input_api = MockInputApi()
+        mock_input_api.files = [
+            MockFile('RELEASE_NOTES.md', ''),
+        ]
+
+        mock_output_api = MockOutputApi()
+        results = PRESUBMIT._CheckTopReleaseNotesChanged(
+            mock_input_api, mock_output_api)
+
+        self.assertEqual(1, len(results))
+        self.assertIsInstance(
+            results[0], mock_output_api.PresubmitPromptWarning, 'Not a warning')
+        self.assertTrue(results[0].message.startswith(
+            'Do not edit RELEASE_NOTES.md'))
+
+    def testUpdateTopReleaseNotesNoWarning(self):
+        mock_input_api = MockInputApi()
+        mock_input_api.files = [
+            MockFile('RELEASE_NOTES.md', ''),
+            MockFile('relnotes/deleted_note.md', ''),
+        ]
+
+        mock_output_api = MockOutputApi()
+        results = PRESUBMIT._CheckTopReleaseNotesChanged(
+            mock_input_api, mock_output_api)
+
+        self.assertEqual(0, len(results))
+
+    def testUpdatePublicHeaderAndNoReleaseNoteGeneratesWarning(self):
+        mock_input_api = MockInputApi()
+        mock_input_api.files = [
+            MockFile('include/core/SkDrawable.h', ''),
+        ]
+
+        mock_output_api = MockOutputApi()
+        results = PRESUBMIT._CheckReleaseNotesForPublicAPI(
+            mock_input_api, mock_output_api)
+
+        self.assertEqual(1, len(results))
+        self.assertIsInstance(
+            results[0], mock_output_api.PresubmitPromptWarning, 'Not a warning')
+
+    def testUpdatePublicHeaderAndReleaseNoteGeneratesNoWarning(self):
+        mock_input_api = MockInputApi()
+        mock_input_api.files = [
+            MockFile('include/core/SkDrawable.h', ''),
+            MockFile('relnotes/new_note.md', ''),
+        ]
+
+        mock_output_api = MockOutputApi()
+        results = PRESUBMIT._CheckReleaseNotesForPublicAPI(
+            mock_input_api, mock_output_api)
+
+        self.assertEqual(0, len(results))
+
+
+if __name__ == '__main__':
+    unittest.main()

PRESUBMIT_test_mocks.py

@@ -0,0 +1,285 @@
+# Copyright 2023 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+# This is a copy of PRESUBMIT_test_mocks.py from the Chromium project.
+
+from collections import defaultdict
+import fnmatch
+import json
+import os
+import re
+import subprocess
+import sys
+
+
+def _ReportErrorFileAndLine(filename, line_num, dummy_line):
+    """Default error formatter for _FindNewViolationsOfRule."""
+    return '%s:%s' % (filename, line_num)
+
+
+class MockCannedChecks(object):
+    def _FindNewViolationsOfRule(self, callable_rule, input_api,
+                                 source_file_filter=None,
+                                 error_formatter=_ReportErrorFileAndLine):
+        """Find all newly introduced violations of a per-line rule (a callable).
+
+        Arguments:
+          callable_rule: a callable taking a file extension and line of input and
+            returning True if the rule is satisfied and False if there was a
+            problem.
+          input_api: object to enumerate the affected files.
+          source_file_filter: a filter to be passed to the input api.
+          error_formatter: a callable taking (filename, line_number, line) and
+            returning a formatted error string.
+
+        Returns:
+          A list of the newly-introduced violations reported by the rule.
+        """
+        errors = []
+        for f in input_api.AffectedFiles(include_deletes=False,
+                                         file_filter=source_file_filter):
+            # For speed, we do two passes, checking first the full file.  Shelling out
+            # to the SCM to determine the changed region can be quite expensive on
+            # Win32.  Assuming that most files will be kept problem-free, we can
+            # skip the SCM operations most of the time.
+            extension = str(f.LocalPath()).rsplit('.', 1)[-1]
+            if all(callable_rule(extension, line) for line in f.NewContents()):
+                # No violation found in full text: can skip considering diff.
+                continue
+
+            for line_num, line in f.ChangedContents():
+                if not callable_rule(extension, line):
+                    errors.append(error_formatter(
+                        f.LocalPath(), line_num, line))
+
+        return errors
+
+
+class MockInputApi(object):
+    """Mock class for the InputApi class.
+
+    This class can be used for unittests for presubmit by initializing the files
+    attribute as the list of changed files.
+    """
+
+    DEFAULT_FILES_TO_SKIP = ()
+
+    def __init__(self):
+        self.canned_checks = MockCannedChecks()
+        self.fnmatch = fnmatch
+        self.json = json
+        self.re = re
+        self.os_path = os.path
+        self.platform = sys.platform
+        self.python_executable = sys.executable
+        self.python3_executable = sys.executable
+        self.platform = sys.platform
+        self.subprocess = subprocess
+        self.sys = sys
+        self.files = []
+        self.is_committing = False
+        self.change = MockChange([])
+        self.presubmit_local_path = os.path.dirname(__file__)
+        self.is_windows = sys.platform == 'win32'
+        self.no_diffs = False
+        # Although this makes assumptions about command line arguments used by test
+        # scripts that create mocks, it is a convenient way to set up the verbosity
+        # via the input api.
+        self.verbose = '--verbose' in sys.argv
+
+    def CreateMockFileInPath(self, f_list):
+        self.os_path.exists = lambda x: x in f_list
+
+    def AffectedFiles(self, file_filter=None, include_deletes=True):
+        for file in self.files:
+            if file_filter and not file_filter(file):
+                continue
+            if not include_deletes and file.Action() == 'D':
+                continue
+            yield file
+
+    def RightHandSideLines(self, source_file_filter=None):
+        affected_files = self.AffectedSourceFiles(source_file_filter)
+        for af in affected_files:
+            lines = af.ChangedContents()
+            for line in lines:
+                yield (af, line[0], line[1])
+
+    def AffectedSourceFiles(self, file_filter=None):
+        return self.AffectedFiles(file_filter=file_filter)
+
+    def FilterSourceFile(self, file,
+                         files_to_check=(), files_to_skip=()):
+        local_path = file.LocalPath()
+        found_in_files_to_check = not files_to_check
+        if files_to_check:
+            if type(files_to_check) is str:
+                raise TypeError(
+                    'files_to_check should be an iterable of strings')
+            for pattern in files_to_check:
+                compiled_pattern = re.compile(pattern)
+                if compiled_pattern.match(local_path):
+                    found_in_files_to_check = True
+                    break
+        if files_to_skip:
+            if type(files_to_skip) is str:
+                raise TypeError(
+                    'files_to_skip should be an iterable of strings')
+            for pattern in files_to_skip:
+                compiled_pattern = re.compile(pattern)
+                if compiled_pattern.match(local_path):
+                    return False
+        return found_in_files_to_check
+
+    def LocalPaths(self):
+        return [file.LocalPath() for file in self.files]
+
+    def PresubmitLocalPath(self):
+        return self.presubmit_local_path
+
+    def ReadFile(self, filename, mode='r'):
+        if hasattr(filename, 'AbsoluteLocalPath'):
+            filename = filename.AbsoluteLocalPath()
+        for file_ in self.files:
+            if file_.LocalPath() == filename:
+                return '\n'.join(file_.NewContents())
+        # Otherwise, file is not in our mock API.
+        raise IOError("No such file or directory: '%s'" % filename)
+
+
+class MockOutputApi(object):
+    """Mock class for the OutputApi class.
+
+    An instance of this class can be passed to presubmit unittests for outputting
+    various types of results.
+    """
+
+    class PresubmitResult(object):
+        def __init__(self, message, items=None, long_text=''):
+            self.message = message
+            self.items = items
+            self.long_text = long_text
+
+        def __repr__(self):
+            return self.message
+
+    class PresubmitError(PresubmitResult):
+        def __init__(self, message, items=None, long_text=''):
+            MockOutputApi.PresubmitResult.__init__(
+                self, message, items, long_text)
+            self.type = 'error'
+
+    class PresubmitPromptWarning(PresubmitResult):
+        def __init__(self, message, items=None, long_text=''):
+            MockOutputApi.PresubmitResult.__init__(
+                self, message, items, long_text)
+            self.type = 'warning'
+
+    class PresubmitNotifyResult(PresubmitResult):
+        def __init__(self, message, items=None, long_text=''):
+            MockOutputApi.PresubmitResult.__init__(
+                self, message, items, long_text)
+            self.type = 'notify'
+
+    class PresubmitPromptOrNotify(PresubmitResult):
+        def __init__(self, message, items=None, long_text=''):
+            MockOutputApi.PresubmitResult.__init__(
+                self, message, items, long_text)
+            self.type = 'promptOrNotify'
+
+    def __init__(self):
+        self.more_cc = []
+
+    def AppendCC(self, more_cc):
+        self.more_cc.append(more_cc)
+
+
+class MockFile(object):
+    """Mock class for the File class.
+
+    This class can be used to form the mock list of changed files in
+    MockInputApi for presubmit unittests.
+    """
+
+    def __init__(self, local_path, new_contents, old_contents=None, action='A',
+                 scm_diff=None):
+        self._local_path = local_path
+        self._new_contents = new_contents
+        self._changed_contents = [(i + 1, l)
+                                  for i, l in enumerate(new_contents)]
+        self._action = action
+        if scm_diff:
+            self._scm_diff = scm_diff
+        else:
+            self._scm_diff = (
+                "--- /dev/null\n+++ %s\n@@ -0,0 +1,%d @@\n" %
+                (local_path, len(new_contents)))
+            for l in new_contents:
+                self._scm_diff += "+%s\n" % l
+        self._old_contents = old_contents
+
+    def Action(self):
+        return self._action
+
+    def ChangedContents(self):
+        return self._changed_contents
+
+    def NewContents(self):
+        return self._new_contents
+
+    def LocalPath(self):
+        return self._local_path
+
+    def AbsoluteLocalPath(self):
+        return self._local_path
+
+    def GenerateScmDiff(self):
+        return self._scm_diff
+
+    def OldContents(self):
+        return self._old_contents
+
+    def rfind(self, p):
+        """os.path.basename is called on MockFile so we need an rfind method."""
+        return self._local_path.rfind(p)
+
+    def __getitem__(self, i):
+        """os.path.basename is called on MockFile so we need a get method."""
+        return self._local_path[i]
+
+    def __len__(self):
+        """os.path.basename is called on MockFile so we need a len method."""
+        return len(self._local_path)
+
+    def replace(self, altsep, sep):
+        """os.path.basename is called on MockFile so we need a replace method."""
+        return self._local_path.replace(altsep, sep)
+
+
+class MockAffectedFile(MockFile):
+    def AbsoluteLocalPath(self):
+        return self._local_path
+
+
+class MockChange(object):
+    """Mock class for Change class.
+
+    This class can be used in presubmit unittests to mock the query of the
+    current change.
+    """
+
+    def __init__(self, changed_files):
+        self._changed_files = changed_files
+        self.author_email = None
+        self.footers = defaultdict(list)
+
+    def LocalPaths(self):
+        return self._changed_files
+
+    def AffectedFiles(self, include_dirs=False, include_deletes=True,
+                      file_filter=None):
+        return self._changed_files
+
+    def GitFootersFromDescription(self):
+        return self.footers