Fix bug where region count was used to determine if file was covered

instead of region coverage.
Add a test for this.
Add a remove_prefix util function.

4 files changed, 52 insertions, 7 deletions

diff --git a/infra/cifuzz/coverage.py b/infra/cifuzz/coverage.py
index 56b55427a..6cfea34b2 100644
--- a/infra/cifuzz/coverage.py
+++ b/infra/cifuzz/coverage.py
@@ -64,26 +64,37 @@ class OssFuzzCoverageGetter:
       logging.error('No coverage data for %s', target)
       return None
 
-    coverage_per_file = target_cov['data'][0]['files']
+    coverage_per_file = get_coverage_per_file(target_cov)
     if not coverage_per_file:
       logging.info('No files found in coverage report.')
       return None
 
     affected_file_list = []
-    for file in coverage_per_file:
-      norm_file_path = os.path.normpath(file['filename'])
+    for file_cov in coverage_per_file:
+      norm_file_path = os.path.normpath(file_cov['filename'])
       if not norm_file_path.startswith(self.repo_path):
         continue
-      if not file['summary']['regions']['count']:
+
+      if not is_file_covered(file_cov):
         # Don't consider a file affected if code in it is never executed.
         continue
 
-      relative_path = file['filename'].replace(self.repo_path, '')
+      relative_path = utils.remove_prefix(file_cov['filename'], self.repo_path)
       affected_file_list.append(relative_path)
 
     return affected_file_list
 
 
+def is_file_covered(file_cov):
+  """Returns whether the file is covered."""
+  return file_cov['summary']['regions']['covered']
+
+
+def get_coverage_per_file(target_cov):
+  """Returns the coverage per file within |target_cov|."""
+  return target_cov['data'][0]['files']
+
+
 def _normalize_repo_path(repo_path):
   """Normalizes and returns |repo_path| to make sure cases like /src/curl and
   /src/curl/ are both handled."""
diff --git a/infra/cifuzz/coverage_test.py b/infra/cifuzz/coverage_test.py
index 0b4f18d44..1271bca1e 100644
--- a/infra/cifuzz/coverage_test.py
+++ b/infra/cifuzz/coverage_test.py
@@ -115,5 +115,34 @@ class GetFilesCoveredByTargetTest(unittest.TestCase):
     self.assertCountEqual(file_list, true_files_list)
 
 
+class IsFileCoveredTest(unittest.TestCase):
+  """Tests for is_file_covered."""
+  def test_is_file_covered_covered(self):
+    """Tests that is_file_covered returns True for a covered file."""
+    file_coverage = {
+        'filename': '/src/systemd/src/basic/locale-util.c',
+        'summary': {
+            'regions': {
+                'count': 204, 'covered': 200, 'notcovered': 200,
+                'percent': 98.03
+            }
+        }
+    }
+    self.assertTrue(coverage.is_file_covered(file_coverage))
+
+  def test_is_file_covered_covered(self):
+    """Tests that is_file_covered returns False for a noncovered file."""
+    file_coverage = {
+        'filename': '/src/systemd/src/basic/locale-util.c',
+        'summary': {
+            'regions': {
+                'count': 204, 'covered': 0, 'notcovered': 0,
+                'percent': 0
+            }
+        }
+    }
+    self.assertFalse(coverage.is_file_covered(file_coverage))
+
+
 if __name__ == '__main__':
   unittest.main()
diff --git a/infra/cifuzz/test_files/example_curl_file_list.json b/infra/cifuzz/test_files/example_curl_file_list.json
index 3ebd7b64a..a79bcec5a 100644
--- a/infra/cifuzz/test_files/example_curl_file_list.json
+++ b/infra/cifuzz/test_files/example_curl_file_list.json
@@ -1 +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
+["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_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/dotdot.c", "lib/easy.c", "lib/escape.c", "lib/file.c", "lib/fileinfo.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/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/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/share.c", "lib/sigpipe.h", "lib/slist.c", "lib/smb.c", "lib/smtp.c", "lib/speedcheck.c", "lib/splay.c", "lib/strcase.c", "lib/strdup.c", "lib/strerror.c", "lib/strtoofft.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/version.c", "lib/vtls/openssl.c", "lib/vtls/vtls.c", "lib/warnless.c", "lib/wildcard.c"]
diff --git a/infra/utils.py b/infra/utils.py
index 795fbcddf..d8ade3891 100644
--- a/infra/utils.py
+++ b/infra/utils.py
@@ -158,4 +158,9 @@ def url_join(*url_parts):
 
 def gs_url_to_https(url):
   """Converts |url| from a GCS URL (beginning with 'gs://') to an HTTPS one."""
-  return url.replace('gs://', GCS_BASE_URL)
+  return url_join(GCS_BASE_URL, remove_prefix(url, 'gs://'))
+
+
+def remove_prefix(string, prefix):
+  """Returns |string| without the leading substring |prefix|."""
+  return string[len(prefix):]