[Taskcluster] Make lint create a GitHub Checks output file (#24556)

See #15412

tools/ci/tc/github_checks_output.py

@@ -1,3 +1,10 @@
+MYPY = False
+if MYPY:
+    # MYPY is set to True when run under Mypy.
+    from typing import Any
+    from typing import Optional
+    from typing import Text
+
 class GitHubChecksOutputter(object):
     """Provides a method to output data to be shown in the GitHub Checks UI.
 
@@ -8,22 +15,28 @@ class GitHubChecksOutputter(object):
     See https://docs.taskcluster.net/docs/reference/integrations/github/checks#custom-text-output-in-checks
     """
     def __init__(self, path):
+        # type: (Text) -> None
         self.path = path
 
     def output(self, line):
+        # type: (Any) -> None
+        # TODO(stephenmcgruer): Once mypy 0.790 is released, we can change this
+        # to AnyStr, as that release teaches mypy about the mode flags of open.
+        # See https://github.com/python/typeshed/pull/4146
         with open(self.path, 'a') as f:
             f.write(line)
             f.write('\n')
 
 
 __outputter = None
-def get_gh_checks_outputter(kwargs):
+def get_gh_checks_outputter(filepath):
+    # type: (Optional[Text]) -> Optional[GitHubChecksOutputter]
     """Return the outputter for GitHub Checks output, if enabled.
 
-    :param kwargs: The arguments passed to the program (to look for the
-                   github_checks_text_file field)
+    :param filepath: The filepath to write GitHub Check output information to,
+                     or None if not enabled.
     """
     global __outputter
-    if kwargs['github_checks_text_file'] and __outputter is None:
-        __outputter = GitHubChecksOutputter(kwargs['github_checks_text_file'])
+    if filepath and __outputter is None:
+        __outputter = GitHubChecksOutputter(filepath)
     return __outputter

tools/ci/tc/tasks/test.yml

@@ -386,7 +386,7 @@ tasks:
         - trigger-pr
       description: >-
         Lint for wpt-specific requirements
-      command: "./wpt lint --all"
+      command: "./wpt lint --all --github-checks-text-file=/home/test/artifacts/checkrun.md"
 
   - update-built:
       use:

tools/lint/lint.py

@@ -17,6 +17,7 @@
 from . import fnmatch
 from . import rules
 from .. import localpaths
+from ..ci.tc.github_checks_output import get_gh_checks_outputter, GitHubChecksOutputter
 from ..gitignore.gitignore import PathFilter
 from ..wpt import testfiles
 from ..manifest.vcs import walk
@@ -30,6 +31,7 @@
 if MYPY:
     # MYPY is set to True when run under Mypy.
     from typing import Any
+    from typing import Callable
     from typing import Dict
     from typing import IO
     from typing import Iterable
@@ -809,41 +811,61 @@ def check_file_contents(repo_root, path, f):
     return errors
 
 
-def output_errors_text(errors):
-    # type: (List[rules.Error]) -> None
-    assert logger is not None
+def output_errors_text(log, errors):
+    # type: (Callable[[Any], None], List[rules.Error]) -> None
     for error_type, description, path, line_number in errors:
         pos_string = path
         if line_number:
             pos_string += ":%s" % line_number
-        logger.error("%s: %s (%s)" % (pos_string, description, error_type))
+        log("%s: %s (%s)" % (pos_string, description, error_type))
 
 
-def output_errors_markdown(errors):
-    # type: (List[rules.Error]) -> None
+def output_errors_markdown(log, errors):
+    # type: (Callable[[Any], None], List[rules.Error]) -> None
     if not errors:
         return
-    assert logger is not None
     heading = """Got lint errors:
 
 | Error Type | Position | Message |
 |------------|----------|---------|"""
     for line in heading.split("\n"):
-        logger.error(line)
+        log(line)
     for error_type, description, path, line_number in errors:
         pos_string = path
         if line_number:
             pos_string += ":%s" % line_number
-        logger.error("%s | %s | %s |" % (error_type, pos_string, description))
+        log("%s | %s | %s |" % (error_type, pos_string, description))
 
 
-def output_errors_json(errors):
-    # type: (List[rules.Error]) -> None
+def output_errors_json(log, errors):
+    # type: (Callable[[Any], None], List[rules.Error]) -> None
     for error_type, error, path, line_number in errors:
+        # We use 'print' rather than the log function to ensure that the output
+        # is valid JSON (e.g. with no logger preamble).
         print(json.dumps({"path": path, "lineno": line_number,
                           "rule": error_type, "message": error}))
 
 
+def output_errors_github_checks(outputter, errors, first_reported):
+    # type: (GitHubChecksOutputter, List[rules.Error], bool) -> None
+    """Output errors to the GitHub Checks output markdown format.
+
+    :param outputter: the GitHub Checks outputter
+    :param errors: a list of error tuples (error type, message, path, line number)
+    :param first_reported: True if these are the first reported errors
+    """
+    if first_reported:
+        outputter.output(
+            "\nChanges in this PR contain lint errors, listed below. These "
+            "errors must either be fixed or added to the list of ignored "
+            "errors; see [the documentation]("
+            "https://web-platform-tests.org/writing-tests/lint-tool.html). "
+            "For help, please tag `@web-platform-tests/wpt-core-team` in a "
+            "comment.\n")
+        outputter.output("```")
+    output_errors_text(outputter.output, errors)
+
+
 def output_error_count(error_count):
     # type: (Dict[Text, int]) -> None
     if not error_count:
@@ -910,6 +932,8 @@ def create_parser():
                         "using fnmatch, except that path separators are normalized.")
     parser.add_argument("--all", action="store_true", help="If no paths are passed, try to lint the whole "
                         "working directory, not just files that changed")
