Add traffic annotation self tests.

A script is added to check network traffic annotation tests. Common
parts of this script and the former annotation checker are separated
as annotation_tools.py.

Bug: 690323
Change-Id: If712cca4359a98549318fafe80d64b651765d800
Reviewed-on: https://chromium-review.googlesource.com/951255
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541755}

tools/traffic_annotation/scripts/README.md

@@ -1,2 +1,21 @@
 # Traffic Annotation Scripts
 This file describes the scripts in `tools/traffic_annotation/scripts`
+
+
+# check_annotations.py
+Runs traffic annotation tests on the changed files or all repository. The tests
+are run in error resilient mode. Requires a compiled build directory to run.
+
+# traffic_annotation_auditor_tests.py
+Runs tests to ensure traffic_annotation_auditor is performing as expected. Tests
+include:
+ - Checking if auditor and underlying clang tool run, and an expected minimum
+   number of outputs are returned.
+ - Checking if enabling or disabling heuristics that make tests faster has any
+   effect on the output.
+ - Checking if running on the whole repository (instead of only changed files)
+   would result in any error.
+This test may take a few hours to run and requires a compiled build directory.
+
+# annotation_tools.py
+Provides tools for annotation test scripts.

tools/traffic_annotation/scripts/annotation_tools.py

@@ -0,0 +1,121 @@
+# Copyright 2018 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Tools for annotation test scripts."""
+
+import os
+import subprocess
+import sys
+
+
+class NetworkTrafficAnnotationTools():
+  def __init__(self, build_path=None):
+    """Initializes a NetworkTrafficAnnotationTools object.
+
+    Args:
+      build_path: str Absolute or relative path to a fully compiled build
+          directory. If not specified, the script tries to find it based on
+          relative position of this file (src/tools/traffic_annotation).
+    """
+    self.this_dir = os.path.dirname(os.path.abspath(__file__))
+
+    if not build_path:
+      build_path = self._FindPossibleBuildPath()
+    if build_path:
+      self.build_path = os.path.abspath(build_path)
+
+    self.auditor_path = None
+
+    # For each platform, map the returned platform name from python sys, to
+    # directory name of traffic_annotation_auditor executable.
+    platform = {
+      'linux2': 'linux64',
+      'darwin': 'mac',
+      'win32': 'win32',
+    }[sys.platform]
+    path = os.path.join(self.this_dir, '..', 'bin', platform,
+                        'traffic_annotation_auditor')
+    if sys.platform == 'win32':
+      path += '.exe'
+    if os.path.exists(path):
+      self.auditor_path = path
+
+  def _FindPossibleBuildPath(self):
+    """Returns the first folder in //out that looks like a build dir."""
+    # Assuming this file is in 'tools/traffic_annotation/scripts', three
+    # directories deeper is 'src' and hopefully there is an 'out' in it.
+    out = os.path.abspath(os.path.join(self.this_dir, '..', '..', '..', 'out'))
+    if os.path.exists(out):
+      for folder in os.listdir(out):
+        candidate = os.path.join(out, folder)
+        if (os.path.isdir(candidate) and
+            self._CheckIfDirectorySeemsAsBuild(candidate)):
+          return candidate
+    return None
+
+  def _CheckIfDirectorySeemsAsBuild(self, path):
+    """Checks to see if a directory seems to be a compiled build directory by
+    searching for 'gen' folder and 'build.ninja' file in it.
+    """
+    return all(os.path.exists(
+        os.path.join(path, item)) for item in ('gen', 'build.ninja'))
+
+  def GetModifiedFiles(self):
+    """Gets the list of modified files from git. Returns None if any error
+    happens.
+
+    Returns:
+      list of str List of modified files. Returns None on errors.
+    """
+
+    # List of files is extracted almost the same way as the following test
+    # recipe: https://cs.chromium.org/chromium/tools/depot_tools/recipes/
+    # recipe_modules/tryserver/api.py
+    # '--no-renames' switch is added so that if a file is renamed, both old and
+    # new name would be given. Old name is needed to discard its data in
+    # annotations.xml and new name is needed for updating the XML and checking
+    # its content for possible changes.
+    args = ["git.bat"] if sys.platform == "win32" else ["git"]
+    args += ["diff", "--cached", "--name-only", "--no-renames"]
+
+    original_path = os.getcwd()
+
+    # Change directory to src (two levels upper than build path).
+    os.chdir(os.path.join(self.build_path, "..", ".."))
+    command = subprocess.Popen(args, stdout=subprocess.PIPE,
+                               stderr=subprocess.PIPE)
+    stdout_text, stderr_text = command.communicate()
+
+    if stderr_text:
+      print("Could not run '%s' to get the list of changed files "
+            "beacuse: %s" % (" ".join(args), stderr_text))
+      os.chdir(original_path)
+      return None
+
+    os.chdir(original_path)
+    return stdout_text.splitlines()
+
+  def CanRunAuditor(self):
+    """Retruns true if all required paths to run auditor are known."""
+    return self.build_path and self.auditor_path
+
+  def RunAuditor(self, args):
+    """Runs traffic annotation auditor and returns the results.
+
+    Args:
+      args: list of str Arguments to be passed to traffic annotation auditor.
+
+    Returns:
+      stdout_text: str Auditor's runtime outputs.
+      stderr_text: str Auditor's returned errors.
+      return_code: int Auditor's exit code.
+    """
+
+    command = subprocess.Popen(
+        [self.auditor_path, "--build-path=" + self.build_path] + args,
+        stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    stdout_text, stderr_text = command.communicate()
+    return_code = command.returncode
+
+    return stdout_text, stderr_text, return_code

tools/traffic_annotation/scripts/check_annotations.py

@@ -10,9 +10,9 @@
 
 import os
 import argparse
-import subprocess
 import sys
 
+from annotation_tools import NetworkTrafficAnnotationTools
 
 # If this test starts failing, please set TEST_IS_ENABLED to "False" and file a
 # bug to get this reenabled, and cc the people listed in
@@ -22,10 +22,6 @@
 
 class NetworkTrafficAnnotationChecker():
   EXTENSIONS = ['.cc', '.mm',]
-  COULD_NOT_RUN_MESSAGE = \
-      'Network traffic annotation presubmit check was not performed. To run ' \
-      'it, a compiled build directory and traffic_annotation_auditor binary ' \
-      'are required.'
 
   def __init__(self, build_path=None):
     """Initializes a NetworkTrafficAnnotationChecker object.
@@ -35,133 +31,61 @@ def __init__(self, build_path=None):
           directory. If not specified, the script tries to find it based on
           relative position of this file (src/tools/traffic_annotation).
     """
-    self.this_dir = os.path.dirname(os.path.abspath(__file__))
-
-    if not build_path:
-      build_path = self._FindPossibleBuildPath()
-    if build_path:
-      self.build_path = os.path.abspath(build_path)
-
-    self.auditor_path = None
-    platform = {
-      'linux2': 'linux64',
-      'darwin': 'mac',
-      'win32': 'win32',
-    }[sys.platform]
-    path = os.path.join(self.this_dir, '..', 'bin', platform,
-                        'traffic_annotation_auditor')
-    if sys.platform == 'win32':
-      path += '.exe'
-    if os.path.exists(path):
-      self.auditor_path = path
-
-  def _FindPossibleBuildPath(self):
-    """Returns the first folder in //out that looks like a build dir."""
-    # Assuming this file is in 'tools/traffic_annotation/scripts', three
-    # directories deeper is 'src' and hopefully there is an 'out' in it.
-    out = os.path.abspath(os.path.join(self.this_dir, '..', '..', '..', 'out'))
-    if os.path.exists(out):
-      for folder in os.listdir(out):
-        candidate = os.path.join(out, folder)
-        if (os.path.isdir(candidate) and
-            self._CheckIfDirectorySeemsAsBuild(candidate)):
-          return candidate
-    return None
-
-  def _CheckIfDirectorySeemsAsBuild(self, path):
-    """Checks to see if a directory seems to be a compiled build directory by
-    searching for 'gen' folder and 'build.ninja' file in it.
-    """
-    return all(os.path.exists(
-        os.path.join(path, item)) for item in ('gen', 'build.ninja'))
-
-  def _AllArgsValid(self):
-    return self.auditor_path and self.build_path
+    self.tools = NetworkTrafficAnnotationTools(build_path)
 
   def ShouldCheckFile(self, file_path):
     """Returns true if the input file has an extension relevant to network
     traffic annotations."""
     return os.path.splitext(file_path)[1] in self.EXTENSIONS
 
-  def CheckFiles(self, file_paths=None, limit=0):
+  def CheckFiles(self, complete_run, limit):
     """Passes all given files to traffic_annotation_auditor to be checked for
     possible violations of network traffic annotation rules.
 
     Args:
-      file_paths: list of str List of files to check. If empty, the whole
-          repository will be checked.
-      limit: int Sets the upper threshold for number of errors and warnings,
-          use 0 for unlimited.
+      complete_run: bool Flag requesting to run test on all relevant files.
+      limit: int The upper threshold for number of errors and warnings. Use 0
+          for unlimited.
 
     Returns:
       int Exit code of the network traffic annotation auditor.
     """
-
-    if not TEST_IS_ENABLED:
+    if not self.tools.CanRunAuditor():
+      print("Network traffic annotation presubmit check was not performed. A "
+            "compiled build directory and traffic_annotation_auditor binary "
+            "are required to do it.")
       return 0
 
-    if not self.build_path:
-      return [self.COULD_NOT_RUN_MESSAGE], []
-
-    if file_paths:
+    if complete_run:
+      file_paths = []
+    else:
+      # Get list of modified files. If failed, silently ignore as the test is
+      # run in error resilient mode.
+      file_paths = self.tools.GetModifiedFiles() or []
       file_paths = [
           file_path for file_path in file_paths if self.ShouldCheckFile(
               file_path)]
-
       if not file_paths:
         return 0
-    else:
-      file_paths = []
 
-    args = [self.auditor_path, "--test-only", "--limit=%i" % limit,
-            "--build-path=" + self.build_path, "--error-resilient"] + file_paths
+    args = ["--test-only", "--limit=%i" % limit, "--error-resilient"] + \
+           file_paths
 
-    command = subprocess.Popen(args, stdout=subprocess.PIPE,
-                               stderr=subprocess.PIPE)
-    stdout_text, stderr_text = command.communicate()
+    stdout_text, stderr_text, return_code = self.tools.RunAuditor(args)
 
     if stdout_text:
       print(stdout_text)
     if stderr_text:
       print("\n[Runtime Messages]:\n%s" % stderr_text)
-    return command.returncode
-
-
-  def GetModifiedFiles(self):
-    """Gets the list of modified files from git. Returns None if any error
-    happens."""
-
-    # List of files is extracted almost the same way as the following test
-    # recipe: https://cs.chromium.org/chromium/tools/depot_tools/recipes/
-    # recipe_modules/tryserver/api.py
-    # '--no-renames' switch is added so that if a file is renamed, both old and
-    # new name would be given. Old name is needed to discard its data in
-    # annotations.xml and new name is needed for updating the XML and checking
-    # its content for possible changes.
-    args = ["git.bat"] if sys.platform == "win32" else ["git"]
-    args += ["diff", "--cached", "--name-only", "--no-renames"]
-
-    original_path = os.getcwd()
-
-    # Change directory to src (two levels upper than build path).
-    os.chdir(os.path.join(self.build_path, "..", ".."))
-    command = subprocess.Popen(args, stdout=subprocess.PIPE,
-                               stderr=subprocess.PIPE)
-    stdout_text, stderr_text = command.communicate()
-
-    if stderr_text:
-      print("Could not run '%s' to get the list of changed files "
-            "beacuse: %s" % (" ".join(args), stderr_text))
-      os.chdir(original_path)
-      return None
-
-    os.chdir(original_path)
-    return stdout_text.splitlines()
+    return return_code
 
 
 def main():
+  if not TEST_IS_ENABLED:
+    return 0
+
   parser = argparse.ArgumentParser(
-      description="Traffic Annotation Auditor Presubmit checker.")
+      description="Network Traffic Annotation Presubmit checker.")
   parser.add_argument(
       '--build-path',
       help='Specifies a compiled build directory, e.g. out/Debug. If not '
@@ -177,18 +101,8 @@ def main():
            'modified files are tested.')
 
   args = parser.parse_args()
-
   checker = NetworkTrafficAnnotationChecker(args.build_path)
-  if args.complete:
-    file_paths = None
-  else:
-    file_paths = checker.GetModifiedFiles()
-    if file_paths is None:
-      return -1
-    if len(file_paths) == 0:
-      return 0
-
-  return checker.CheckFiles(file_paths=file_paths, limit=args.limit)
+  return checker.CheckFiles(args.complete, args.limit)
 
 
 if '__main__' == __name__: