From baef16e57cb0da77aff310df15d16e082123186a Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Fri, 27 Sep 2019 10:29:51 +0200 Subject: [PATCH 1/6] backend: move COVERAGE_EXTENSIONS in tools --- backend/code_coverage_backend/api.py | 2 +- backend/code_coverage_backend/config.py | 19 ------------------- tools/code_coverage_tools/__init__.py | 21 +++++++++++++++++++++ 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/backend/code_coverage_backend/api.py b/backend/code_coverage_backend/api.py index 4d46da2a2..cd21c603d 100644 --- a/backend/code_coverage_backend/api.py +++ b/backend/code_coverage_backend/api.py @@ -6,9 +6,9 @@ import structlog from flask import abort -from code_coverage_backend.config import COVERAGE_EXTENSIONS from code_coverage_backend.gcp import load_cache from code_coverage_backend.report import DEFAULT_FILTER +from code_coverage_tools import COVERAGE_EXTENSIONS DEFAULT_REPOSITORY = "mozilla-central" logger = structlog.get_logger(__name__) diff --git a/backend/code_coverage_backend/config.py b/backend/code_coverage_backend/config.py index 9a084a9bc..ac46c032e 100644 --- a/backend/code_coverage_backend/config.py +++ b/backend/code_coverage_backend/config.py @@ -7,22 +7,3 @@ PROJECT_NAME = "code-coverage-backend" APP_NAME = "code_coverage_backend" -COVERAGE_EXTENSIONS = [ - # C - "c", - "h", - # C++ - "cpp", - "cc", - "cxx", - "hh", - "hpp", - "hxx", - # JavaScript - "js", - "jsm", - "xul", - "xml", - "html", - "xhtml", -] diff --git a/tools/code_coverage_tools/__init__.py b/tools/code_coverage_tools/__init__.py index e69de29bb..feef1ddb8 100644 --- a/tools/code_coverage_tools/__init__.py +++ b/tools/code_coverage_tools/__init__.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- + +COVERAGE_EXTENSIONS = [ + # C + "c", + "h", + # C++ + "cpp", + "cc", + "cxx", + "hh", + "hpp", + "hxx", + # JavaScript + "js", + "jsm", + "xul", + "xml", + "html", + "xhtml", +] From 509a1fd04a28c85f53b37bb93b9143013f672af1 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Fri, 27 Sep 2019 10:53:11 +0200 Subject: [PATCH 2/6] bot: Check path is supported and not 3rd party before sending a warning, fixes #200 --- bot/code_coverage_bot/phabricator.py | 44 ++++++++++++++++++++- bot/tests/conftest.py | 1 + bot/tests/test_phabricator.py | 58 ++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 1 deletion(-) diff --git a/bot/code_coverage_bot/phabricator.py b/bot/code_coverage_bot/phabricator.py index ee40f428d..b7e61c0d9 100644 --- a/bot/code_coverage_bot/phabricator.py +++ b/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: diff --git a/bot/tests/conftest.py b/bot/tests/conftest.py index db2f0163e..ca90c44ba 100644 --- a/bot/tests/conftest.py +++ b/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) diff --git a/bot/tests/test_phabricator.py b/bot/tests/test_phabricator.py index 4090819c8..7f608540c 100644 --- a/bot/tests/test_phabricator.py +++ b/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 From 0add4f05909ad0a799f88d6049d9811dd2255665 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Fri, 27 Sep 2019 12:27:05 +0200 Subject: [PATCH 3/6] Set Rust as supported extension --- bot/tests/test_phabricator.py | 2 +- tools/code_coverage_tools/__init__.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bot/tests/test_phabricator.py b/bot/tests/test_phabricator.py index 7f608540c..76c0e6104 100644 --- a/bot/tests/test_phabricator.py +++ b/bot/tests/test_phabricator.py @@ -348,7 +348,6 @@ def test_supported_extensions(mock_secrets, fake_hg_repo): 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 @@ -375,3 +374,4 @@ def test_supported_extensions(mock_secrets, fake_hg_repo): 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 + assert phabricator.is_supported_extension("test.rs") is True diff --git a/tools/code_coverage_tools/__init__.py b/tools/code_coverage_tools/__init__.py index feef1ddb8..804d1b0bf 100644 --- a/tools/code_coverage_tools/__init__.py +++ b/tools/code_coverage_tools/__init__.py @@ -18,4 +18,6 @@ "xml", "html", "xhtml", + # Rust + "rs", ] From e1252ca3028541b54cbb58374264ba194ec01708 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Fri, 27 Sep 2019 12:31:44 +0200 Subject: [PATCH 4/6] bot: Allow reporting of 3rd party --- bot/code_coverage_bot/phabricator.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/bot/code_coverage_bot/phabricator.py b/bot/code_coverage_bot/phabricator.py index b7e61c0d9..01cda3b0c 100644 --- a/bot/code_coverage_bot/phabricator.py +++ b/bot/code_coverage_bot/phabricator.py @@ -50,13 +50,15 @@ def _find_coverage(self, report, path): parts = path.split("/") for part in filter(None, parts): if part not in report["children"]: - # Only send warning for supported extensions - if self.is_supported_extension(path): - logger.warn("Path {} not found in report".format(path)) - else: + # Only send warning for non 3rd party + supported extensions + if self.is_third_party(path): + logger.info("Path not found in report for third party", path=path) + elif not self.is_supported_extension(path): logger.info( "Path not found in report for unsupported extension", path=path ) + else: + logger.warn("Path not found in report", path=path) return report = report["children"][part] @@ -146,11 +148,6 @@ 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: From c18ab84ccff54501306824bff7295c39d83835fa Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Fri, 27 Sep 2019 12:33:58 +0200 Subject: [PATCH 5/6] Remove extra new line --- bot/code_coverage_bot/phabricator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bot/code_coverage_bot/phabricator.py b/bot/code_coverage_bot/phabricator.py index 01cda3b0c..e17d97655 100644 --- a/bot/code_coverage_bot/phabricator.py +++ b/bot/code_coverage_bot/phabricator.py @@ -147,7 +147,6 @@ def generate(self, report, changesets): # For each file... for path in changeset["files"]: - # Retrieve the coverage data. coverage_record = self._find_coverage(report, path) if coverage_record is None: From 0429116fad66754504374e8abf7d7dbf73f2d658 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Fri, 27 Sep 2019 12:41:07 +0200 Subject: [PATCH 6/6] Add rust in backend check --- backend/tests/test_coverage.py | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/tests/test_coverage.py b/backend/tests/test_coverage.py index 5e31c0e9f..e92320007 100644 --- a/backend/tests/test_coverage.py +++ b/backend/tests/test_coverage.py @@ -23,6 +23,7 @@ def test_coverage_supported_extensions_api(client): "hxx", "js", "jsm", + "rs", "xul", "xml", "html",