[CIFuzz] Support ALLOWED_BROKEN_TARGETS_PERCENTAGE

3 files changed, 64 insertions, 41 deletions

diff --git a/infra/cifuzz/cifuzz.py b/infra/cifuzz/cifuzz.py
index 660075bf..319a254b 100644
--- a/infra/cifuzz/cifuzz.py
+++ b/infra/cifuzz/cifuzz.py
@@ -255,6 +255,7 @@ 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
@@ -266,6 +267,14 @@ 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 2da8b5ff..99fdecc6 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
-import unittest.mock
+from unittest import mock
 
 # pylint: disable=wrong-import-position
 sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
@@ -56,6 +56,8 @@ 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."""
@@ -213,12 +215,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 unittest.mock.patch.object(
-        fuzz_target.FuzzTarget, 'is_reproducible',
-        side_effect=[True,
-                     False]), unittest.mock.patch.object(cifuzz,
-                                                         'is_project_sanitizer',
-                                                         return_value=True):
+    with mock.patch.object(fuzz_target.FuzzTarget,
+                           'is_reproducible',
+                           side_effect=[True, False]), 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')
@@ -229,12 +231,12 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase):
 
   def test_old_bug_found(self):
     """Test run_fuzzers with a bug found in OSS-Fuzz before."""
-    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):
+    with mock.patch.object(fuzz_target.FuzzTarget,
+                           'is_reproducible',
+                           side_effect=[True, True]), 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')
@@ -245,7 +247,7 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase):
 
   def test_invalid_build(self):
     """Test run_fuzzers with an invalid build."""
-    with tempfile.TemporaryDirectory() as tmp_dir, unittest.mock.patch.object(
+    with tempfile.TemporaryDirectory() as tmp_dir, mock.patch.object(
         cifuzz, 'is_project_sanitizer', return_value=True):
       out_path = os.path.join(tmp_dir, 'out')
       os.mkdir(out_path)
@@ -255,7 +257,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, unittest.mock.patch.object(
+    with tempfile.TemporaryDirectory() as tmp_dir, mock.patch.object(
         cifuzz, 'is_project_sanitizer', return_value=True):
       out_path = os.path.join(tmp_dir, 'out')
       os.mkdir(out_path)
@@ -315,6 +317,19 @@ 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."""
@@ -335,10 +350,9 @@ class GetFilesCoveredByTargetUnitTest(unittest.TestCase):
   def test_valid_target(self):
     """Tests that covered files can be retrieved from a coverage report."""
 
-    with unittest.mock.patch.object(
-        cifuzz,
-        'get_target_coverage_report',
-        return_value=self.fuzzer_cov_report_example):
+    with 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')
 
@@ -379,9 +393,8 @@ class GetTargetCoverageReporUnitTest(unittest.TestCase):
 
   def test_valid_target(self):
     """Test a target's coverage report can be downloaded and parsed."""
-    with unittest.mock.patch.object(cifuzz,
-                                    'get_json_from_url',
-                                    return_value='{}') as mock_get_json:
+    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)
       (url,), _ = mock_get_json.call_args
       self.assertEqual(
@@ -413,9 +426,8 @@ 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 unittest.mock.patch.object(cifuzz,
-                                    'get_json_from_url',
-                                    return_value='{}') as mock_fun:
+    with 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
@@ -438,52 +450,51 @@ 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, unittest.mock.patch.object(
+    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 unittest.mock.patch.object(cifuzz,
-                                      'get_files_covered_by_target',
-                                      side_effect=[[self.example_file_changed],
-                                                   None]):
+      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, unittest.mock.patch.object(
+    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 unittest.mock.patch.object(cifuzz,
-                                      'get_files_covered_by_target',
-                                      side_effect=[[self.example_file_changed],
-                                                   ['not/a/real/file']]):
+      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, unittest.mock.patch.object(
+    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 unittest.mock.patch.object(cifuzz,
-                                      'get_files_covered_by_target',
-                                      side_effect=[None, None]):
+      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, unittest.mock.patch.object(
+    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 unittest.mock.patch.object(
+      with 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 cbcbc0bc..f8cf0110 100755
--- a/infra/presubmit.py
+++ b/infra/presubmit.py
@@ -334,6 +334,9 @@ 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,