diff options
| -rw-r--r-- | infra/cifuzz/cifuzz.py | 9 | ||||
| -rw-r--r-- | infra/cifuzz/cifuzz_test.py | 93 | ||||
| -rwxr-xr-x | infra/presubmit.py | 3 |
3 files changed, 41 insertions, 64 deletions
diff --git a/infra/cifuzz/cifuzz.py b/infra/cifuzz/cifuzz.py index 319a254b..660075bf 100644 --- a/infra/cifuzz/cifuzz.py +++ b/infra/cifuzz/cifuzz.py @@ -255,7 +255,6 @@ def check_fuzzer_build(out_dir, sanitizer='address'): Returns: True if fuzzers are correct. """ - if not os.path.exists(out_dir): logging.error('Invalid out directory: %s.', out_dir) return False @@ -267,14 +266,6 @@ def check_fuzzer_build(out_dir, sanitizer='address'): '--cap-add', 'SYS_PTRACE', '-e', 'FUZZING_ENGINE=' + DEFAULT_ENGINE, '-e', 'SANITIZER=' + sanitizer, '-e', 'ARCHITECTURE=' + DEFAULT_ARCHITECTURE ] - - allowed_broken_targets_percentage = os.getenv( - 'ALLOWED_BROKEN_TARGETS_PERCENTAGE') - if allowed_broken_targets_percentage is not None: - set_env_var_arg = ('ALLOWED_BROKEN_TARGETS_PERCENTAGE=' + - allowed_broken_targets_percentage) - command += ['-e', set_env_var_arg] - container = utils.get_container_name() if container: command += ['-e', 'OUT=' + out_dir, '--volumes-from', container] diff --git a/infra/cifuzz/cifuzz_test.py b/infra/cifuzz/cifuzz_test.py index 99fdecc6..2da8b5ff 100644 --- a/infra/cifuzz/cifuzz_test.py +++ b/infra/cifuzz/cifuzz_test.py @@ -22,7 +22,7 @@ import shutil import sys import tempfile import unittest -from unittest import mock +import unittest.mock # pylint: disable=wrong-import-position sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) @@ -56,8 +56,6 @@ MEMORY_FUZZER = 'curl_fuzzer_memory' UNDEFINED_FUZZER_DIR = os.path.join(TEST_FILES_PATH, 'out', 'undefined') UNDEFINED_FUZZER = 'curl_fuzzer_undefined' -# pylint: disable=no-self-use - class BuildFuzzersIntegrationTest(unittest.TestCase): """Test build_fuzzers function in the utils module.""" @@ -215,12 +213,12 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase): # 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. - with mock.patch.object(fuzz_target.FuzzTarget, - 'is_reproducible', - side_effect=[True, False]), mock.patch.object( - cifuzz, - 'is_project_sanitizer', - return_value=True): + with unittest.mock.patch.object( + fuzz_target.FuzzTarget, 'is_reproducible', + side_effect=[True, + False]), unittest.mock.patch.object(cifuzz, + 'is_project_sanitizer', + return_value=True): run_success, bug_found = cifuzz.run_fuzzers(10, TEST_FILES_PATH, EXAMPLE_PROJECT) build_dir = os.path.join(TEST_FILES_PATH, 'out', 'oss_fuzz_latest') @@ -231,12 +229,12 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase): def test_old_bug_found(self): """Test run_fuzzers with a bug found in OSS-Fuzz before.""" - with mock.patch.object(fuzz_target.FuzzTarget, - 'is_reproducible', - side_effect=[True, True]), mock.patch.object( - cifuzz, - 'is_project_sanitizer', - return_value=True): + with unittest.mock.patch.object( + fuzz_target.FuzzTarget, 'is_reproducible', + side_effect=[True, + True]), unittest.mock.patch.object(cifuzz, + 'is_project_sanitizer', + return_value=True): run_success, bug_found = cifuzz.run_fuzzers(10, TEST_FILES_PATH, EXAMPLE_PROJECT) build_dir = os.path.join(TEST_FILES_PATH, 'out', 'oss_fuzz_latest') @@ -247,7 +245,7 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase): def test_invalid_build(self): """Test run_fuzzers with an invalid build.""" - with tempfile.TemporaryDirectory() as tmp_dir, mock.patch.object( + with tempfile.TemporaryDirectory() as tmp_dir, unittest.mock.patch.object( cifuzz, 'is_project_sanitizer', return_value=True): out_path = os.path.join(tmp_dir, 'out') os.mkdir(out_path) @@ -257,7 +255,7 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase): def test_invalid_fuzz_seconds(self): """Tests run_fuzzers with an invalid fuzz seconds.""" - with tempfile.TemporaryDirectory() as tmp_dir, mock.patch.object( + with tempfile.TemporaryDirectory() as tmp_dir, unittest.mock.patch.object( cifuzz, 'is_project_sanitizer', return_value=True): out_path = os.path.join(tmp_dir, 'out') os.mkdir(out_path) @@ -317,19 +315,6 @@ class CheckFuzzerBuildUnitTest(unittest.TestCase): """Checks a directory that exists but does not have fuzzers is False.""" self.assertFalse(cifuzz.check_fuzzer_build(TEST_FILES_PATH)) - # TODO(metzman): Make it easier to patch environ without thinking about it. - @mock.patch.dict(os.environ, {'ALLOWED_BROKEN_TARGETS_PERCENTAGE': '0'}) - @mock.patch('helper.docker_run') - 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 - 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.""" @@ -350,9 +335,10 @@ class GetFilesCoveredByTargetUnitTest(unittest.TestCase): def test_valid_target(self): """Tests that covered files can be retrieved from a coverage report.""" - with mock.patch.object(cifuzz, - 'get_target_coverage_report', - return_value=self.fuzzer_cov_report_example): + with unittest.mock.patch.object( + cifuzz, + 'get_target_coverage_report', + return_value=self.fuzzer_cov_report_example): file_list = cifuzz.get_files_covered_by_target( self.proj_cov_report_example, self.example_fuzzer, '/src/curl') @@ -393,8 +379,9 @@ class GetTargetCoverageReporUnitTest(unittest.TestCase): def test_valid_target(self): """Test 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: + with unittest.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) (url,), _ = mock_get_json.call_args self.assertEqual( @@ -426,8 +413,9 @@ class GetLatestCoverageReportUnitTest(unittest.TestCase): NOTE: This test relies on the test_project repo's coverage report. Example was not used because it has no coverage reports. """ - with mock.patch.object(cifuzz, 'get_json_from_url', - return_value='{}') as mock_fun: + with unittest.mock.patch.object(cifuzz, + 'get_json_from_url', + return_value='{}') as mock_fun: cifuzz.get_latest_cov_report_info(self.test_project) (url,), _ = mock_fun.call_args @@ -450,51 +438,52 @@ class KeepAffectedFuzzersUnitTest(unittest.TestCase): 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( + with tempfile.TemporaryDirectory() as tmp_dir, unittest.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]): + with unittest.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( + with tempfile.TemporaryDirectory() as tmp_dir, unittest.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']]): + with unittest.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( + with tempfile.TemporaryDirectory() as tmp_dir, unittest.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=[None, None]): + with unittest.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( + with tempfile.TemporaryDirectory() as tmp_dir, unittest.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( + with unittest.mock.patch.object( cifuzz, 'get_files_covered_by_target', side_effect=[self.example_file_changed, self.example_file_changed]): diff --git a/infra/presubmit.py b/infra/presubmit.py index f8cf0110..cbcbc0bc 100755 --- a/infra/presubmit.py +++ b/infra/presubmit.py @@ -334,9 +334,6 @@ def run_tests(): for file in get_changed_files(): changed_dirs.add(os.path.dirname(file)) - # TODO(metzman): This approach for running tests is probably flawed since - # tests can fail even if their directory isn't changed. Figure out if it is - # needed (to save time) and remove it if it isn't. suite_list = [] for change_dir in changed_dirs: suite_list.append(unittest.TestLoader().discover(change_dir, |