+    parser.add_argument("--github-checks-text-file", type=ensure_text,
+                        help="Path to GitHub checks output file for Taskcluster runs")
     return parser
 
 
@@ -935,11 +959,13 @@ def main(**kwargs_str):
 
     ignore_glob = kwargs.get("ignore_glob", [])
 
-    return lint(repo_root, paths, output_format, ignore_glob)
+    github_checks_outputter = get_gh_checks_outputter(kwargs["github_checks_text_file"])
 
+    return lint(repo_root, paths, output_format, ignore_glob, github_checks_outputter)
 
-def lint(repo_root, paths, output_format, ignore_glob=None):
-    # type: (Text, List[Text], Text, Optional[List[Text]]) -> int
+
+def lint(repo_root, paths, output_format, ignore_glob=None, github_checks_outputter=None):
+    # type: (Text, List[Text], Text, Optional[List[Text]], Optional[GitHubChecksOutputter]) -> int
     error_count = defaultdict(int)  # type: Dict[Text, int]
     last = None
 
@@ -964,11 +990,16 @@ def process_errors(errors):
         """
 
         errors = filter_ignorelist_errors(ignorelist, errors)
-
         if not errors:
             return None
 
-        output_errors(errors)
+        assert logger is not None
+        output_errors(logger.error, errors)
+
+        if github_checks_outputter:
+            first_output = len(error_count) == 0
+            output_errors_github_checks(github_checks_outputter, errors, first_output)
+
         for error_type, error, path, line in errors:
             error_count[error_type] += 1
 
@@ -1002,6 +1033,10 @@ def process_errors(errors):
             assert logger is not None
             for line in (ERROR_MSG % (last[0], last[1], last[0], last[1])).split("\n"):
                 logger.info(line)
+
+    if error_count and github_checks_outputter:
+        github_checks_outputter.output("```")
+
     return sum(itervalues(error_count))
 
 

tools/lint/tests/test_lint.py

@@ -530,6 +530,7 @@ def test_main_with_args():
                                           [os.path.relpath(os.path.join(os.getcwd(), x), repo_root)
                                            for x in ['a', 'b', 'c']],
                                           "normal",
+                                          None,
                                           None)
     finally:
         sys.argv = orig_argv
@@ -542,7 +543,7 @@ def test_main_no_args():
         with _mock_lint('lint', return_value=True) as m:
             with _mock_lint('changed_files', return_value=['foo', 'bar']):
                 lint_mod.main(**vars(create_parser().parse_args()))
-                m.assert_called_once_with(repo_root, ['foo', 'bar'], "normal", None)
+                m.assert_called_once_with(repo_root, ['foo', 'bar'], "normal", None, None)
     finally:
         sys.argv = orig_argv
 
@@ -554,6 +555,6 @@ def test_main_all():
         with _mock_lint('lint', return_value=True) as m:
             with _mock_lint('all_filesystem_paths', return_value=['foo', 'bar']):
                 lint_mod.main(**vars(create_parser().parse_args()))
-                m.assert_called_once_with(repo_root, ['foo', 'bar'], "normal", None)
+                m.assert_called_once_with(repo_root, ['foo', 'bar'], "normal", None, None)
     finally:
         sys.argv = orig_argv

tools/wptrunner/wptrunner/stability.py

@@ -347,7 +347,7 @@ def check_stability(logger, repeat_loop=10, repeat_restart=5, chaos_mode=True, m
     start_time = datetime.now()
     step_results = []
 
-    github_checks_outputter = get_gh_checks_outputter(kwargs)
+    github_checks_outputter = get_gh_checks_outputter(kwargs["github_checks_text_file"])
 
     for desc, step_func in steps:
         if max_time and datetime.now() - start_time > max_time:

tools/wptrunner/wptrunner/wptcommandline.py

@@ -5,7 +5,7 @@
 from collections import OrderedDict
 from distutils.spawn import find_executable
 from datetime import timedelta
-from six import iterkeys, itervalues, iteritems
+from six import ensure_text, iterkeys, itervalues, iteritems
 
 from . import config
 from . import wpttest
@@ -350,7 +350,7 @@ def create_parser(product_choices=None):
 
     taskcluster_group = parser.add_argument_group("Taskcluster-specific")
     taskcluster_group.add_argument("--github-checks-text-file",
-                                   dest="github_checks_text_file",
+                                   type=ensure_text,
                                    help="Path to GitHub checks output file")
 
     webkit_group = parser.add_argument_group("WebKit-specific")