bot: Check path is supported and not 3rd party before sending a warning, fixes #200

Author: Bastien Abadie

bot/code_coverage_bot/phabricator.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 
+import os
 import re
 
 import structlog
@@ -9,6 +10,7 @@
 
 from code_coverage_bot import hgmo
 from code_coverage_bot.secrets import secrets
+from code_coverage_tools import COVERAGE_EXTENSIONS
 
 logger = structlog.get_logger(__name__)
 
@@ -29,6 +31,16 @@ def __init__(self, repo_dir, revision):
         self.repo_dir = repo_dir
         self.revision = revision
 
+        # Read third party exclusion lists from repo
+        third_parties = os.path.join(
+            self.repo_dir, "tools/rewriting/ThirdPartyPaths.txt"
+        )
+        if os.path.exists(third_parties):
+            self.third_parties = [line.rstrip() for line in open(third_parties)]
+        else:
+            self.third_parties = []
+            logger.warn("Missing third party exclusion list", path=third_parties)
+
     def _find_coverage(self, report, path):
         """
         Find coverage value in a covdir report
@@ -38,7 +50,13 @@ def _find_coverage(self, report, path):
         parts = path.split("/")
         for part in filter(None, parts):
             if part not in report["children"]:
-                logger.warn("Path {} not found in report".format(path))
+                # Only send warning for supported extensions
+                if self.is_supported_extension(path):
+                    logger.warn("Path {} not found in report".format(path))
+                else:
+                    logger.info(
+                        "Path not found in report for unsupported extension", path=path
+                    )
                 return
             report = report["children"][part]
 
@@ -95,6 +113,24 @@ def _apply_coverage_map(self, annotate, coverage_map):
 
         return phab_coverage_data
 
+    def is_third_party(self, path):
+        """
+        Check a file against known list of third party paths
+        """
+        for third_party in self.third_parties:
+            if path.startswith(third_party):
+                return True
+        return False
+
+    def is_supported_extension(self, path):
+        """
+        Check a file has a supported extension
+        """
+        _, ext = os.path.splitext(path)
+        if not ext:
+            return False
+        return ext[1:] in COVERAGE_EXTENSIONS
+
     def generate(self, report, changesets):
         results = {}
 
@@ -109,6 +145,12 @@ def generate(self, report, changesets):
 
                 # For each file...
                 for path in changeset["files"]:
+
+                    # Skip third party files
+                    if self.is_third_party(path):
+                        logger.info("Skip third party path", path=path)
+                        continue
+
                     # Retrieve the coverage data.
                     coverage_record = self._find_coverage(report, path)
                     if coverage_record is None:

bot/tests/conftest.py

@@ -25,6 +25,7 @@ def copy_pushlog_database(remote, local):
 
 def add_file(hg, repo_dir, name, contents):
     path = os.path.join(repo_dir, name)
+    os.makedirs(os.path.dirname(path), exist_ok=True)
 
     with open(path, "w") as f:
         f.write(contents)

bot/tests/test_phabricator.py

@@ -317,3 +317,61 @@ def test_backout_removed_file(mock_secrets, fake_hg_repo):
         1: {"file": {"coverage": "NUCCCCU", "lines_added": 7, "lines_covered": 5}},
         2: {},
     }
+
+
+def test_third_party(mock_secrets, fake_hg_repo):
+    hg, local, remote = fake_hg_repo
+
+    add_file(hg, local, "tools/rewriting/ThirdPartyPaths.txt", "third_party\nsome/path")
+    revision = commit(hg, 1)
+
+    phabricator = PhabricatorUploader(local, revision)
+
+    assert phabricator.third_parties == ["third_party", "some/path"]
+
+    assert phabricator.is_third_party("js/src/xx.cpp") is False
+    assert phabricator.is_third_party("dom/media/yyy.h") is False
+    assert phabricator.is_third_party("third_party/test.cpp") is True
+    assert phabricator.is_third_party("some/test.cpp") is False
+    assert phabricator.is_third_party("some/path/test.cpp") is True
+
+
+def test_supported_extensions(mock_secrets, fake_hg_repo):
+    hg, local, remote = fake_hg_repo
+
+    add_file(hg, local, "file", "1\n2\n3\n4\n5\n6\n7\n")
+    revision = commit(hg, 1)
+
+    phabricator = PhabricatorUploader(local, revision)
+
+    assert phabricator.is_supported_extension("README") is False
+    assert phabricator.is_supported_extension("requirements.txt") is False
+    assert phabricator.is_supported_extension("tools/Cargo.toml") is False
+    assert phabricator.is_supported_extension("tools/Cargo.lock") is False
+    assert phabricator.is_supported_extension("rust/code.rs") is False
+    assert phabricator.is_supported_extension("dom/feature.idl") is False
+    assert phabricator.is_supported_extension("dom/feature.webidl") is False
+    assert phabricator.is_supported_extension("xpcom/moz.build") is False
+    assert phabricator.is_supported_extension("payload.json") is False
+    assert phabricator.is_supported_extension("inline.patch") is False
+    assert phabricator.is_supported_extension("README.mozilla") is False
+    assert phabricator.is_supported_extension("config.yml") is False
+    assert phabricator.is_supported_extension("config.yaml") is False
+    assert phabricator.is_supported_extension("config.ini") is False
+    assert phabricator.is_supported_extension("tooling.py") is False
+
+    assert phabricator.is_supported_extension("test.cpp") is True
+    assert phabricator.is_supported_extension("some/path/to/test.cpp") is True
+    assert phabricator.is_supported_extension("xxxYYY.h") is True
+    assert phabricator.is_supported_extension("test.c") is True
+    assert phabricator.is_supported_extension("test.cc") is True
+    assert phabricator.is_supported_extension("test.cxx") is True
+    assert phabricator.is_supported_extension("test.hh") is True
+    assert phabricator.is_supported_extension("test.hpp") is True
+    assert phabricator.is_supported_extension("test.hxx") is True
+    assert phabricator.is_supported_extension("test.js") is True
+    assert phabricator.is_supported_extension("test.jsm") is True
+    assert phabricator.is_supported_extension("test.xul") is True
+    assert phabricator.is_supported_extension("test.xml") is True
+    assert phabricator.is_supported_extension("test.html") is True
+    assert phabricator.is_supported_extension("test.xhtml") is True