diff options
Diffstat (limited to 'infra/presubmit.py')
| -rwxr-xr-x | infra/presubmit.py | 141 |
1 files changed, 93 insertions, 48 deletions
diff --git a/infra/presubmit.py b/infra/presubmit.py index 90b4f90ac..6db1862be 100755 --- a/infra/presubmit.py +++ b/infra/presubmit.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright 2020 Google LLC. +# Copyright 2020 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -14,7 +14,7 @@ # limitations under the License. # ################################################################################ -"""Check code for common issues before submitting.""" +"""Checks code for common issues before submitting.""" import argparse import os @@ -23,6 +23,8 @@ import sys import unittest import yaml +import constants + _SRC_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) @@ -62,8 +64,8 @@ def _check_one_lib_fuzzing_engine(build_sh_file): def check_lib_fuzzing_engine(paths): - """Call _check_one_lib_fuzzing_engine on each path in |paths|. Return True if - the result of every call is True.""" + """Calls _check_one_lib_fuzzing_engine on each path in |paths|. Returns True + if the result of every call is True.""" return all([_check_one_lib_fuzzing_engine(path) for path in paths]) @@ -73,9 +75,9 @@ class ProjectYamlChecker: # Sections in a project.yaml and the constant values that they are allowed # to have. SECTIONS_AND_CONSTANTS = { - 'sanitizers': {'address', 'none', 'memory', 'undefined', 'dataflow'}, - 'architectures': {'i386', 'x86_64'}, - 'fuzzing_engines': {'afl', 'libfuzzer', 'honggfuzz', 'dataflow', 'none'}, + 'sanitizers': constants.SANITIZERS, + 'architectures': constants.ARCHITECTURES, + 'fuzzing_engines': constants.ENGINES, } # Note: this list must be updated when we allow new sections. @@ -100,15 +102,6 @@ class ProjectYamlChecker: 'view_restrictions', ] - LANGUAGES_SUPPORTED = [ - 'c', - 'c++', - 'go', - 'jvm', - 'python', - 'rust', - ] - # Note that some projects like boost only have auto-ccs. However, forgetting # primary contact is probably a mistake. REQUIRED_SECTIONS = ['primary_contact', 'main_repo'] @@ -121,7 +114,7 @@ class ProjectYamlChecker: self.success = True def do_checks(self): - """Do all project.yaml checks. Return True if they pass.""" + """Does all project.yaml checks. Returns True if they pass.""" if self.is_disabled(): return True @@ -131,23 +124,43 @@ class ProjectYamlChecker: self.check_valid_section_names, self.check_valid_emails, self.check_valid_language, + self.check_dataflow, ] for check_function in checks: check_function() return self.success def is_disabled(self): - """Is this project disabled.""" + """Returns True if this project is disabled.""" return self.data.get('disabled', False) def error(self, message): - """Print an error message and set self.success to False.""" + """Prints an error message and sets self.success to False.""" self.success = False print('Error in {filename}: {message}'.format(filename=self.filename, message=message)) + def check_dataflow(self): + """Checks that if "dataflow" is specified in "fuzzing_engines", it is also + specified in "sanitizers", and that if specified in "sanitizers", it is also + specified in "fuzzing_engines". Returns True if this condition is met.""" + engines = self.data.get('fuzzing_engines', []) + dfsan_engines = 'dataflow' in engines + sanitizers = self.data.get('sanitizers', []) + dfsan_sanitizers = 'dataflow' in sanitizers + + if dfsan_engines and not dfsan_sanitizers: + self.error('"dataflow" only specified in "fuzzing_engines" must also be ' + 'specified in "sanitizers" or in neither.') + return + + if dfsan_sanitizers and not dfsan_engines: + self.error('"dataflow" only specified in "sanitizers" must also be ' + 'specified in "fuzzing_engines" or in neither.') + return + def check_project_yaml_constants(self): - """Check that certain sections only have certain constant values.""" + """Returns True if certain sections only have certain constant values.""" for section, allowed_constants in self.SECTIONS_AND_CONSTANTS.items(): if section not in self.data: continue @@ -172,20 +185,20 @@ class ProjectYamlChecker: self.error('Not allowed value in the project.yaml: ' + str(constant)) def check_valid_section_names(self): - """Check that only valid sections are included.""" + """Returns True if all section names are valid.""" for name in self.data: if name not in self.VALID_SECTION_NAMES: self.error('{name} is not a valid section name ({valid_names})'.format( name=name, valid_names=self.VALID_SECTION_NAMES)) def check_required_sections(self): - """Check that all required sections are present.""" + """Returns True if all required sections are in |self.data|.""" for section in self.REQUIRED_SECTIONS: if section not in self.data: self.error(section + ' section is missing.') def check_valid_emails(self): - """Check that emails are valid looking.""" + """Returns True if emails are valid looking..""" # Get email addresses. email_addresses = [] primary_contact = self.data.get('primary_contact') @@ -201,18 +214,18 @@ class ProjectYamlChecker: self.error(email_address + ' is an invalid email address.') def check_valid_language(self): - """Check that the language is specified and valid.""" + """Returns True if the language is specified and valid.""" language = self.data.get('language') if not language: self.error('Missing "language" attribute in project.yaml.') - elif language not in self.LANGUAGES_SUPPORTED: + elif language not in constants.LANGUAGES: self.error( '"language: {language}" is not supported ({supported}).'.format( - language=language, supported=self.LANGUAGES_SUPPORTED)) + language=language, supported=constants.LANGUAGES)) def _check_one_project_yaml(project_yaml_filename): - """Do checks on the project.yaml file.""" + """Does checks on the project.yaml file. Returns True on success.""" if not _is_project_file(project_yaml_filename, 'project.yaml'): return True @@ -221,13 +234,13 @@ def _check_one_project_yaml(project_yaml_filename): def check_project_yaml(paths): - """Call _check_one_project_yaml on each path in |paths|. Return True if - the result of every call is True.""" + """Calls _check_one_project_yaml on each path in |paths|. Returns True if the + result of every call is True.""" return all([_check_one_project_yaml(path) for path in paths]) def do_checks(changed_files): - """Run all presubmit checks return False if any fails.""" + """Runs all presubmit checks. Returns False if any fails.""" checks = [ check_license, yapf, lint, check_project_yaml, check_lib_fuzzing_engine ] @@ -245,6 +258,7 @@ _CHECK_LICENSE_EXTENSIONS = [ '.cc', '.cpp', '.css', + '.Dockerfile', '.h', '.htm', '.html', @@ -253,17 +267,21 @@ _CHECK_LICENSE_EXTENSIONS = [ '.py', '.sh', ] +THIRD_PARTY_DIR_NAME = 'third_party' _LICENSE_STRING = 'http://www.apache.org/licenses/LICENSE-2.0' def check_license(paths): - """Validate license header.""" + """Validates license header.""" if not paths: return True success = True for path in paths: + path_parts = str(path).split(os.sep) + if any(path_part == THIRD_PARTY_DIR_NAME for path_part in path_parts): + continue filename = os.path.basename(path) extension = os.path.splitext(path)[1] if (filename not in _CHECK_LICENSE_FILENAMES and @@ -279,7 +297,7 @@ def check_license(paths): def bool_to_returncode(success): - """Return 0 if |success|. Otherwise return 1.""" + """Returns 0 if |success|. Otherwise returns 1.""" if success: print('Success.') return 0 @@ -294,7 +312,7 @@ def is_nonfuzzer_python(path): def lint(_=None): - """Run python's linter on infra. Return False if it fails linting.""" + """Runs python's linter on infra. Returns False if it fails linting.""" command = ['python3', '-m', 'pylint', '-j', '0', 'infra'] returncode = subprocess.run(command, check=False).returncode @@ -302,9 +320,9 @@ def lint(_=None): def yapf(paths, validate=True): - """Do yapf on |path| if it is Python file. Only validates format if - |validate| otherwise, formats the file. Returns False if validation - or formatting fails.""" + """Does yapf on |path| if it is Python file. Only validates format if + |validate|. Otherwise, formats the file. Returns False if validation or + formatting fails.""" paths = [path for path in paths if is_nonfuzzer_python(path)] if not paths: return True @@ -318,9 +336,9 @@ def yapf(paths, validate=True): def get_changed_files(): - """Return a list of absolute paths of files changed in this git branch.""" + """Returns a list of absolute paths of files changed in this git branch.""" branch_commit_hash = subprocess.check_output( - ['git', 'merge-base', 'FETCH_HEAD', 'origin/HEAD']).strip().decode() + ['git', 'merge-base', 'HEAD', 'origin/HEAD']).strip().decode() diff_commands = [ # Return list of modified files in the commits on this branch. @@ -354,9 +372,9 @@ def run_build_tests(): def run_nonbuild_tests(parallel): - """Run all tests but build tests. Do it in parallel if |parallel|. The reason - why we exclude build tests is because they use an emulator that prevents them - from being used in parallel.""" + """Runs all tests but build tests. Does them in parallel if |parallel|. The + reason why we exclude build tests is because they use an emulator that + prevents them from being used in parallel.""" # We look for all project directories because otherwise pytest won't run tests # that are not in valid modules (e.g. "base-images"). relevant_dirs = set() @@ -369,21 +387,34 @@ def run_nonbuild_tests(parallel): # pass directories to pytest. command = [ 'pytest', - # Test errors with error: "ModuleNotFoundError: No module named 'apt'. - '--ignore-glob=infra/base-images/base-sanitizer-libs-builder/*', '--ignore-glob=infra/build/*', ] if parallel: command.extend(['-n', 'auto']) command += list(relevant_dirs) print('Running non-build tests.') - return subprocess.run(command, check=False).returncode == 0 + # TODO(metzman): Get rid of this once config_utils stops using it. + env = os.environ.copy() + env['CIFUZZ_TEST'] = '1' + + return subprocess.run(command, check=False, env=env).returncode == 0 -def run_tests(_=None, parallel=False): + +def run_tests(_=None, parallel=False, build_tests=True, nonbuild_tests=True): """Runs all unit tests.""" - nonbuild_success = run_nonbuild_tests(parallel) - build_success = run_build_tests() + build_success = True + nonbuild_success = True + if nonbuild_tests: + nonbuild_success = run_nonbuild_tests(parallel) + else: + print('Skipping nonbuild tests as specified.') + + if build_tests: + build_success = run_build_tests() + else: + print('Skipping build tests as specified.') + return nonbuild_success and build_success @@ -411,6 +442,17 @@ def main(): action='store_true', help='Run tests in parallel.', default=False) + parser.add_argument('-s', + '--skip-build-tests', + action='store_true', + help='Skip build tests which are slow and must run ' + 'sequentially.', + default=False) + parser.add_argument('-n', + '--skip-nonbuild-tests', + action='store_true', + help='Only do build tests.', + default=False) args = parser.parse_args() if args.all_files: @@ -434,7 +476,10 @@ def main(): return bool_to_returncode(success) if args.command == 'infra-tests': - success = run_tests(relevant_files, parallel=args.parallel) + success = run_tests(relevant_files, + parallel=args.parallel, + build_tests=(not args.skip_build_tests), + nonbuild_tests=(not args.skip_nonbuild_tests)) return bool_to_returncode(success) # Do all the checks (but no tests). |