diff options
| -rw-r--r-- | infra/build_specified_commit_test.py | 11 | ||||
| -rw-r--r-- | infra/cifuzz/actions/build_fuzzers/build_fuzzers_entrypoint.py | 30 | ||||
| -rw-r--r-- | infra/cifuzz/actions/run_fuzzers/run_fuzzers_entrypoint.py | 15 | ||||
| -rw-r--r-- | infra/cifuzz/cifuzz.py | 65 | ||||
| -rw-r--r-- | infra/cifuzz/cifuzz_test.py | 289 | ||||
| -rw-r--r-- | infra/cifuzz/fuzz_target.py | 55 | ||||
| -rw-r--r-- | infra/cifuzz/fuzz_target_test.py | 81 | ||||
| -rw-r--r-- | infra/cifuzz/test_files/example_curl_file_list | bin | 1940 -> 0 bytes | |||
| -rw-r--r-- | infra/cifuzz/test_files/example_curl_file_list.json | 1 | ||||
| -rw-r--r-- | infra/repo_manager_test.py | 40 | ||||
| -rw-r--r-- | infra/test_repos.py | 3 | ||||
| -rw-r--r-- | infra/utils_test.py | 16 |
12 files changed, 292 insertions, 314 deletions
diff --git a/infra/build_specified_commit_test.py b/infra/build_specified_commit_test.py index 6f666856a..7a8c8fefb 100644 --- a/infra/build_specified_commit_test.py +++ b/infra/build_specified_commit_test.py @@ -35,7 +35,7 @@ class BuildImageIntegrationTests(unittest.TestCase): """Testing if an image can be built from different states e.g. a commit.""" def test_build_fuzzers_from_commit(self): - """Tests if the fuzzers can build at a proper commit. + """Tests if the fuzzers can build at a specified commit. This is done by using a known regression range for a specific test case. The old commit should show the error when its fuzzers run and the new one @@ -72,8 +72,15 @@ class BuildImageIntegrationTests(unittest.TestCase): def test_detect_main_repo_from_commit(self): """Test the detect main repo function from build specific commit module.""" - for example_repo in test_repos.TEST_REPOS: + # TODO(metzman): Fix these tests so they don't randomly break because of + # changes in the outside world. + test_repos_list = [ + repo for repo in test_repos.TEST_REPOS if repo.project_name != 'usrsctp' + ] + for example_repo in test_repos_list: if example_repo.new_commit: + # TODO(metzman): This function calls _build_image_with_retries which + # has a long delay (30 seconds). Figure out how to make this quicker. repo_origin, repo_name = build_specified_commit.detect_main_repo( example_repo.project_name, commit=example_repo.new_commit) self.assertEqual(repo_origin, example_repo.git_url) diff --git a/infra/cifuzz/actions/build_fuzzers/build_fuzzers_entrypoint.py b/infra/cifuzz/actions/build_fuzzers/build_fuzzers_entrypoint.py index f46e6390e..689862c04 100644 --- a/infra/cifuzz/actions/build_fuzzers/build_fuzzers_entrypoint.py +++ b/infra/cifuzz/actions/build_fuzzers/build_fuzzers_entrypoint.py @@ -17,8 +17,7 @@ import logging import os import sys -# pylint: disable=wrong-import-position -# pylint: disable=import-error +# pylint: disable=wrong-import-position,import-error sys.path.append(os.path.join(os.environ['OSS_FUZZ_ROOT'], 'infra', 'cifuzz')) import cifuzz @@ -50,7 +49,7 @@ def main(): SANITIZER: The sanitizer to use when running fuzzers. Returns: - 0 on success or 1 on Failure. + 0 on success or 1 on failure. """ oss_fuzz_project_name = os.environ.get('OSS_FUZZ_PROJECT_NAME') github_repo_name = os.path.basename(os.environ.get('GITHUB_REPOSITORY')) @@ -82,18 +81,19 @@ def main(): return returncode if event == 'pull_request': - with open(os.environ.get('GITHUB_EVENT_PATH'), encoding='utf-8') as file: - event = json.load(file) - pr_ref = 'refs/pull/{0}/merge'.format(event['pull_request']['number']) - if not cifuzz.build_fuzzers(oss_fuzz_project_name, - github_repo_name, - workspace, - pr_ref=pr_ref, - sanitizer=sanitizer): - logging.error( - 'Error building fuzzers for project %s with pull request %s.', - oss_fuzz_project_name, pr_ref) - return returncode + event_path = os.environ.get('GITHUB_EVENT_PATH') + with open(event_path, encoding='utf-8') as file_handle: + event = json.load(file_handle) + pr_ref = 'refs/pull/{0}/merge'.format(event['pull_request']['number']) + if not cifuzz.build_fuzzers(oss_fuzz_project_name, + github_repo_name, + workspace, + pr_ref=pr_ref, + sanitizer=sanitizer): + logging.error( + 'Error building fuzzers for project %s with pull request %s.', + oss_fuzz_project_name, pr_ref) + return returncode out_dir = os.path.join(workspace, 'out') if cifuzz.check_fuzzer_build(out_dir, sanitizer=sanitizer): diff --git a/infra/cifuzz/actions/run_fuzzers/run_fuzzers_entrypoint.py b/infra/cifuzz/actions/run_fuzzers/run_fuzzers_entrypoint.py index 7f39388f6..9f748e7eb 100644 --- a/infra/cifuzz/actions/run_fuzzers/run_fuzzers_entrypoint.py +++ b/infra/cifuzz/actions/run_fuzzers/run_fuzzers_entrypoint.py @@ -16,12 +16,11 @@ import logging import os import sys -# pylint: disable=wrong-import-position -# pylint: disable=import-error +# pylint: disable=wrong-import-position,import-error sys.path.append(os.path.join(os.environ['OSS_FUZZ_ROOT'], 'infra', 'cifuzz')) import cifuzz -# TODO: Turn default logging to INFO when CIFuzz is stable +# TODO: Turn default logging to INFO when CIFuzz is stable. logging.basicConfig( format='%(asctime)s - %(name)s - %(levelname)s - %(message)s', level=logging.DEBUG) @@ -32,9 +31,9 @@ def main(): This is the entrypoint for the run_fuzzers github action. This action can be added to any OSS-Fuzz project's workflow that uses Github. - NOTE: libfuzzer binaries must be located in the ${GITHUB_WORKSPACE}/out + NOTE: libFuzzer binaries must be located in the ${GITHUB_WORKSPACE}/out directory in order for this action to be used. This action will only fuzz the - binary's that are located in that directory. It is reccomended that you add + binaries that are located in that directory. It is recommended that you add the build_fuzzers action preceding this one. NOTE: Any crash report will be in the filepath: @@ -50,7 +49,7 @@ def main(): SANITIZER: The sanitizer to use when running fuzzers. Returns: - 0 on success or 1 on Failure. + 0 on success or 1 on failure. """ fuzz_seconds = int(os.environ.get('FUZZ_SECONDS', 600)) workspace = os.environ.get('GITHUB_WORKSPACE') @@ -65,7 +64,7 @@ def main(): if dry_run: # A testcase file is required in order for CIFuzz to surface bugs. # If the file does not exist, the action will crash attempting to upload it. - # The dry run needs this file because it is set to upload a test case both + # The dry run needs this file because it is set to upload a testcase both # on successful runs and on failures. out_dir = os.path.join(workspace, 'out', 'artifacts') os.makedirs(out_dir, exist_ok=True) @@ -82,7 +81,7 @@ def main(): oss_fuzz_project_name, sanitizer=sanitizer) if not run_status: - logging.error('Error occured while running in workspace %s.', workspace) + logging.error('Error occurred while running in workspace %s.', workspace) return returncode if bug_found: logging.info('Bug found.') diff --git a/infra/cifuzz/cifuzz.py b/infra/cifuzz/cifuzz.py index 11a2485bd..130cf51ed 100644 --- a/infra/cifuzz/cifuzz.py +++ b/infra/cifuzz/cifuzz.py @@ -29,8 +29,7 @@ import urllib.request import fuzz_target -# pylint: disable=wrong-import-position -# pylint: disable=import-error +# pylint: disable=wrong-import-position,import-error sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) import build_specified_commit import helper @@ -38,7 +37,7 @@ import repo_manager import utils # From clusterfuzz: src/python/crash_analysis/crash_analyzer.py -# Used to get the beginning of the stack trace. +# Used to get the beginning of the stacktrace. STACKTRACE_TOOL_MARKERS = [ b'AddressSanitizer', b'ASAN:', @@ -53,7 +52,7 @@ STACKTRACE_TOOL_MARKERS = [ ] # From clusterfuzz: src/python/crash_analysis/crash_analyzer.py -# Used to get the end of the stack trace. +# Used to get the end of the stacktrace. STACKTRACE_END_MARKERS = [ b'ABORTING', b'END MEMORY TOOL REPORT', @@ -66,14 +65,14 @@ STACKTRACE_END_MARKERS = [ b'minidump has been written', ] -# Default fuzz configuration. +# Default fuzz configuration. DEFAULT_ENGINE = 'libfuzzer' DEFAULT_ARCHITECTURE = 'x86_64' # The path to get project's latest report json files. LATEST_REPORT_INFO_PATH = 'oss-fuzz-coverage/latest_report_info/' -# TODO: Turn default logging to WARNING when CIFuzz is stable +# TODO(metzman): Turn default logging to WARNING when CIFuzz is stable. logging.basicConfig( format='%(asctime)s - %(name)s - %(levelname)s - %(message)s', level=logging.DEBUG) @@ -93,14 +92,13 @@ def checkout_specified_commit(build_repo_manager, pr_ref, commit_sha): 'Using current repo state', pr_ref or commit_sha) -# pylint: disable=too-many-arguments -# pylint: disable=too-many-locals -def build_fuzzers(project_name, - project_repo_name, - workspace, - pr_ref=None, - commit_sha=None, - sanitizer='address'): +def build_fuzzers( # pylint: disable=too-many-arguments,too-many-locals + project_name, + project_repo_name, + workspace, + pr_ref=None, + commit_sha=None, + sanitizer='address'): """Builds all of the fuzzers for a specific OSS-Fuzz project. Args: @@ -200,7 +198,11 @@ def build_fuzzers(project_name, return True -def run_fuzzers(fuzz_seconds, workspace, project_name, sanitizer='address'): +def run_fuzzers( # pylint: disable=too-many-arguments,too-many-locals + fuzz_seconds, + workspace, + project_name, + sanitizer='address'): """Runs all fuzzers for a specific OSS-Fuzz project. Args: @@ -225,17 +227,16 @@ def run_fuzzers(fuzz_seconds, workspace, project_name, sanitizer='address'): os.makedirs(artifacts_dir, exist_ok=True) if not fuzz_seconds or fuzz_seconds < 1: logging.error('Fuzz_seconds argument must be greater than 1, but was: %s.', - format(fuzz_seconds)) + fuzz_seconds) return False, False # Get fuzzer information. fuzzer_paths = utils.get_fuzz_targets(out_dir) if not fuzzer_paths: - logging.error('No fuzzers were found in out directory: %s.', - format(out_dir)) + logging.error('No fuzzers were found in out directory: %s.', out_dir) return False, False - # Run fuzzers for alotted time. + # Run fuzzers for allotted time. total_num_fuzzers = len(fuzzer_paths) fuzzers_left_to_run = total_num_fuzzers min_seconds_per_fuzzer = fuzz_seconds // total_num_fuzzers @@ -249,15 +250,15 @@ def run_fuzzers(fuzz_seconds, workspace, project_name, sanitizer='address'): project_name, sanitizer=sanitizer) start_time = time.time() - test_case, stack_trace = target.fuzz() + testcase, stacktrace = target.fuzz() fuzz_seconds -= (time.time() - start_time) - if not test_case or not stack_trace: + if not testcase or not stacktrace: logging.info('Fuzzer %s, finished running.', target.target_name) else: logging.info(b'Fuzzer %s, detected error: %s.', target.target_name, - stack_trace) - shutil.move(test_case, os.path.join(artifacts_dir, 'test_case')) - parse_fuzzer_output(stack_trace, artifacts_dir) + stacktrace) + shutil.move(testcase, os.path.join(artifacts_dir, 'test_case')) + parse_fuzzer_output(stacktrace, artifacts_dir) return True, True fuzzers_left_to_run -= 1 @@ -361,7 +362,7 @@ def get_target_coverage_report(latest_cov_info, target_name): def get_files_covered_by_target(latest_cov_info, target_name, oss_fuzz_repo_path): - """Gets a list of files covered by the specific fuzz target. + """Gets a list of source files covered by the specific fuzz target. Args: latest_cov_info: A dict containing a project's latest cov report info. @@ -372,7 +373,7 @@ def get_files_covered_by_target(latest_cov_info, target_name, A list of files that the fuzzer covers or None. """ if not oss_fuzz_repo_path: - logging.error('Project souce location in docker is not found.' + logging.error('Project source location in docker is not found.' 'Can\'t get coverage information from OSS-Fuzz.') return None target_cov = get_target_coverage_report(latest_cov_info, target_name) @@ -456,17 +457,17 @@ def remove_unaffected_fuzzers(project_name, out_dir, files_changed, try: os.remove(os.path.join(out_dir, fuzzer)) except OSError as error: - logging.error('%s occured while removing file %s', error, fuzzer) + logging.error('%s occurred while removing file %s', error, fuzzer) def get_json_from_url(url): - """Gets a json object from a specified http url. + """Gets a json object from a specified HTTP URL. Args: url: The url of the json to be downloaded. Returns: - Json dict or None on failure. + A dictionary deserialized from JSON or None on failure. """ try: response = urllib.request.urlopen(url) @@ -474,10 +475,10 @@ def get_json_from_url(url): logging.error('HTTP error with url %s.', url) return None try: - # read().decode() fixes compatability issue with urllib response object. + # read().decode() fixes compatibility issue with urllib response object. result_json = json.loads(response.read().decode()) - except (ValueError, TypeError, json.JSONDecodeError) as excp: - logging.error('Loading json from url %s failed with: %s.', url, str(excp)) + except (ValueError, TypeError, json.JSONDecodeError) as err: + logging.error('Loading json from url %s failed with: %s.', url, str(err)) return None return result_json diff --git a/infra/cifuzz/cifuzz_test.py b/infra/cifuzz/cifuzz_test.py index 1de18e8a3..741c41e07 100644 --- a/infra/cifuzz/cifuzz_test.py +++ b/infra/cifuzz/cifuzz_test.py @@ -11,19 +11,20 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -"""Test the functionality of the cifuzz module's functions: +"""Tests the functionality of the cifuzz module's functions: 1. Building fuzzers. 2. Running fuzzers. """ import json import os -import pickle import shutil import sys import tempfile import unittest from unittest import mock +import parameterized + # pylint: disable=wrong-import-position INFRA_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.append(INFRA_DIR) @@ -64,10 +65,10 @@ UNDEFINED_FUZZER = 'curl_fuzzer_undefined' class BuildFuzzersIntegrationTest(unittest.TestCase): - """Test build_fuzzers function in the utils module.""" + """Integration tests for build_fuzzers.""" def test_valid_commit(self): - """Test building fuzzers with valid inputs.""" + """Tests building fuzzers with valid inputs.""" with tempfile.TemporaryDirectory() as tmp_dir: out_path = os.path.join(tmp_dir, 'out') os.mkdir(out_path) @@ -81,7 +82,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase): os.path.exists(os.path.join(out_path, EXAMPLE_BUILD_FUZZER))) def test_valid_pull_request(self): - """Test building fuzzers with valid pull request.""" + """Tests building fuzzers with valid pull request.""" with tempfile.TemporaryDirectory() as tmp_dir: out_path = os.path.join(tmp_dir, 'out') os.mkdir(out_path) @@ -94,7 +95,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase): os.path.exists(os.path.join(out_path, EXAMPLE_BUILD_FUZZER))) def test_invalid_pull_request(self): - """Test building fuzzers with invalid pull request.""" + """Tests building fuzzers with invalid pull request.""" with tempfile.TemporaryDirectory() as tmp_dir: out_path = os.path.join(tmp_dir, 'out') os.mkdir(out_path) @@ -105,7 +106,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase): pr_ref='ref-1/merge')) def test_invalid_project_name(self): - """Test building fuzzers with invalid project name.""" + """Tests building fuzzers with invalid project name.""" with tempfile.TemporaryDirectory() as tmp_dir: self.assertFalse( cifuzz.build_fuzzers( @@ -115,7 +116,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase): commit_sha='0b95fe1039ed7c38fea1f97078316bfc1030c523')) def test_invalid_repo_name(self): - """Test building fuzzers with invalid repo name.""" + """Tests building fuzzers with invalid repo name.""" with tempfile.TemporaryDirectory() as tmp_dir: self.assertFalse( cifuzz.build_fuzzers( @@ -125,7 +126,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase): commit_sha='0b95fe1039ed7c38fea1f97078316bfc1030c523')) def test_invalid_commit_sha(self): - """Test building fuzzers with invalid commit SHA.""" + """Tests building fuzzers with invalid commit SHA.""" with tempfile.TemporaryDirectory() as tmp_dir: with self.assertRaises(AssertionError): cifuzz.build_fuzzers(EXAMPLE_PROJECT, @@ -134,7 +135,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase): commit_sha='') def test_invalid_workspace(self): - """Test building fuzzers with invalid workspace.""" + """Tests building fuzzers with invalid workspace.""" self.assertFalse( cifuzz.build_fuzzers( EXAMPLE_PROJECT, @@ -144,78 +145,77 @@ class BuildFuzzersIntegrationTest(unittest.TestCase): )) -class RunMemoryFuzzerIntegrationTest(unittest.TestCase): - """Test build_fuzzers function in the cifuzz module.""" +def remove_test_files(out_parent_dir, allowlist): + """Removes test files from |out_parent_dir| that are not in |allowlist|, a + list of files with paths relative to the out directory.""" + out_dir = os.path.join(out_parent_dir, 'out') + allowlist = set(allowlist) + for rel_out_path in os.listdir(out_dir): + if rel_out_path in allowlist: + continue + path_to_remove = os.path.join(out_dir, rel_out_path) + if os.path.isdir(path_to_remove): + shutil.rmtree(path_to_remove) + else: + os.remove(path_to_remove) + + +class RunFuzzerIntegrationTestMixin: # pylint: disable=too-few-public-methods,invalid-name + """Mixin for integration test classes that runbuild_fuzzers on builds of a + specific sanitizer.""" + # These must be defined by children. + FUZZER_DIR = None + FUZZER = None def tearDown(self): - """Remove any existing crashes and test files.""" - out_dir = os.path.join(MEMORY_FUZZER_DIR, 'out') - for out_file in os.listdir(out_dir): - out_path = os.path.join(out_dir, out_file) - #pylint: disable=consider-using-in - if out_file == MEMORY_FUZZER: - continue - if os.path.isdir(out_path): - shutil.rmtree(out_path) - else: - os.remove(out_path) + """Removes any existing crashes and test files.""" + remove_test_files(self.FUZZER_DIR, self.FUZZER) - def test_run_with_memory_sanitizer(self): - """Test run_fuzzers with a valid build.""" + def _test_run_with_sanitizer(self, fuzzer_dir, sanitizer): + """Calls run_fuzzers on fuzzer_dir and |sanitizer| and asserts + the run succeeded and that no bug was found.""" run_success, bug_found = cifuzz.run_fuzzers(10, - MEMORY_FUZZER_DIR, + fuzzer_dir, 'curl', - sanitizer='memory') + sanitizer=sanitizer) self.assertTrue(run_success) self.assertFalse(bug_found) -class RunUndefinedFuzzerIntegrationTest(unittest.TestCase): - """Test build_fuzzers function in the cifuzz module.""" +class RunMemoryFuzzerIntegrationTest(unittest.TestCase, + RunFuzzerIntegrationTestMixin): + """Integration test for build_fuzzers with an MSAN build.""" + FUZZER_DIR = MEMORY_FUZZER_DIR + FUZZER = MEMORY_FUZZER - def tearDown(self): - """Remove any existing crashes and test files.""" - out_dir = os.path.join(UNDEFINED_FUZZER_DIR, 'out') - for out_file in os.listdir(out_dir): - out_path = os.path.join(out_dir, out_file) - #pylint: disable=consider-using-in - if out_file == UNDEFINED_FUZZER: - continue - if os.path.isdir(out_path): - shutil.rmtree(out_path) - else: - os.remove(out_path) + def test_run_with_memory_sanitizer(self): + """Tests run_fuzzers with a valid MSAN build.""" + self._test_run_with_sanitizer(self.FUZZER_DIR, 'memory') + + +class RunUndefinedFuzzerIntegrationTest(unittest.TestCase, + RunFuzzerIntegrationTestMixin): + """Integration test for build_fuzzers with an UBSAN build.""" + FUZZER_DIR = UNDEFINED_FUZZER_DIR + FUZZER = UNDEFINED_FUZZER def test_run_with_undefined_sanitizer(self): - """Test run_fuzzers with a valid build.""" - run_success, bug_found = cifuzz.run_fuzzers(10, - UNDEFINED_FUZZER_DIR, - 'curl', - sanitizer='undefined') - self.assertTrue(run_success) - self.assertFalse(bug_found) + """Tests run_fuzzers with a valid MSAN build.""" + self._test_run_with_sanitizer(self.FUZZER_DIR, 'undefined') class RunAddressFuzzersIntegrationTest(unittest.TestCase): - """Test build_fuzzers function in the cifuzz module.""" + """Integration tests for build_fuzzers with an ASAN build.""" def tearDown(self): - """Remove any existing crashes and test files.""" - out_dir = os.path.join(TEST_FILES_PATH, 'out') + """Removes any existing crashes and test files.""" files_to_keep = [ 'undefined', 'memory', EXAMPLE_CRASH_FUZZER, EXAMPLE_NOCRASH_FUZZER ] - for out_file in os.listdir(out_dir): - out_path = os.path.join(out_dir, out_file) - if out_file in files_to_keep: - continue - if os.path.isdir(out_path): - shutil.rmtree(out_path) - else: - os.remove(out_path) + remove_test_files(TEST_FILES_PATH, files_to_keep) def test_new_bug_found(self): - """Test run_fuzzers with a valid build.""" + """Tests run_fuzzers with a valid ASAN build.""" # Set the first return value to True, then the second to False to # emulate a bug existing in the current PR but not on the downloaded # OSS-Fuzz build. @@ -231,7 +231,7 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase): self.assertTrue(bug_found) def test_old_bug_found(self): - """Test run_fuzzers with a bug found in OSS-Fuzz before.""" + """Tests run_fuzzers with a bug found in OSS-Fuzz before.""" with mock.patch.object(fuzz_target.FuzzTarget, 'is_reproducible', side_effect=[True, True]): @@ -244,7 +244,7 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase): self.assertFalse(bug_found) def test_invalid_build(self): - """Test run_fuzzers with an invalid build.""" + """Tests run_fuzzers with an invalid ASAN build.""" with tempfile.TemporaryDirectory() as tmp_dir: out_path = os.path.join(tmp_dir, 'out') os.mkdir(out_path) @@ -269,8 +269,8 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase): self.assertFalse(bug_found) -class ParseOutputUnitTest(unittest.TestCase): - """Test parse_fuzzer_output function in the cifuzz module.""" +class ParseOutputTest(unittest.TestCase): + """Tests parse_fuzzer_output.""" def test_parse_valid_output(self): """Checks that the parse fuzzer output can correctly parse output.""" @@ -284,9 +284,9 @@ class ParseOutputUnitTest(unittest.TestCase): self.assertCountEqual(os.listdir(tmp_dir), result_files) # Compare the bug summaries. - with open(os.path.join(tmp_dir, 'bug_summary.txt'), 'r') as bug_summary: + with open(os.path.join(tmp_dir, 'bug_summary.txt')) as bug_summary: detected_summary = bug_summary.read() - with open(os.path.join(test_summary_path), 'r') as bug_summary: + with open(test_summary_path) as bug_summary: real_summary = bug_summary.read() self.assertEqual(detected_summary, real_summary) @@ -297,7 +297,7 @@ class ParseOutputUnitTest(unittest.TestCase): self.assertEqual(len(os.listdir(tmp_dir)), 0) -class CheckFuzzerBuildUnitTest(unittest.TestCase): +class CheckFuzzerBuildTest(unittest.TestCase): """Tests the check_fuzzer_build function in the cifuzz module.""" def test_correct_fuzzer_build(self): @@ -318,28 +318,27 @@ class CheckFuzzerBuildUnitTest(unittest.TestCase): def test_allow_broken_fuzz_targets_percentage(self, mocked_docker_run): """Tests that ALLOWED_BROKEN_TARGETS_PERCENTAGE is set when running docker if it is set in the environment.""" - test_fuzzer_dir = os.path.join(TEST_FILES_PATH, 'out') mocked_docker_run.return_value = 0 + test_fuzzer_dir = os.path.join(TEST_FILES_PATH, 'out') cifuzz.check_fuzzer_build(test_fuzzer_dir) self.assertIn('-e ALLOWED_BROKEN_TARGETS_PERCENTAGE=0', ' '.join(mocked_docker_run.call_args[0][0])) -class GetFilesCoveredByTargetUnitTest(unittest.TestCase): - """Test to get the files covered by a fuzz target in the cifuzz module.""" +class GetFilesCoveredByTargetTest(unittest.TestCase): + """Tests get_files_covered_by_target.""" example_cov_json = 'example_curl_cov.json' example_fuzzer_cov_json = 'example_curl_fuzzer_cov.json' example_fuzzer = 'curl_fuzzer' - example_curl_file_list = 'example_curl_file_list' def setUp(self): - with open(os.path.join(TEST_FILES_PATH, self.example_cov_json), - 'r') as file: - self.proj_cov_report_example = json.loads(file.read()) - with open(os.path.join(TEST_FILES_PATH, self.example_fuzzer_cov_json), - 'r') as file: - self.fuzzer_cov_report_example = json.loads(file.read()) + with open(os.path.join(TEST_FILES_PATH, self.example_cov_json) + ) as file_handle: + self.proj_cov_report_example = json.loads(file_handle.read()) + with open(os.path.join(TEST_FILES_PATH, self.example_fuzzer_cov_json) + ) as file_handle: + self.fuzzer_cov_report_example = json.loads(file_handle.read()) def test_valid_target(self): """Tests that covered files can be retrieved from a coverage report.""" @@ -350,13 +349,14 @@ class GetFilesCoveredByTargetUnitTest(unittest.TestCase): file_list = cifuzz.get_files_covered_by_target( self.proj_cov_report_example, self.example_fuzzer, '/src/curl') - with open(os.path.join(TEST_FILES_PATH, 'example_curl_file_list'), - 'rb') as file_handle: - true_files_list = pickle.load(file_handle) + curl_files_list_path = os.path.join( + TEST_FILES_PATH, 'example_curl_file_list.json') + with open(curl_files_list_path) as file_handle: + true_files_list = json.load(file_handle) self.assertCountEqual(file_list, true_files_list) def test_invalid_target(self): - """Test asserts an invalid fuzzer returns None.""" + """Tests passing invalid fuzz target returns None.""" self.assertIsNone( cifuzz.get_files_covered_by_target(self.proj_cov_report_example, 'not-a-fuzzer', '/src/curl')) @@ -365,7 +365,7 @@ class GetFilesCoveredByTargetUnitTest(unittest.TestCase): '/src/curl')) def test_invalid_project_build_dir(self): - """Test asserts an invalid build dir returns None.""" + """Tests passing an invalid build directory returns None.""" self.assertIsNone( cifuzz.get_files_covered_by_target(self.proj_cov_report_example, self.example_fuzzer, '/no/pe')) @@ -374,43 +374,44 @@ class GetFilesCoveredByTargetUnitTest(unittest.TestCase): self.example_fuzzer, '')) -class GetTargetCoverageReporUnitTest(unittest.TestCase): - """Test get_target_coverage_report function in the cifuzz module.""" +class GetTargetCoverageReportTest(unittest.TestCase): + """Tests get_target_coverage_report.""" example_cov_json = 'example_curl_cov.json' example_fuzzer = 'curl_fuzzer' def setUp(self): with open(os.path.join(TEST_FILES_PATH, self.example_cov_json), - 'r') as file: - self.cov_exmp = json.loads(file.read()) + 'r') as file_handle: + self.example_cov = json.loads(file_handle.read()) def test_valid_target(self): - """Test a target's coverage report can be downloaded and parsed.""" + """Tests that a target's coverage report can be downloaded and parsed.""" with mock.patch.object(cifuzz, 'get_json_from_url', return_value='{}') as mock_get_json: - cifuzz.get_target_coverage_report(self.cov_exmp, self.example_fuzzer) + cifuzz.get_target_coverage_report(self.example_cov, self.example_fuzzer) (url,), _ = mock_get_json.call_args self.assertEqual( 'https://storage.googleapis.com/oss-fuzz-coverage/' 'curl/fuzzer_stats/20200226/curl_fuzzer.json', url) def test_invalid_target(self): - """Test an invalid target coverage report will be None.""" + """Tests that passing an invalid target coverage report returns None.""" self.assertIsNone( - cifuzz.get_target_coverage_report(self.cov_exmp, 'not-valid-target')) - self.assertIsNone(cifuzz.get_target_coverage_report(self.cov_exmp, '')) + cifuzz.get_target_coverage_report(self.example_cov, 'not-valid-target')) + self.assertIsNone(cifuzz.get_target_coverage_report(self.example_cov, '')) def test_invalid_project_json(self): - """Test a project json coverage report will be None.""" + """Tests that passing an invalid project json coverage report returns + None.""" self.assertIsNone( cifuzz.get_target_coverage_report('not-a-proj', self.example_fuzzer)) self.assertIsNone(cifuzz.get_target_coverage_report('', self.example_fuzzer)) -class GetLatestCoverageReportUnitTest(unittest.TestCase): - """Test get_latest_cov_report_info function in the cifuzz module.""" +class GetLatestCoverageReportTest(unittest.TestCase): + """Tests get_latest_cov_report_info.""" test_project = 'curl' @@ -418,7 +419,7 @@ class GetLatestCoverageReportUnitTest(unittest.TestCase): """Tests that a project's coverage report can be downloaded and parsed. NOTE: This test relies on the test_project repo's coverage report. - Example was not used because it has no coverage reports. + The "example" project was not used because it has no coverage reports. """ with mock.patch.object(cifuzz, 'get_json_from_url', return_value='{}') as mock_fun: @@ -430,80 +431,56 @@ class GetLatestCoverageReportUnitTest(unittest.TestCase): 'latest_report_info/curl.json', url) def test_get_invalid_project(self): - """Tests a project's coverage report will return None if bad project.""" + """Tests that passing a bad project returns None.""" self.assertIsNone(cifuzz.get_latest_cov_report_info('not-a-proj')) self.assertIsNone(cifuzz.get_latest_cov_report_info('')) -class KeepAffectedFuzzersUnitTest(unittest.TestCase): - """Test the keep_affected_fuzzer method in the CIFuzz module.""" +EXAMPLE_FILE_CHANGED = 'test.txt' - test_fuzzer_1 = os.path.join(TEST_FILES_PATH, 'out', 'example_crash_fuzzer') - test_fuzzer_2 = os.path.join(TEST_FILES_PATH, 'out', 'example_nocrash_fuzzer') - example_file_changed = 'test.txt' - def test_keeping_fuzzer_w_no_coverage(self): - """Tests that a specific fuzzer is kept if it is deemed affected.""" - with tempfile.TemporaryDirectory() as tmp_dir, mock.patch.object( - cifuzz, 'get_latest_cov_report_info', return_value=1): - shutil.copy(self.test_fuzzer_1, tmp_dir) - shutil.copy(self.test_fuzzer_2, tmp_dir) - with mock.patch.object(cifuzz, - 'get_files_covered_by_target', - side_effect=[[self.example_file_changed], None]): - cifuzz.remove_unaffected_fuzzers(EXAMPLE_PROJECT, tmp_dir, - [self.example_file_changed], '') - self.assertEqual(2, len(os.listdir(tmp_dir))) - - def test_keeping_specific_fuzzer(self): - """Tests that a specific fuzzer is kept if it is deemed affected.""" - with tempfile.TemporaryDirectory() as tmp_dir, mock.patch.object( - cifuzz, 'get_latest_cov_report_info', return_value=1): - shutil.copy(self.test_fuzzer_1, tmp_dir) - shutil.copy(self.test_fuzzer_2, tmp_dir) - with mock.patch.object(cifuzz, - 'get_files_covered_by_target', - side_effect=[[self.example_file_changed], - ['not/a/real/file']]): - cifuzz.remove_unaffected_fuzzers(EXAMPLE_PROJECT, tmp_dir, - [self.example_file_changed], '') - self.assertEqual(1, len(os.listdir(tmp_dir))) - - def test_no_fuzzers_kept_fuzzer(self): - """Tests that if there is no affected then all fuzzers are kept.""" - with tempfile.TemporaryDirectory() as tmp_dir, mock.patch.object( - cifuzz, 'get_latest_cov_report_info', return_value=1): - shutil.copy(self.test_fuzzer_1, tmp_dir) - shutil.copy(self.test_fuzzer_2, tmp_dir) +class RemoveUnaffectedFuzzersTest(unittest.TestCase): + """Tests remove_unaffected_fuzzers.""" + + TEST_FUZZER_1 = os.path.join(TEST_FILES_PATH, 'out', 'example_crash_fuzzer') + TEST_FUZZER_2 = os.path.join(TEST_FILES_PATH, 'out', 'example_nocrash_fuzzer') + + # yapf: disable + @parameterized.parameterized.expand([ + # Tests a specific affected fuzzers is kept. + ([[EXAMPLE_FILE_CHANGED], None], 2,), + + # Tests specific affected fuzzer is kept. + ([[EXAMPLE_FILE_CHANGED], ['not/a/real/file']], 1), + + # Tests all fuzzers are kept if none are deemed affected. + ([None, None], 2), + + # Tests that multiple fuzzers are kept if multiple fuzzers are affected. + ([[EXAMPLE_FILE_CHANGED], [EXAMPLE_FILE_CHANGED]], 2), + ]) + # yapf: enable + def test_remove_unaffected_fuzzers(self, side_effect, expected_dir_len): + """Tests that remove_unaffected_fuzzers has the intended effect.""" + with tempfile.TemporaryDirectory() as tmp_dir, mock.patch( + 'cifuzz.get_latest_cov_report_info', return_value=1): with mock.patch.object(cifuzz, - 'get_files_covered_by_target', - side_effect=[None, None]): - cifuzz.remove_unaffected_fuzzers(EXAMPLE_PROJECT, tmp_dir, - [self.example_file_changed], '') - self.assertEqual(2, len(os.listdir(tmp_dir))) - - def test_both_fuzzers_kept_fuzzer(self): - """Tests that if both fuzzers are affected then both fuzzers are kept.""" - with tempfile.TemporaryDirectory() as tmp_dir, mock.patch.object( - cifuzz, 'get_latest_cov_report_info', return_value=1): - shutil.copy(self.test_fuzzer_1, tmp_dir) - shutil.copy(self.test_fuzzer_2, tmp_dir) - with mock.patch.object( - cifuzz, - 'get_files_covered_by_target', - side_effect=[self.example_file_changed, self.example_file_changed]): + 'get_files_covered_by_target') as mocked_get_files: + mocked_get_files.side_effect = side_effect + shutil.copy(self.TEST_FUZZER_1, tmp_dir) + shutil.copy(self.TEST_FUZZER_2, tmp_dir) cifuzz.remove_unaffected_fuzzers(EXAMPLE_PROJECT, tmp_dir, - [self.example_file_changed], '') - self.assertEqual(2, len(os.listdir(tmp_dir))) + [EXAMPLE_FILE_CHANGED], '') + self.assertEqual(expected_dir_len, len(os.listdir(tmp_dir))) @unittest.skip('Test is too long to be run with presubmit.') class BuildSantizerIntegrationTest(unittest.TestCase): - """Integration tests for the build_fuzzers function in the cifuzz module. - Note: This test relies on the curl project being an OSS-Fuzz project.""" + """Integration tests for the build_fuzzers. + Note: This test relies on "curl" being an OSS-Fuzz project.""" def test_valid_project_curl_memory(self): - """Test if sanitizers can be detected from project.yaml""" + """Tests that MSAN can be detected from project.yaml""" with tempfile.TemporaryDirectory() as tmp_dir: self.assertTrue( cifuzz.build_fuzzers('curl', @@ -513,7 +490,7 @@ class BuildSantizerIntegrationTest(unittest.TestCase): sanitizer='memory')) def test_valid_project_curl_undefined(self): - """Test if sanitizers can be detected from project.yaml""" + """Test that UBSAN can be detected from project.yaml""" with tempfile.TemporaryDirectory() as tmp_dir: self.assertTrue( cifuzz.build_fuzzers('curl', diff --git a/infra/cifuzz/fuzz_target.py b/infra/cifuzz/fuzz_target.py index 28dd80baa..176f23089 100644 --- a/infra/cifuzz/fuzz_target.py +++ b/infra/cifuzz/fuzz_target.py @@ -58,8 +58,8 @@ REPRODUCE_ATTEMPTS = 10 # Seconds on top of duration until a timeout error is raised. BUFFER_TIME = 10 -# Log message for is_crash_reportable if it can't check if crash repros -# on OSS-Fuzz build. +# Log message for is_crash_reportable if it can't check if crash reproduces on +# an OSS-Fuzz build. COULD_NOT_TEST_ON_OSS_FUZZ_MESSAGE = ( 'Crash is reproducible. Could not run OSS-Fuzz build of ' 'target to determine if this pull request introduced crash. ' @@ -110,7 +110,7 @@ class FuzzTarget: """Starts the fuzz target run for the length of time specified by duration. Returns: - (test_case, stack trace, time in seconds) on crash or + (testcase, stacktrace, time in seconds) on crash or (None, None, time in seconds) on timeout or error. """ logging.info('Fuzzer %s, started.', self.target_name) @@ -158,19 +158,19 @@ class FuzzTarget: # Crash was discovered. logging.info('Fuzzer %s, ended before timeout.', self.target_name) - test_case = self.get_test_case(stderr) - if not test_case: - logging.error(b'No test case found in stack trace: %s.', stderr) + testcase = self.get_testcase(stderr) + if not testcase: + logging.error(b'No testcase found in stacktrace: %s.', stderr) return None, None - if self.is_crash_reportable(test_case): - return test_case, stderr + if self.is_crash_reportable(testcase): + return testcase, stderr return None, None - def is_reproducible(self, test_case, target_path): - """Checks if the test case reproduces. + def is_reproducible(self, testcase, target_path): + """Checks if the testcase reproduces. Args: - test_case: The path to the test case to be tested. + testcase: The path to the testcase to be tested. target_path: The path to the fuzz target to be tested Returns: @@ -192,13 +192,13 @@ class FuzzTarget: if container: command += [ '--volumes-from', container, '-e', 'OUT=' + target_dirname, '-e', - 'TESTCASE=' + test_case + 'TESTCASE=' + testcase ] else: command += [ '-v', '%s:/out' % target_dirname, '-v', - '%s:/testcase' % test_case + '%s:/testcase' % testcase ] command += [ @@ -219,7 +219,7 @@ class FuzzTarget: target_path) return False - def is_crash_reportable(self, test_case): + def is_crash_reportable(self, testcase): """Returns True if a crash is reportable. This means the crash is reproducible but not reproducible on a build from OSS-Fuzz (meaning the crash was introduced by this PR). @@ -228,7 +228,7 @@ class FuzzTarget: by the pull request if it is reproducible. Args: - test_case: The path to the test_case that triggered the crash. + testcase: The path to the testcase that triggered the crash. Returns: True if the crash was introduced by the current pull request. @@ -236,11 +236,11 @@ class FuzzTarget: Raises: ReproduceError if we can't attempt to reproduce the crash on the PR build. """ - if not os.path.exists(test_case): - raise ReproduceError('Test case %s not found.' % test_case) + if not os.path.exists(testcase): + raise ReproduceError('Testcase %s not found.' % testcase) try: - reproducible_on_pr_build = self.is_reproducible(test_case, + reproducible_on_pr_build = self.is_reproducible(testcase, self.target_path) except ReproduceError as error: logging.error('Could not run target when checking for reproducibility.' @@ -252,8 +252,7 @@ class FuzzTarget: return reproducible_on_pr_build if not reproducible_on_pr_build: - logging.info( - 'Failed to reproduce the crash using the obtained test case.') + logging.info('Failed to reproduce the crash using the obtained testcase.') return False oss_fuzz_build_dir = self.download_oss_fuzz_build() @@ -265,7 +264,7 @@ class FuzzTarget: oss_fuzz_target_path = os.path.join(oss_fuzz_build_dir, self.target_name) try: reproducible_on_oss_fuzz_build = self.is_reproducible( - test_case, oss_fuzz_target_path) + testcase, oss_fuzz_target_path) except ReproduceError: # This happens if the project has OSS-Fuzz builds, but the fuzz target # is not in it (e.g. because the fuzz target is new). @@ -281,21 +280,21 @@ class FuzzTarget: logging.info('The crash is reproducible without the current pull request.') return False - def get_test_case(self, error_bytes): - """Gets the file from a fuzzer run stack trace. + def get_testcase(self, error_bytes): + """Gets the file from a fuzzer run stacktrace. Args: error_bytes: The bytes containing the output from the fuzzer. Returns: - The path to the test case or None if not found. + The path to the testcase or None if not found. """ match = re.search(rb'\bTest unit written to \.\/([^\s]+)', error_bytes) if match: return os.path.join(self.out_dir, match.group(1).decode('utf-8')) return None - def get_lastest_build_version(self): + def get_latest_build_version(self): """Gets the latest OSS-Fuzz build version for a projects' fuzzers. Returns: @@ -332,7 +331,7 @@ class FuzzTarget: if os.path.exists(os.path.join(build_dir, self.target_name)): return build_dir os.makedirs(build_dir, exist_ok=True) - latest_build_str = self.get_lastest_build_version() + latest_build_str = self.get_latest_build_version() if not latest_build_str: return None @@ -401,7 +400,7 @@ def download_url(url, filename, num_retries=3): def download_and_unpack_zip(url, out_dir): - """Downloads and unpacks a zip file from an http url. + """Downloads and unpacks a zip file from an HTTP URL. Args: url: A url to the zip file to be downloaded and unpacked. @@ -431,7 +430,7 @@ def download_and_unpack_zip(url, out_dir): def url_join(*url_parts): - """Joins URLs together using the posix join method. + """Joins URLs together using the POSIX join method. Args: url_parts: Sections of a URL to be joined. diff --git a/infra/cifuzz/fuzz_target_test.py b/infra/cifuzz/fuzz_target_test.py index b4600f017..3e8788813 100644 --- a/infra/cifuzz/fuzz_target_test.py +++ b/infra/cifuzz/fuzz_target_test.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -"""Test the functionality of the fuzz_target module.""" +"""Tests the functionality of the fuzz_target module.""" import os import sys @@ -23,9 +23,8 @@ import urllib.error import parameterized from pyfakefs import fake_filesystem_unittest -# Pylint has issue importing utils which is why error suppression is required. -# pylint: disable=wrong-import-position -# pylint: disable=import-error +# Pylint has an issue importing utils which is why error suppression is needed. +# pylint: disable=wrong-import-position,import-error sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) import fuzz_target @@ -46,7 +45,7 @@ EXECUTE_FAILURE_RETVAL = ('', '', 1) # TODO(metzman): Use patch from test_libs/helpers.py in clusterfuzz so that we # don't need to accept this as an argument in every test method. @unittest.mock.patch('utils.get_container_name', return_value='container') -class IsReproducibleUnitTest(fake_filesystem_unittest.TestCase): +class IsReproducibleTest(fake_filesystem_unittest.TestCase): """Tests the is_reproducible method in the fuzz_target.FuzzTarget class.""" def setUp(self): @@ -112,42 +111,42 @@ class IsReproducibleUnitTest(fake_filesystem_unittest.TestCase): self.assertFalse(result) -class GetTestCaseUnitTest(unittest.TestCase): - """Test get_test_case function in the fuzz_target module.""" +class GetTestCaseTest(unittest.TestCase): + """Tests get_testcase.""" def setUp(self): - """Sets up dummy fuzz target to test get_test_case method.""" + """Sets up dummy fuzz target to test get_testcase method.""" self.test_target = fuzz_target.FuzzTarget('/example/path', 10, '/example/outdir') def test_valid_error_string(self): - """Tests that get_test_case returns the correct test case give an error.""" - test_case_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), - 'test_files', - 'example_crash_fuzzer_output.txt') - with open(test_case_path, 'rb') as test_fuzz_output: - parsed_test_case = self.test_target.get_test_case(test_fuzz_output.read()) + """Tests that get_testcase returns the correct testcase give an error.""" + testcase_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), + 'test_files', + 'example_crash_fuzzer_output.txt') + with open(testcase_path, 'rb') as test_fuzz_output: + parsed_testcase = self.test_target.get_testcase(test_fuzz_output.read()) self.assertEqual( - parsed_test_case, + parsed_testcase, '/example/outdir/crash-ad6700613693ef977ff3a8c8f4dae239c3dde6f5') def test_invalid_error_string(self): - """Tests that get_test_case returns None with a bad error string.""" - self.assertIsNone(self.test_target.get_test_case(b'')) - self.assertIsNone(self.test_target.get_test_case(b' Example crash string.')) + """Tests that get_testcase returns None with a bad error string.""" + self.assertIsNone(self.test_target.get_testcase(b'')) + self.assertIsNone(self.test_target.get_testcase(b' Example crash string.')) def test_encoding(self): - """Tests that get_test_case accepts bytes and returns a string.""" + """Tests that get_testcase accepts bytes and returns a string.""" fuzzer_output = b'\x8fTest unit written to ./crash-1' - result = self.test_target.get_test_case(fuzzer_output) + result = self.test_target.get_testcase(fuzzer_output) self.assertTrue(isinstance(result, str)) -class DownloadLatestCorpusUnitTest(unittest.TestCase): - """Test parse_fuzzer_output function in the cifuzz module.""" +class DownloadLatestCorpusTest(unittest.TestCase): + """Tests parse_fuzzer_output.""" def test_download_valid_projects_corpus(self): - """Tests that a vaild fuzz target returns a corpus directory.""" + """Tests that a valid fuzz target returns a corpus directory.""" with tempfile.TemporaryDirectory() as tmp_dir: test_target = fuzz_target.FuzzTarget('testfuzzer', 3, 'test_out') test_target.project_name = EXAMPLE_PROJECT @@ -166,9 +165,9 @@ class DownloadLatestCorpusUnitTest(unittest.TestCase): os.path.join(tmp_dir, 'backup_corpus', EXAMPLE_FUZZER)) def test_download_invalid_projects_corpus(self): - """Tests that a invaild fuzz target does not return None.""" + """Tests that a invade fuzz target does not return None.""" with tempfile.TemporaryDirectory() as tmp_dir: - test_target = fuzz_target.FuzzTarget('testfuzzer', 3, tmp_dir) + test_target = fuzz_target.FuzzTarget('test fuzzer', 3, tmp_dir) corpus_path = test_target.download_latest_corpus() self.assertIsNone(corpus_path) test_target = fuzz_target.FuzzTarget('not_a_fuzzer', 3, tmp_dir, @@ -177,8 +176,8 @@ class DownloadLatestCorpusUnitTest(unittest.TestCase): self.assertIsNone(corpus_path) -class IsCrashReportableUnitTest(fake_filesystem_unittest.TestCase): - """Test is_crash_reportable method of fuzz_target.FuzzTarget.""" +class IsCrashReportableTest(fake_filesystem_unittest.TestCase): + """Tests the is_crash_reportable method of FuzzTarget.""" def setUp(self): """Sets up dummy fuzz target to test is_crash_reportable method.""" @@ -236,8 +235,8 @@ class IsCrashReportableUnitTest(fake_filesystem_unittest.TestCase): @unittest.mock.patch('fuzz_target.FuzzTarget.is_reproducible', return_value=[True]) def test_reproducible_no_oss_fuzz_target(self, _, mocked_info): - """Tests that is_crash_reportable returns True when a crash repros on the - PR build but the target is not in the OSS-Fuzz build (usually because it + """Tests that is_crash_reportable returns True when a crash reproduces on + the PR build but the target is not in the OSS-Fuzz build (usually because it is new).""" os.remove(self.oss_fuzz_target_path) @@ -261,14 +260,14 @@ class IsCrashReportableUnitTest(fake_filesystem_unittest.TestCase): 'Assuming this pull request introduced crash.') -class GetLatestBuildVersionUnitTest(unittest.TestCase): - """Test the get_latest_build_version function in the fuzz_target module.""" +class GetLatestBuildVersionTest(unittest.TestCase): + """Tests the get_latest_build_version function.""" def test_get_valid_project(self): """Tests that the latest build can be retrieved from GCS.""" test_target = fuzz_target.FuzzTarget('/example/path', 10, '/example/outdir', 'example') - latest_build = test_target.get_lastest_build_version() + latest_build = test_target.get_latest_build_version() self.assertIsNotNone(latest_build) self.assertTrue(latest_build.endswith('.zip')) self.assertTrue('address' in latest_build) @@ -277,22 +276,22 @@ class GetLatestBuildVersionUnitTest(unittest.TestCase): """Tests that the latest build returns None when project doesn't exist.""" test_target = fuzz_target.FuzzTarget('/example/path', 10, '/example/outdir', 'not-a-proj') - self.assertIsNone(test_target.get_lastest_build_version()) + self.assertIsNone(test_target.get_latest_build_version()) test_target = fuzz_target.FuzzTarget('/example/path', 10, '/example/outdir') - self.assertIsNone(test_target.get_lastest_build_version()) + self.assertIsNone(test_target.get_latest_build_version()) -class DownloadOSSFuzzBuildDirIntegrationTests(unittest.TestCase): - """Test the download_oss_fuzz_build in function in the fuzz_target module.""" +class DownloadOSSFuzzBuildDirIntegrationTest(unittest.TestCase): + """Tests download_oss_fuzz_build.""" def test_single_download(self): """Tests that the build directory was only downloaded once.""" with tempfile.TemporaryDirectory() as tmp_dir: test_target = fuzz_target.FuzzTarget('/example/do_stuff_fuzzer', 10, tmp_dir, 'example') - latest_version = test_target.get_lastest_build_version() + latest_version = test_target.get_latest_build_version() with unittest.mock.patch( - 'fuzz_target.FuzzTarget.get_lastest_build_version', + 'fuzz_target.FuzzTarget.get_latest_build_version', return_value=latest_version) as mocked_get_latest_build_version: for _ in range(5): oss_fuzz_build_path = test_target.download_oss_fuzz_build() @@ -327,7 +326,7 @@ class DownloadOSSFuzzBuildDirIntegrationTests(unittest.TestCase): class DownloadUrlTest(unittest.TestCase): - """Test that download_url works.""" + """Tests that download_url works.""" URL = 'example.com/file' FILE_PATH = '/tmp/file' @@ -363,8 +362,8 @@ class DownloadUrlTest(unittest.TestCase): self.URL, 3) -class DownloadAndUnpackZipUnitTests(unittest.TestCase): - """Test the download and unpack functionality in the fuzz_target module.""" +class DownloadAndUnpackZipTest(unittest.TestCase): + """Tests download_and_unpack_zip.""" def test_bad_zip_download(self): """Tests download_and_unpack_zip returns none when a bad zip is passed.""" diff --git a/infra/cifuzz/test_files/example_curl_file_list b/infra/cifuzz/test_files/example_curl_file_list Binary files differdeleted file mode 100644 index f9443d84f..000000000 --- a/infra/cifuzz/test_files/example_curl_file_list +++ /dev/null diff --git a/infra/cifuzz/test_files/example_curl_file_list.json b/infra/cifuzz/test_files/example_curl_file_list.json new file mode 100644 index 000000000..3ebd7b64a --- /dev/null +++ b/infra/cifuzz/test_files/example_curl_file_list.json @@ -0,0 +1 @@ +["lib/asyn-thread.c", "lib/base64.c", "lib/conncache.c", "lib/connect.c", "lib/content_encoding.c", "lib/cookie.c", "lib/curl_addrinfo.c", "lib/curl_ctype.c", "lib/curl_endian.c", "lib/curl_fnmatch.c", "lib/curl_get_line.c", "lib/curl_gethostname.c", "lib/curl_memrchr.c", "lib/curl_ntlm_core.c", "lib/curl_ntlm_wb.c", "lib/curl_range.c", "lib/curl_sasl.c", "lib/curl_threads.c", "lib/dict.c", "lib/doh.c", "lib/dotdot.c", "lib/easy.c", "lib/escape.c", "lib/file.c", "lib/fileinfo.c", "lib/formdata.c", "lib/ftp.c", "lib/ftplistparser.c", "lib/getenv.c", "lib/getinfo.c", "lib/gopher.c", "lib/hash.c", "lib/hmac.c", "lib/hostasyn.c", "lib/hostcheck.c", "lib/hostip.c", "lib/hostip6.c", "lib/http.c", "lib/http2.c", "lib/http_chunks.c", "lib/http_digest.c", "lib/http_ntlm.c", "lib/http_proxy.c", "lib/if2ip.c", "lib/imap.c", "lib/llist.c", "lib/md4.c", "lib/md5.c", "lib/memdebug.c", "lib/mime.c", "lib/mprintf.c", "lib/multi.c", "lib/netrc.c", "lib/nonblock.c", "lib/parsedate.c", "lib/pingpong.c", "lib/pop3.c", "lib/progress.c", "lib/rand.c", "lib/rename.c", "lib/rtsp.c", "lib/select.c", "lib/sendf.c", "lib/setopt.c", "lib/sha256.c", "lib/share.c", "lib/sigpipe.h", "lib/slist.c", "lib/smb.c", "lib/smtp.c", "lib/socks.c", "lib/speedcheck.c", "lib/splay.c", "lib/strcase.c", "lib/strdup.c", "lib/strerror.c", "lib/strtoofft.c", "lib/telnet.c", "lib/tftp.c", "lib/timeval.c", "lib/transfer.c", "lib/url.c", "lib/urlapi.c", "lib/vauth/cleartext.c", "lib/vauth/cram.c", "lib/vauth/digest.c", "lib/vauth/ntlm.c", "lib/vauth/oauth2.c", "lib/vauth/vauth.c", "lib/version.c", "lib/vtls/openssl.c", "lib/vtls/vtls.c", "lib/warnless.c", "lib/wildcard.c"]
\ No newline at end of file diff --git a/infra/repo_manager_test.py b/infra/repo_manager_test.py index e8d168c5c..f3294bc2a 100644 --- a/infra/repo_manager_test.py +++ b/infra/repo_manager_test.py @@ -9,14 +9,14 @@ # Unless required by applicable law or agreed to in writing, software # distributed under the License is distributed on an "AS IS" BASIS, # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing perepo_managerissions and +# See the License for the specific language governing permissions and # limitations under the License. """Test the functionality of the RepoManager class.""" import os +import tempfile import unittest from unittest import mock -import tempfile import repo_manager import utils @@ -24,12 +24,8 @@ import utils OSS_FUZZ_REPO = 'https://github.com/google/oss-fuzz' -class TestRepoManager(unittest.TestCase): - """Class to test the functionality of the RepoManager class.""" - - -class RepoManagerCloneUnitTest(unittest.TestCase): - """Class to test the functionality of clone of the RepoManager class.""" +class RepoManagerCloneTest(unittest.TestCase): + """Tests the cloning functionality of RepoManager.""" def test_clone_valid_repo(self): """Tests the correct location of the git repo.""" @@ -41,7 +37,7 @@ class RepoManagerCloneUnitTest(unittest.TestCase): test_repo_manager.remove_repo() def test_clone_invalid_repo(self): - """Test that constructing RepoManager with an invalid repo will fail.""" + """Tests that constructing RepoManager with an invalid repo will fail.""" with tempfile.TemporaryDirectory() as tmp_dir: with self.assertRaises(ValueError): repo_manager.RepoManager(' ', tmp_dir) @@ -52,8 +48,8 @@ class RepoManagerCloneUnitTest(unittest.TestCase): tmp_dir) -class RepoManagerCheckoutUnitTest(unittest.TestCase): - """Class to test the functionality of checkout of the RepoManager class.""" +class RepoManagerCheckoutTest(unittest.TestCase): + """Tests the checkout functionality of RepoManager.""" def test_checkout_valid_commit(self): """Tests that the git checkout command works.""" @@ -76,9 +72,8 @@ class RepoManagerCheckoutUnitTest(unittest.TestCase): test_repo_manager.checkout_commit('not-a-valid-commit') -class RepoManagerGetCommitListUnitTest(unittest.TestCase): - """Class to test the functionality of get commit list in the - RepoManager class.""" +class RepoManagerGetCommitListTest(unittest.TestCase): + """Tests the get_commit_list method of RepoManager.""" def test_get_valid_commit_list(self): """Tests an accurate commit list can be retrieved from the repo manager.""" @@ -95,9 +90,9 @@ class RepoManagerGetCommitListUnitTest(unittest.TestCase): result_list = test_repo_manager.get_commit_list(new_commit, old_commit) self.assertListEqual(commit_list, result_list) - def test_invalid_commit_list(self): - """Tests that the propper Errors are thrown when invalid commits are - passed.""" + def test_get_invalid_commit_list(self): + """Tests that the proper errors are thrown when invalid commits are + passed to get_commit_list.""" with tempfile.TemporaryDirectory() as tmp_dir: old_commit = '04ea24ee15bbe46a19e5da6c5f022a2ffdfbdb3b' new_commit = 'fa662173bfeb3ba08d2e84cefc363be11e6c8463' @@ -107,12 +102,11 @@ class RepoManagerGetCommitListUnitTest(unittest.TestCase): with self.assertRaises(ValueError): test_repo_manager.get_commit_list(new_commit, 'fakecommit') with self.assertRaises(RuntimeError): - # pylint: disable=arguments-out-of-order - test_repo_manager.get_commit_list(old_commit, new_commit) + test_repo_manager.get_commit_list(old_commit, new_commit) # pylint: disable=arguments-out-of-order -class GitDiffUnitTest(unittest.TestCase): - """Class testing functionality of get_git_diff in the repo_manager module.""" +class GitDiffTest(unittest.TestCase): + """Tests get_git_diff.""" def test_diff_exists(self): """Tests that a real diff is returned when a valid repo manager exists.""" @@ -150,8 +144,8 @@ class GitDiffUnitTest(unittest.TestCase): self.assertIsNone(diff) -class CheckoutPRIntegrationTest(unittest.TestCase): - """Class testing functionality of checkout_pr in the repo_manager module.""" +class CheckoutPrIntegrationTest(unittest.TestCase): + """Does Integration tests on the checkout_pr method of RepoManager.""" def test_pull_request_exists(self): """Tests that a diff is returned when a valid PR is checked out.""" diff --git a/infra/test_repos.py b/infra/test_repos.py index 09e6937f4..080ba53eb 100644 --- a/infra/test_repos.py +++ b/infra/test_repos.py @@ -35,12 +35,13 @@ TEST_DIR_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), # WARNING: These tests are dependent upon the following repos existing and # the specified commits existing. +# TODO(metzman): Fix this problem. TEST_REPOS = [ ExampleRepo(project_name='usrsctp', oss_repo_name='usrsctp', git_repo_name='usrsctp', image_location='/src', - git_url='https://github.com/weinrank/usrsctp', + git_url='https://github.com/sctplab/usrsctp', old_commit='4886aaa49fb90e479226fcfc3241d74208908232', new_commit='c710749b1053978179a027973a3ea3bccf20ee5c', intro_commit='457d6ead58e82584d9dcb826f6739347f59ebd3a', diff --git a/infra/utils_test.py b/infra/utils_test.py index 7b822a1b3..0fc614f86 100644 --- a/infra/utils_test.py +++ b/infra/utils_test.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -"""Test the functionality of the utils module's functions""" +"""Tests the functionality of the utils module's functions""" import os import tempfile @@ -26,11 +26,11 @@ TEST_OUT_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'cifuzz', 'test_files', 'out') -class IsFuzzTargetLocalUnitTest(unittest.TestCase): - """Test is_fuzz_target_local function in the utils module.""" +class IsFuzzTargetLocalTest(unittest.TestCase): + """Tests the is_fuzz_target_local function.""" def test_invalid_filepath(self): - """Test the function with an invalid file path.""" + """Tests the function with an invalid file path.""" is_local = utils.is_fuzz_target_local('not/a/real/file') self.assertFalse(is_local) is_local = utils.is_fuzz_target_local('') @@ -48,8 +48,8 @@ class IsFuzzTargetLocalUnitTest(unittest.TestCase): self.assertFalse(is_local) -class GetFuzzTargetsUnitTest(unittest.TestCase): - """Test get_fuzz_targets function in the utils module.""" +class GetFuzzTargetsTest(unittest.TestCase): + """Tests the get_fuzz_targets function.""" def test_valid_filepath(self): """Tests that fuzz targets can be retrieved once the fuzzers are built.""" @@ -70,8 +70,8 @@ class GetFuzzTargetsUnitTest(unittest.TestCase): self.assertFalse(fuzz_targets) -class ExecuteUnitTest(unittest.TestCase): - """Test execute function in the utils module.""" +class ExecuteTest(unittest.TestCase): + """Tests the execute function.""" def test_valid_command(self): """Tests that execute can produce valid output.""" |