[infra] Presubmit fix - test (#3443)

Fixes issue that presubmit test was not returning failure even when a test failed.

.travis.yml

@@ -17,8 +17,6 @@ matrix:
       install:
         - pip install -r infra/dev-requirements.txt
       script: ./infra/presubmit.py
-    - name: "tests"
-      script: sudo ./infra/presubmit.py test
     - name: "libfuzzer address x86_64"
       env:
         - TRAVIS_ENGINE=libfuzzer
@@ -54,6 +52,8 @@ matrix:
         - TRAVIS_ENGINE=dataflow
         - TRAVIS_SANITIZER=dataflow
         - TRAVIS_ARCHITECTURE=x86_64
+    - name: "infra-tests"
+      script: sudo ./infra/presubmit.py infra-tests
 
 script: ./infra/travis/travis_build.py
 

infra/cifuzz/fuzz_target_test.py

@@ -33,7 +33,11 @@ import utils
 EXAMPLE_PROJECT = 'example'
 
 # An example fuzzer that triggers an error.
-EXAMPLE_FUZZER = 'do_stuff_fuzzer'
+EXAMPLE_FUZZER = 'example_crash_fuzzer'
+
+# Location of files used for testing.
+TEST_FILES_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)),
+                               'test_files')
 
 
 class IsReproducibleUnitTest(unittest.TestCase):
@@ -51,8 +55,7 @@ class IsReproducibleUnitTest(unittest.TestCase):
                                     'execute',
                                     side_effect=test_all_success) as patch:
       self.assertTrue(
-          self.test_target.is_reproducible('/fake/path/to/testcase',
-                                           '/fake/target'))
+          self.test_target.is_reproducible(TEST_FILES_PATH, '/not/exist'))
       self.assertEqual(1, patch.call_count)
 
     test_one_success = [(0, 0, 0)] * 9 + [(0, 0, 1)]
@@ -60,8 +63,7 @@ class IsReproducibleUnitTest(unittest.TestCase):
                                     'execute',
                                     side_effect=test_one_success) as patch:
       self.assertTrue(
-          self.test_target.is_reproducible('/fake/path/to/testcase',
-                                           '/fake/target'))
+          self.test_target.is_reproducible(TEST_FILES_PATH, '/not/exist'))
       self.assertEqual(10, patch.call_count)
 
   def test_with_not_reproducible(self):
@@ -70,8 +72,7 @@ class IsReproducibleUnitTest(unittest.TestCase):
     with unittest.mock.patch.object(utils, 'execute',
                                     side_effect=test_all_fail) as patch:
       self.assertFalse(
-          self.test_target.is_reproducible('/fake/path/to/testcase',
-                                           '/fake/target'))
+          self.test_target.is_reproducible(TEST_FILES_PATH, '/not/exist'))
       self.assertEqual(10, patch.call_count)
 
 
@@ -86,7 +87,8 @@ class GetTestCaseUnitTest(unittest.TestCase):
   def test_with_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_fuzzer_output.txt')
+                                  'test_files',
+                                  'example_crash_fuzzer_output.txt')
     with open(test_case_path, 'r') as test_fuzz_output:
       parsed_test_case = self.test_target.get_test_case(test_fuzz_output.read())
     self.assertEqual(
@@ -118,7 +120,7 @@ class DownloadLatestCorpusUnitTest(unittest.TestCase):
             url,
             'https://storage.googleapis.com/example-backup.' \
             'clusterfuzz-external.appspot.com/corpus/libFuzzer/' \
-            'example_do_stuff_fuzzer/public.zip'
+            'example_crash_fuzzer/public.zip'
         )
         self.assertEqual(out_dir,
                          os.path.join(tmp_dir, 'backup_corpus', EXAMPLE_FUZZER))

infra/presubmit.py

@@ -326,12 +326,14 @@ def run_tests():
   changed_dirs = set()
   for file in get_changed_files():
     changed_dirs.add(os.path.dirname(file))
+
   suite_list = []
   for change_dir in changed_dirs:
     suite_list.append(unittest.TestLoader().discover(change_dir,
                                                      pattern='*_test.py'))
   full_suite = unittest.TestSuite(suite_list)
-  unittest.TextTestRunner().run(full_suite)
+  result = unittest.TextTestRunner().run(full_suite)
+  return not result.failures
 
 
 def main():
@@ -339,7 +341,7 @@ def main():
   # Get program arguments.
   parser = argparse.ArgumentParser(description='Presubmit script for oss-fuzz.')
   parser.add_argument('command',
-                      choices=['format', 'lint', 'license', 'test'],
+                      choices=['format', 'lint', 'license', 'infra-tests'],
                       nargs='?')
   args = parser.parse_args()
 
@@ -360,8 +362,8 @@ def main():
     success = check_license(changed_files)
     return bool_to_returncode(success)
 
-  if args.command == 'test':
-    return run_tests()
+  if args.command == 'infra-tests':
+    return bool_to_returncode(run_tests())
 
   # Do all the checks (but no tests).
   success = do_checks(changed_files)

infra/utils_test.py

@@ -42,7 +42,7 @@ class IsFuzzTargetLocalUnitTest(unittest.TestCase):
     """Checks is_fuzz_target_local function with a valid filepath."""
 
     is_local = utils.is_fuzz_target_local(
-        os.path.join(TEST_OUT_DIR, 'do_stuff_fuzzer'))
+        os.path.join(TEST_OUT_DIR, 'example_crash_fuzzer'))
     self.assertTrue(is_local)
     is_local = utils.is_fuzz_target_local(TEST_OUT_DIR)
     self.assertFalse(is_local)
@@ -54,8 +54,10 @@ class GetFuzzTargetsUnitTest(unittest.TestCase):
   def test_valid_filepath(self):
     """Tests that fuzz targets can be retrieved once the fuzzers are built."""
     fuzz_targets = utils.get_fuzz_targets(TEST_OUT_DIR)
+    crash_fuzzer_path = os.path.join(TEST_OUT_DIR, 'example_crash_fuzzer')
+    nocrash_fuzzer_path = os.path.join(TEST_OUT_DIR, 'example_nocrash_fuzzer')
     self.assertCountEqual(fuzz_targets,
-                          [os.path.join(TEST_OUT_DIR, 'do_stuff_fuzzer')])
+                          [crash_fuzzer_path, nocrash_fuzzer_path])
     fuzz_targets = utils.get_fuzz_targets(
         os.path.join(helper.OSSFUZZ_DIR, 'infra'))
     self.assertFalse(fuzz_targets)