diff --git a/tools/traffic_annotation/scripts/README.md b/tools/traffic_annotation/scripts/README.md index 245670a9867061..8a4b9fa94a2917 100644 --- a/tools/traffic_annotation/scripts/README.md +++ b/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. \ No newline at end of file diff --git a/tools/traffic_annotation/scripts/annotation_tools.py b/tools/traffic_annotation/scripts/annotation_tools.py new file mode 100644 index 00000000000000..ca372dcb48fadc --- /dev/null +++ b/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 diff --git a/tools/traffic_annotation/scripts/check_annotations.py b/tools/traffic_annotation/scripts/check_annotations.py index 1bdbf9fdc8527d..1e117f833e1305 100755 --- a/tools/traffic_annotation/scripts/check_annotations.py +++ b/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__: diff --git a/tools/traffic_annotation/scripts/traffic_annotation_auditor_tests.py b/tools/traffic_annotation/scripts/traffic_annotation_auditor_tests.py new file mode 100755 index 00000000000000..9d431b0f558096 --- /dev/null +++ b/tools/traffic_annotation/scripts/traffic_annotation_auditor_tests.py @@ -0,0 +1,65 @@ +#!/usr/bin/env python +# 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. + +"""Runs tests to ensure annotation tests are working as expected. +""" + +import os +import argparse +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 +# //tools/traffic_annotation/OWNERS. +TEST_IS_ENABLED = True + + +class TrafficAnnotationTestsChecker(): + def __init__(self, build_path=None): + """Initializes a TrafficAnnotationTestsChecker object. + + Args: + build_path: str Absolute or relative path to a fully compiled build + directory. + """ + self.tools = NetworkTrafficAnnotationTools(build_path) + + def RunAllTests(self): + result = self.RunOnAllFiles() + #TODO(rhalavati): Add more tests, and create a pipeline for them. + return result + + def RunOnAllFiles(self): + args = ["--test-only"] + _, stderr_text, return_code = self.tools.RunAuditor(args) + + if not return_code: + print("RunOnAlFiles Passed.") + elif stderr_text: + print(stderr_text) + return return_code + + +def main(): + if not TEST_IS_ENABLED: + return 0 + + parser = argparse.ArgumentParser( + description="Traffic Annotation Tests checker.") + parser.add_argument( + '--build-path', + help='Specifies a compiled build directory, e.g. out/Debug. If not ' + 'specified, the script tries to guess it. Will not proceed if not ' + 'found.') + + args = parser.parse_args() + checker = TrafficAnnotationTestsChecker(args.build_path) + return checker.RunAllTests() + + +if '__main__' == __name__: + sys.exit(main()) \ No newline at end of file