diff --git a/bot/code_review_bot/revisions.py b/bot/code_review_bot/revisions.py index af7c2aa8..1bf13628 100644 --- a/bot/code_review_bot/revisions.py +++ b/bot/code_review_bot/revisions.py @@ -375,19 +375,6 @@ class Revision(object): return any(_is_idl(f) for f in self.files) - @property - def has_infer_files(self): - """ - Check if this revision has any file that might - be a Java file - """ - - def _is_infer(filename): - _, ext = os.path.splitext(filename) - return ext.lower() in settings.java_extensions - - return any(_is_infer(f) for f in self.files) - def add_improvement_patch(self, analyzer, content): """ Save an improvement patch, and make it available diff --git a/bot/code_review_bot/tasks/coverity.py b/bot/code_review_bot/tasks/coverity.py deleted file mode 100644 index 8d448b1c..00000000 --- a/bot/code_review_bot/tasks/coverity.py +++ /dev/null @@ -1,147 +0,0 @@ -# -*- coding: utf-8 -*- -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. - -import structlog - -from code_review_bot import Issue -from code_review_bot import Level -from code_review_bot import Reliability -from code_review_bot.tasks.base import AnalysisTask - -logger = structlog.get_logger(__name__) - -ISSUE_MARKDOWN = """ -## coverity error - -- **Message**: {message} -- **Location**: {location} -- **Coverity check**: {check} -- **Publishable **: {publishable} -- **Is Local**: {is_local} -- **Reliability**: {reliability} (false positive risk) -""" - -ISSUE_ELEMENT_IN_STACK = """ -- //{file_path}:{line_number}//: --- `{path_type}: {description}`. -""" - -ISSUE_RELATION = """ -The path that leads to this defect is: -""" - - -class CoverityIssue(Issue): - """ - An issue reported by coverity - """ - - def __init__(self, analyzer, revision, issue, file_path): - super().__init__( - analyzer, - revision, - file_path, - line=issue["line"], - nb_lines=1, - check=issue["flag"], - level=Level.Warning, - message=issue["message"], - ) - self.reliability = ( - Reliability(issue["reliability"]) - if "reliability" in issue - else Reliability.Unknown - ) - - self.state_on_server = issue["extra"]["stateOnServer"] - - # If we have `stack` in the `try` result then embed it in the message. - if "stack" in issue["extra"]: - self.message += ISSUE_RELATION - stack = issue["extra"]["stack"] - for event in stack: - # When an event has `path_type` of `caretline` we skip it. - if event["path_type"] == "caretline": - continue - self.message += ISSUE_ELEMENT_IN_STACK.format( - file_path=event["file_path"], - line_number=event["line_number"], - path_type=event["path_type"], - description=event["description"], - ) - - @property - def display_name(self): - """ - Set the display name as `Coverity` - """ - return self.analyzer.display_name - - def is_local(self): - """ - The given coverity issue should be only locally stored and not in the - remote snapshot - """ - # According to Coverity manual: - # presentInReferenceSnapshot - True if the issue is present in the reference - # snapshot specified in the cov-run-desktop command, false if not. - return ( - self.state_on_server is not None - and "presentInReferenceSnapshot" in self.state_on_server - and self.state_on_server["presentInReferenceSnapshot"] is False - ) - - def validates(self): - """ - Publish only local Coverity issues - """ - return self.is_local() - - def as_text(self): - """ - Build the text body published on reporters - """ - # If there is the reliability index use it - return ( - f"Checker reliability is {self.reliability.value}, meaning that the false positive ratio is {self.reliability.invert}.\n{self.message}" - if self.reliability != Reliability.Unknown - else self.message - ) - - def as_markdown(self): - return ISSUE_MARKDOWN.format( - check=self.check, - message=self.message, - location="{}:{}".format(self.path, self.line), - publishable=self.is_publishable() and "yes" or "no", - is_local=self.is_local() and "yes" or "no", - reliability=self.reliability.value, - ) - - -class CoverityTask(AnalysisTask): - """ - Support remote Coverity analyzer - """ - - artifacts = ["public/code-review/coverity.json"] - - @property - def display_name(self): - return "Coverity" - - def parse_issues(self, artifacts, revision): - """ - Parse issues from a pre-translated Coverity report - """ - assert isinstance(artifacts, dict) - return [ - CoverityIssue( - analyzer=self, revision=revision, issue=warning, file_path=path - ) - for artifact in artifacts.values() - for path, items in artifact["files"].items() - for warning in items["warnings"] - ] diff --git a/bot/code_review_bot/tasks/infer.py b/bot/code_review_bot/tasks/infer.py deleted file mode 100644 index ff455406..00000000 --- a/bot/code_review_bot/tasks/infer.py +++ /dev/null @@ -1,91 +0,0 @@ -# -*- coding: utf-8 -*- -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. - -import structlog - -from code_review_bot import Issue -from code_review_bot import Level -from code_review_bot.tasks.base import AnalysisTask - -logger = structlog.get_logger(__name__) - -ISSUE_MARKDOWN = """ -## infer error - -- **Message**: {message} -- **Location**: {location} -- **In patch**: {in_patch} -- **Infer check**: {check} -- **Publishable **: {publishable} -""" - - -class InferIssue(Issue): - """ - An issue reported by infer - """ - - def __init__(self, analyzer, entry, revision): - assert isinstance(entry, dict) - kind = entry.get("kind") or entry.get("severity") - assert kind is not None, "Missing issue kind" - super().__init__( - analyzer, - revision, - path=entry["file"], - line=entry["line"], - nb_lines=1, - check=entry["bug_type"], - column=entry["column"], - level=Level.Warning, - message=entry["qualifier"], - ) - - def validates(self): - """ - Publish infer issues all the time - """ - return True - - def as_text(self): - """ - Build the text body published on reporters - """ - message = self.message - if len(message) > 0: - message = message[0].capitalize() + message[1:] - return "{}: {} [infer: {}]".format(self.level.name, message, self.check) - - def as_markdown(self): - return ISSUE_MARKDOWN.format( - check=self.check, - message=self.message, - location="{}:{}:{}".format(self.path, self.line, self.column), - in_patch="yes" if self.revision.contains(self) else "no", - publishable="yes" if self.is_publishable() else "no", - ) - - -class InferTask(AnalysisTask): - """ - Support remote Infer analyzer - """ - - artifacts = ["public/code-review/infer.json"] - - def build_help_message(self, files): - files = " ".join(files) - return f"`./mach static-analysis check-java {files} (Java)" - - def parse_issues(self, artifacts, revision): - """ - Parse issues from a direct Infer JSON report - """ - assert isinstance(artifacts, dict) - return [ - InferIssue(analyzer=self, revision=revision, entry=issue) - for issues in artifacts.values() - for issue in issues - ] diff --git a/bot/code_review_bot/workflow.py b/bot/code_review_bot/workflow.py index 5a77ac0b..7fcb0083 100644 --- a/bot/code_review_bot/workflow.py +++ b/bot/code_review_bot/workflow.py @@ -24,10 +24,8 @@ from code_review_bot.tasks.clang_format import ClangFormatTask from code_review_bot.tasks.clang_tidy import ClangTidyTask from code_review_bot.tasks.clang_tidy_external import ExternalTidyTask from code_review_bot.tasks.coverage import ZeroCoverageTask -from code_review_bot.tasks.coverity import CoverityTask from code_review_bot.tasks.default import DefaultTask from code_review_bot.tasks.docupload import DocUploadTask -from code_review_bot.tasks.infer import InferTask from code_review_bot.tasks.lint import MozLintTask from code_review_bot.tasks.tgdiff import TaskGraphDiffTask @@ -408,10 +406,6 @@ class Workflow(object): return ClangTidyTask(task_id, task_status) elif name == "source-test-clang-format": return ClangFormatTask(task_id, task_status) - elif name in ("source-test-coverity-coverity", "coverity"): - return CoverityTask(task_id, task_status) - elif name == "source-test-infer-infer": - return InferTask(task_id, task_status) elif name == "source-test-doc-upload": return DocUploadTask(task_id, task_status) elif name == "source-test-clang-external": diff --git a/bot/tests/conftest.py b/bot/tests/conftest.py index 33adb8a6..6556d089 100644 --- a/bot/tests/conftest.py +++ b/bot/tests/conftest.py @@ -23,8 +23,6 @@ from code_review_bot.backend import BackendAPI from code_review_bot.config import settings from code_review_bot.tasks.clang_tidy import ClangTidyIssue from code_review_bot.tasks.clang_tidy import ClangTidyTask -from code_review_bot.tasks.coverity import CoverityIssue -from code_review_bot.tasks.coverity import CoverityTask from code_review_bot.tasks.default import DefaultTask MOCK_DIR = os.path.join(os.path.dirname(__file__), "mocks") @@ -106,29 +104,6 @@ def mock_task(): return _build -@pytest.fixture -def mock_coverity_issues(mock_revision, mock_task): - """ - Build a list of Coverity issues - """ - - return [ - CoverityIssue( - mock_task(CoverityTask, "mock-coverity"), - mock_revision, - { - "reliability": "high", - "line": i, - "message": "Unidentified symbol", - "extra": {"category": "bug", "stateOnServer": []}, - "flag": "flag", - }, - "some/file/path", - ) - for i in range(2) - ] - - @pytest.fixture def mock_clang_tidy_issues(mock_revision, mock_task): """ diff --git a/bot/tests/fixtures/infer_issues_0.16.0.json b/bot/tests/fixtures/infer_issues_0.16.0.json deleted file mode 100644 index 586e3114..00000000 --- a/bot/tests/fixtures/infer_issues_0.16.0.json +++ /dev/null @@ -1,137 +0,0 @@ -[ - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "ff8f5eb34138154ca151793c1c06d75d", - "in_patch": false, - "level": "warning", - "line": 234, - "message": "object returned by `profile.getDir()` could be null and is dereferenced at line 234.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "d5f13a2699444a1e6fd01ee893021df9", - "in_patch": false, - "level": "warning", - "line": 233, - "message": "object returned by `GeckoViewActivity$3.this$0.mNotificationIDMap.get(notification.tag)` could be null and is dereferenced at line 233.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "b437f17fcf55998ac056e1430d5f20d7", - "in_patch": false, - "level": "warning", - "line": 302, - "message": "object `profile` last assigned on line 301 could be null and is dereferenced at line 302.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "c6c50ee2d4d9e42321e4c2db9c1bae4e", - "in_patch": false, - "level": "warning", - "line": 511, - "message": "object `notification` last assigned on line 510 could be null and is dereferenced at line 511.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "347d91163a9d6fdc12dde4db32b66f2e", - "in_patch": false, - "level": "warning", - "line": 599, - "message": "object returned by `getActiveProfile()` at line 599.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "5e1014e2af3a453121bbad66f83e580f", - "in_patch": false, - "level": "warning", - "line": 688, - "message": "object returned by `GeckoViewActivity$ExampleContentDelegate.this$0.getSupportActionBar()` at line 688.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "f6fe7dddcc3fc6354fe7cb8b693c20bc", - "in_patch": false, - "level": "warning", - "line": 690, - "message": "object returned by `GeckoViewActivity$ExampleContentDelegate.this$0.getSupportActionBar()` at line 690.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "19d91e8dbe059ad53856211e03d555d0", - "in_patch": false, - "level": "warning", - "line": 913, - "message": "object `name` last assigned on line 911 at line 913.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "a66f388d848ae6ea5cdc1acb1c920d97", - "in_patch": false, - "level": "warning", - "line": 918, - "message": "object `name` last assigned on line 911 at line 918.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - } -] \ No newline at end of file diff --git a/bot/tests/fixtures/infer_issues_0.17.0.json b/bot/tests/fixtures/infer_issues_0.17.0.json deleted file mode 100644 index f0d00dde..00000000 --- a/bot/tests/fixtures/infer_issues_0.17.0.json +++ /dev/null @@ -1,482 +0,0 @@ -[ - { - "analyzer": "mock-infer", - "check": "INEFFICIENT_KEYSET_ITERATOR", - "column": -1, - "fix": null, - "hash": "15a65ca6ef4fff6c49a4da94e33147f0", - "in_patch": false, - "level": "warning", - "line": 102, - "message": "Accessing a value using a key that was retrieved from a `keySet` iterator. It is more efficient to use an iterator on the `entrySet` of the map, avoiding the extra `HashMap.get(key)` lookup.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/gecko/mozglue/GeckoLoader.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "ff8f5eb34138154ca151793c1c06d75d", - "in_patch": false, - "level": "warning", - "line": 234, - "message": "object returned by `profile.getDir()` could be null and is dereferenced at line 234.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "d5f13a2699444a1e6fd01ee893021df9", - "in_patch": false, - "level": "warning", - "line": 233, - "message": "object returned by `GeckoViewActivity$3.this$0.mNotificationIDMap.get(notification.tag)` could be null and is dereferenced at line 233.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "b437f17fcf55998ac056e1430d5f20d7", - "in_patch": false, - "level": "warning", - "line": 302, - "message": "object `profile` last assigned on line 301 could be null and is dereferenced at line 302.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "c6c50ee2d4d9e42321e4c2db9c1bae4e", - "in_patch": false, - "level": "warning", - "line": 511, - "message": "object `notification` last assigned on line 510 could be null and is dereferenced at line 511.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "347d91163a9d6fdc12dde4db32b66f2e", - "in_patch": false, - "level": "warning", - "line": 599, - "message": "object returned by `getActiveProfile()` could be null and is dereferenced at line 599.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "5e1014e2af3a453121bbad66f83e580f", - "in_patch": false, - "level": "warning", - "line": 688, - "message": "object returned by `GeckoViewActivity$ExampleContentDelegate.this$0.getSupportActionBar()` could be null and is dereferenced at line 688.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "f6fe7dddcc3fc6354fe7cb8b693c20bc", - "in_patch": false, - "level": "warning", - "line": 690, - "message": "object returned by `GeckoViewActivity$ExampleContentDelegate.this$0.getSupportActionBar()` could be null and is dereferenced at line 690.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "19d91e8dbe059ad53856211e03d555d0", - "in_patch": false, - "level": "warning", - "line": 913, - "message": "object `name` last assigned on line 911 could be null and is dereferenced at line 913.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "NULL_DEREFERENCE", - "column": -1, - "fix": null, - "hash": "a66f388d848ae6ea5cdc1acb1c920d97", - "in_patch": false, - "level": "warning", - "line": 918, - "message": "object `name` last assigned on line 911 could be null and is dereferenced at line 918.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "f7777c76a86c6f0a8985eb42cade7cf2", - "in_patch": false, - "level": "warning", - "line": 345, - "message": "Method `GeckoRuntime GeckoRuntime.create(Context)` runs on UI thread (because `GeckoRuntime GeckoRuntime.create(Context)` is annotated `UiThread`), and may block; calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "cb0ea2ee6fa35fd1d87cd30684db40ef", - "in_patch": false, - "level": "warning", - "line": 469, - "message": "Method `GeckoRuntime GeckoRuntime.create(Context,GeckoRuntimeSettings)` runs on UI thread (because `GeckoRuntime GeckoRuntime.create(Context,GeckoRuntimeSettings)` is annotated `UiThread`), and may block; calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "c9fa983da567a3979858771134a98c18", - "in_patch": false, - "level": "warning", - "line": 154, - "message": "Method `GeckoRuntime GeckoRuntime.getDefault(Context)` runs on UI thread (because `GeckoRuntime GeckoRuntime.getDefault(Context)` is annotated `UiThread`), and may block; calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "84044d09e46ec121db56ddcc7c94ed5b", - "in_patch": false, - "level": "warning", - "line": 599, - "message": "Method `File GeckoRuntime.getProfileDir()` runs on UI thread (because `File GeckoRuntime.getProfileDir()` is annotated `UiThread`), and may block; calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STRICT_MODE_VIOLATION", - "column": -1, - "fix": null, - "hash": "e0187572bb209236aa30c2803d1fdaae", - "in_patch": false, - "level": "warning", - "line": 599, - "message": "Method `File GeckoRuntime.getProfileDir()` runs on UI thread (because `File GeckoRuntime.getProfileDir()` is annotated `UiThread`), and may violate Strict Mode; calls `java.lang.String[] File.list()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "b9cd808ab39fff1f7dc7f11698a1b943", - "in_patch": false, - "level": "warning", - "line": 301, - "message": "Method `boolean GeckoRuntime.init(Context,GeckoRuntimeSettings)` runs on UI thread (because it calls `void ThreadUtils.assertOnUiThread()`) and locks `this` in `class org.mozilla.gecko.GeckoThread`, which may be held by another thread which calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "375b98e61d6411f524192bcd879f35a2", - "in_patch": false, - "level": "warning", - "line": 272, - "message": "Method `boolean GeckoRuntime.init(Context,GeckoRuntimeSettings)` runs on UI thread (because it calls `void ThreadUtils.assertOnUiThread()`), and may block; calls `void Object.wait()`.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "59cf8d63b1c2aea73690efcf322223e8", - "in_patch": false, - "level": "warning", - "line": 188, - "message": "Method `GeckoResult GeckoViewActivity$2.onCloseTab(WebExtension,GeckoSession)` runs on UI thread (because class `GeckoResult GeckoViewActivity$2.onCloseTab(WebExtension,GeckoSession)` overrides `GeckoResult WebExtensionController$TabDelegate.onCloseTab(WebExtension,GeckoSession)`, which is annotated `UiThread`), and may block; calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "21f178431017db7d129703f9ee726cad", - "in_patch": false, - "level": "warning", - "line": 733, - "message": "Method `void GeckoViewActivity$ExampleContentDelegate.onCrash(GeckoSession)` runs on UI thread (because class `void GeckoViewActivity$ExampleContentDelegate.onCrash(GeckoSession)` overrides `void GeckoSession$ContentDelegate.onCrash(GeckoSession)`, which is annotated `UiThread`) and locks `this.mBundle` in `class org.mozilla.geckoview.GeckoSessionSettings`, which may be held by another thread which calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "f6ec06db8963418a09e5209317692863", - "in_patch": false, - "level": "warning", - "line": 176, - "message": "Method `void GeckoViewActivity.onCreate(Bundle)` runs on UI thread (because `void GeckoViewActivity.onCreate(Bundle)` is a standard UI-thread method) and locks `this` in `class org.mozilla.gecko.GeckoThread`, which may be held by another thread which calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "fcddfcb2f7ea910b4414ac16fe7dc06f", - "in_patch": false, - "level": "warning", - "line": 255, - "message": "Method `void GeckoViewActivity.onCreate(Bundle)` runs on UI thread (because `void GeckoViewActivity.onCreate(Bundle)` is a standard UI-thread method), and may block; calls `void Object.wait()`.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "7edd0633fe178d2170f2fa8a11c11aba", - "in_patch": false, - "level": "warning", - "line": 245, - "message": "Method `void GeckoViewActivity.onCreate(Bundle)` runs on UI thread (because `void GeckoViewActivity.onCreate(Bundle)` is a standard UI-thread method) and locks `this.mBundle` in `class org.mozilla.geckoview.GeckoSessionSettings`, which may be held by another thread which calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "433c177b7062f87f60498e678be28dd5", - "in_patch": false, - "level": "warning", - "line": 248, - "message": "Method `void GeckoViewActivity.onCreate(Bundle)` runs on UI thread (because `void GeckoViewActivity.onCreate(Bundle)` is a standard UI-thread method) and locks `this.mBundle` in `class org.mozilla.geckoview.GeckoSessionSettings`, which may be held by another thread which calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "504f9ae58c4df8d56e20b320e470eb63", - "in_patch": false, - "level": "warning", - "line": 257, - "message": "Method `void GeckoViewActivity.onCreate(Bundle)` runs on UI thread (because `void GeckoViewActivity.onCreate(Bundle)` is a standard UI-thread method) and locks `this.mBundle` in `class org.mozilla.geckoview.GeckoSessionSettings`, which may be held by another thread which calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "d536dafd9b3719feac94ba496ea9f89b", - "in_patch": false, - "level": "warning", - "line": 518, - "message": "Method `void GeckoViewActivity.onNewIntent(Intent)` runs on UI thread (because `void WebNotification.click()` is annotated `UiThread`), and may block; calls `void Object.wait()`.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "8e71a4ac4111ae1d80ca381650a3e5ae", - "in_patch": false, - "level": "warning", - "line": 997, - "message": "Method `GeckoResult GeckoViewActivity$ExampleNavigationDelegate.onNewSession(GeckoSession,String)` runs on UI thread (because class `GeckoResult GeckoViewActivity$ExampleNavigationDelegate.onNewSession(GeckoSession,String)` overrides `GeckoResult GeckoSession$NavigationDelegate.onNewSession(GeckoSession,String)`, which is annotated `UiThread`), and may block; calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "f5212cd75048eb95a46f5809841dc42f", - "in_patch": false, - "level": "warning", - "line": 181, - "message": "Method `GeckoResult GeckoViewActivity$2.onNewTab(WebExtension,String)` runs on UI thread (because class `GeckoResult GeckoViewActivity$2.onNewTab(WebExtension,String)` overrides `GeckoResult WebExtensionController$TabDelegate.onNewTab(WebExtension,String)`, which is annotated `UiThread`), and may block; calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "8c11c6691d967fce46a23cd305d10213", - "in_patch": false, - "level": "warning", - "line": 415, - "message": "Method `boolean GeckoViewActivity.onOptionsItemSelected(MenuItem)` runs on UI thread (because `void GeckoSession.open(GeckoRuntime)` is annotated `UiThread`), and may block; calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "476ed81c472fdbc9cf119e0f28337892", - "in_patch": false, - "level": "warning", - "line": 552, - "message": "Method `void GeckoViewActivity.onRequestPermissionsResult(int,java.lang.String[],int[])` runs on UI thread (because `GeckoSession$PermissionDelegate GeckoSession.getPermissionDelegate()` is annotated `UiThread`), and may block; calls `void Object.wait()`.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "53b2282bea68c626437042136c13a32f", - "in_patch": false, - "level": "warning", - "line": 352, - "message": "Method `void GeckoViewActivity.onRestoreInstanceState(Bundle)` runs on UI thread (because class `org.mozilla.geckoview.GeckoView` is annotated `UiThread`), and may block; calls `void Object.wait()`. Additional report(s) on the same line were suppressed.", - "nb_lines": 1, - "path": "mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "3aa4f8e608a4cd7e550bf9fd202a29f5", - "in_patch": false, - "level": "warning", - "line": 407, - "message": "Method `GeckoResult GeckoRuntime.registerWebExtension(WebExtension)` runs on UI thread (because `GeckoResult GeckoRuntime.registerWebExtension(WebExtension)` is annotated `UiThread`), and may block; calls `void Object.wait()`.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java", - "publishable": false, - "validates": true - }, - { - "analyzer": "mock-infer", - "check": "STARVATION", - "column": -1, - "fix": null, - "hash": "c25d6299639c7146fc69d08d9d07fb22", - "in_patch": false, - "level": "warning", - "line": 437, - "message": "Method `GeckoResult GeckoRuntime.unregisterWebExtension(WebExtension)` runs on UI thread (because `GeckoResult GeckoRuntime.unregisterWebExtension(WebExtension)` is annotated `UiThread`), and may block; calls `void Object.wait()`.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java", - "publishable": false, - "validates": true - } -] \ No newline at end of file diff --git a/bot/tests/mocks/coverity_simple.json b/bot/tests/mocks/coverity_simple.json deleted file mode 100644 index 296a7466..00000000 --- a/bot/tests/mocks/coverity_simple.json +++ /dev/null @@ -1 +0,0 @@ -{"files": {"js/src/jit/BaselineCompiler.cpp": {"warnings": [{"line": 3703, "reliability": "medium", "message": "Dereferencing a pointer that might be \"nullptr\" \"env\" when calling \"lookupImport\".", "flag": "NULL_RETURNS", "extra": {"category": "Null pointer dereferences", "stateOnServer": {"ownerLdapServerName": "local", "stream": "Firefox", "cid": 95687, "cached": false, "retrievalDateTime": "2019-05-13T10:20:22+00:00", "firstDetectedDateTime": "2019-04-08T12:57:07+00:00", "presentInReferenceSnapshot": false, "components": ["js"], "customTriage": {}, "triage": {"fixTarget": "Untargeted", "severity": "Unspecified", "classification": "Unclassified", "owner": "try", "legacy": "False", "action": "Undecided", "externalReference": ""}}, "stack": [{"line_number": 3697, "description": "\"GetModuleEnvironmentForScript\" returns \"nullptr\" (checked 2 out of 2 times).", "file_path": "js/src/jit/BaselineCompiler.cpp", "path_type": "returned_null"}, {"line_number": 3697, "description": "Assigning: \"env\" = \"nullptr\" return value from \"GetModuleEnvironmentForScript\".", "file_path": "js/src/jit/BaselineCompiler.cpp", "path_type": "var_assigned"}, {"line_number": 3703, "description": "Dereferencing a pointer that might be \"nullptr\" \"env\" when calling \"lookupImport\".", "file_path": "js/src/jit/BaselineCompiler.cpp", "path_type": "dereference"}]}}]}}} \ No newline at end of file diff --git a/bot/tests/test_backend.py b/bot/tests/test_backend.py index b858d44d..badc0a39 100644 --- a/bot/tests/test_backend.py +++ b/bot/tests/test_backend.py @@ -8,7 +8,7 @@ import pytest from code_review_bot.backend import BackendAPI -def test_publication(mock_coverity_issues, mock_revision, mock_backend, mock_hgmo): +def test_publication(mock_clang_tidy_issues, mock_revision, mock_backend, mock_hgmo): """ Test publication of issues on the backend """ @@ -55,8 +55,8 @@ def test_publication(mock_coverity_issues, mock_revision, mock_backend, mock_hgm assert len(issues) == 0 # Let's publish them - published = r.publish_issues(mock_coverity_issues, mock_revision) - assert published == len(mock_coverity_issues) == 2 + published = r.publish_issues(mock_clang_tidy_issues, mock_revision) + assert published == len(mock_clang_tidy_issues) == 2 # Check the issues in the backend assert len(issues) == 1 @@ -64,35 +64,35 @@ def test_publication(mock_coverity_issues, mock_revision, mock_backend, mock_hgm assert len(issues[42]) == 2 assert issues[42] == [ { - "analyzer": "mock-coverity", - "check": "flag", - "column": None, - "hash": "3731a6559c9a72d09f4bad85db3f0416", + "analyzer": "mock-clang-tidy", + "check": "clanck.checker", + "column": 46, + "hash": "8ab67a6518302144cb375f04f3f17c8a", "id": "9f6aa76a-623d-5096-82ed-876b01f9fbce", "in_patch": False, "level": "warning", - "line": None, - "message": "Unidentified symbol", + "line": 57, + "message": "Some Error Message", "nb_lines": 1, - "path": "some/file/path", + "path": "dom/animation/Animation.cpp", "publishable": False, - "validates": False, + "validates": True, "fix": None, }, { - "analyzer": "mock-coverity", - "check": "flag", - "column": None, - "hash": "1fcc4d02d6184028f40b48e877be62b4", + "analyzer": "mock-clang-tidy", + "check": "clanck.checker", + "column": 46, + "hash": "1ca389dd8fa5cce3bbd496bc228daaaf", "id": "98d7e3b0-e903-57e3-9973-d11d3a9849f4", "in_patch": False, - "level": "warning", - "line": 1, - "message": "Unidentified symbol", + "level": "error", + "line": 57, + "message": "Some Error Message", "nb_lines": 1, - "path": "some/file/path", - "publishable": False, - "validates": False, + "path": "dom/animation/Animation.cpp", + "publishable": True, + "validates": True, "fix": None, }, ] @@ -130,7 +130,7 @@ def test_missing_bugzilla_id(mock_revision, mock_backend, mock_hgmo): } -def test_repo_url(mock_coverity_issues, mock_revision, mock_backend, mock_hgmo): +def test_repo_url(mock_revision, mock_backend, mock_hgmo): """ Check that the backend client verifies repositories are URLs """ @@ -154,7 +154,7 @@ def test_repo_url(mock_coverity_issues, mock_revision, mock_backend, mock_hgmo): def test_publication_failures( - mock_coverity_issues, mock_revision, mock_backend, mock_hgmo + mock_clang_tidy_issues, mock_revision, mock_backend, mock_hgmo ): """ Test publication of issues on the backend with some bad urls @@ -174,12 +174,12 @@ def test_publication_failures( assert r.enabled is True # Use a bad relative path in last issue - mock_coverity_issues[-1].path = "../../../bad/path.cpp" - assert mock_coverity_issues[0].path == "some/file/path" + mock_clang_tidy_issues[-1].path = "../../../bad/path.cpp" + assert mock_clang_tidy_issues[0].path == "dom/animation/Animation.cpp" # Only one issue should be published as the bad one is ignored mock_revision.issues_url = "http://code-review-backend.test/v1/diff/42/issues/" - published = r.publish_issues(mock_coverity_issues, mock_revision) + published = r.publish_issues(mock_clang_tidy_issues, mock_revision) assert published == 1 # Check the issues in the backend @@ -188,19 +188,19 @@ def test_publication_failures( assert len(issues[42]) == 1 assert issues[42] == [ { - "analyzer": "mock-coverity", - "check": "flag", - "column": None, - "hash": "3731a6559c9a72d09f4bad85db3f0416", + "analyzer": "mock-clang-tidy", + "check": "clanck.checker", + "column": 46, + "hash": "8ab67a6518302144cb375f04f3f17c8a", "id": "9f6aa76a-623d-5096-82ed-876b01f9fbce", "in_patch": False, "level": "warning", - "line": None, - "message": "Unidentified symbol", + "line": 57, + "message": "Some Error Message", "nb_lines": 1, - "path": "some/file/path", + "path": "dom/animation/Animation.cpp", "publishable": False, - "validates": False, + "validates": True, "fix": None, } ] diff --git a/bot/tests/test_coverity.py b/bot/tests/test_coverity.py deleted file mode 100644 index 5235c784..00000000 --- a/bot/tests/test_coverity.py +++ /dev/null @@ -1,142 +0,0 @@ -# -*- coding: utf-8 -*- -import json -import os - -from code_review_bot.tasks.coverity import CoverityIssue -from code_review_bot.tasks.coverity import CoverityTask -from code_review_bot.tasks.coverity import Reliability -from conftest import MOCK_DIR - - -def mock_coverity(name): - """ - Load a Coverity mock file, as a Taskcluster artifact payload - """ - path = os.path.join(MOCK_DIR, "coverity_{}.json".format(name)) - assert os.path.exists(path), "Missing coverity mock {}".format(path) - with open(path) as f: - return {"public/code-review/coverity.json": json.load(f)} - - -def test_simple(mock_revision, mock_config, log, mock_hgmo, mock_task): - """ - Test parsing a simple Coverity artifact - """ - - task = mock_task(CoverityTask, "mock-coverity") - issues = task.parse_issues(mock_coverity("simple"), mock_revision) - assert len(issues) == 1 - assert all(map(lambda i: isinstance(i, CoverityIssue), issues)) - - issue = issues[0] - - assert issue.analyzer == task - assert issue.analyzer.name == "mock-coverity" - assert issue.revision == mock_revision - assert issue.reliability == Reliability.Medium - assert issue.path == "js/src/jit/BaselineCompiler.cpp" - assert issue.line == 3703 - assert issue.check == "NULL_RETURNS" - assert ( - issue.message - == """Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport". -The path that leads to this defect is: - -- //js/src/jit/BaselineCompiler.cpp:3697//: --- `returned_null: "GetModuleEnvironmentForScript" returns "nullptr" (checked 2 out of 2 times).`. - -- //js/src/jit/BaselineCompiler.cpp:3697//: --- `var_assigned: Assigning: "env" = "nullptr" return value from "GetModuleEnvironmentForScript".`. - -- //js/src/jit/BaselineCompiler.cpp:3703//: --- `dereference: Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport".`. -""" - ) - assert issue.state_on_server == { - "cached": False, - "cid": 95687, - "components": ["js"], - "customTriage": {}, - "firstDetectedDateTime": "2019-04-08T12:57:07+00:00", - "ownerLdapServerName": "local", - "presentInReferenceSnapshot": False, - "retrievalDateTime": "2019-05-13T10:20:22+00:00", - "stream": "Firefox", - "triage": { - "action": "Undecided", - "classification": "Unclassified", - "externalReference": "", - "fixTarget": "Untargeted", - "legacy": "False", - "owner": "try", - "severity": "Unspecified", - }, - } - assert issue.nb_lines == 1 - - assert issue.path == "js/src/jit/BaselineCompiler.cpp" - assert issue.validates() - assert not issue.is_publishable() - - assert issue.as_phabricator_lint() == { - "code": "NULL_RETURNS", - "line": 3703, - "name": "Coverity", - "path": "js/src/jit/BaselineCompiler.cpp", - "severity": "warning", - "description": """WARNING: Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport". -The path that leads to this defect is: - -- //js/src/jit/BaselineCompiler.cpp:3697//: --- `returned_null: "GetModuleEnvironmentForScript" returns "nullptr" (checked 2 out of 2 times).`. - -- //js/src/jit/BaselineCompiler.cpp:3697//: --- `var_assigned: Assigning: "env" = "nullptr" return value from "GetModuleEnvironmentForScript".`. - -- //js/src/jit/BaselineCompiler.cpp:3703//: --- `dereference: Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport".`. -""", - } - - assert ( - issue.as_text() - == """Checker reliability is medium, meaning that the false positive ratio is medium. -Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport". -The path that leads to this defect is: - -- //js/src/jit/BaselineCompiler.cpp:3697//: --- `returned_null: "GetModuleEnvironmentForScript" returns "nullptr" (checked 2 out of 2 times).`. - -- //js/src/jit/BaselineCompiler.cpp:3697//: --- `var_assigned: Assigning: "env" = "nullptr" return value from "GetModuleEnvironmentForScript".`. - -- //js/src/jit/BaselineCompiler.cpp:3703//: --- `dereference: Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport".`. -""" - ) - assert issue.as_dict() == { - "analyzer": "mock-coverity", - "in_patch": False, - "check": "NULL_RETURNS", - "column": None, - "level": "warning", - "line": 3703, - "message": """Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport". -The path that leads to this defect is: - -- //js/src/jit/BaselineCompiler.cpp:3697//: --- `returned_null: "GetModuleEnvironmentForScript" returns "nullptr" (checked 2 out of 2 times).`. - -- //js/src/jit/BaselineCompiler.cpp:3697//: --- `var_assigned: Assigning: "env" = "nullptr" return value from "GetModuleEnvironmentForScript".`. - -- //js/src/jit/BaselineCompiler.cpp:3703//: --- `dereference: Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport".`. -""", - "nb_lines": 1, - "path": "js/src/jit/BaselineCompiler.cpp", - "publishable": False, - "validates": True, - "hash": "8c10f73c6f509336236f407c3f9d312a", - "fix": None, - } diff --git a/bot/tests/test_infer.py b/bot/tests/test_infer.py deleted file mode 100644 index fcafdfb9..00000000 --- a/bot/tests/test_infer.py +++ /dev/null @@ -1,111 +0,0 @@ -# -*- coding: utf-8 -*- -import json -import os - -import pytest - -from code_review_bot.tasks.infer import InferIssue -from code_review_bot.tasks.infer import InferTask -from conftest import FIXTURES_DIR - - -def test_as_text(mock_revision, mock_task): - """ - Test text export for InferIssue - """ - parts = { - "file": "path/to/file.java", - "line": 3, - "column": -1, - "bug_type": "SOMETYPE", - "kind": "ERROR", - "qualifier": "Error on this line", - } - issue = InferIssue(mock_task(InferTask, "mock-infer"), parts, mock_revision) - - expected = "Warning: Error on this line [infer: SOMETYPE]" - assert issue.as_text() == expected - - -def test_as_dict(mock_revision, mock_hgmo, mock_task): - """ - Test dict export for InferIssue - """ - - parts = { - "file": "path/to/file.java", - "line": 3, - "column": -1, - "bug_type": "SOMETYPE", - "kind": "WARNING", - "qualifier": "Error on this line", - } - issue = InferIssue(mock_task(InferTask, "mock-infer"), parts, mock_revision) - - assert issue.as_dict() == { - "analyzer": "mock-infer", - "check": "SOMETYPE", - "column": -1, - "in_patch": False, - "level": "warning", - "line": 3, - "message": "Error on this line", - "nb_lines": 1, - "path": "path/to/file.java", - "publishable": False, - "validates": True, - "hash": "f446f72d667c04e3d9164f91af424414", - "fix": None, - } - - -def test_as_markdown(mock_revision, mock_task): - """ - Test markdown generation for InferIssue - """ - - parts = { - "file": "path/to/file.java", - "line": 3, - "column": -1, - "bug_type": "SOMETYPE", - "kind": "WARNING", - "qualifier": "Error on this line", - } - issue = InferIssue(mock_task(InferTask, "mock-infer"), parts, mock_revision) - - assert ( - issue.as_markdown() - == """ -## infer error - -- **Message**: Error on this line -- **Location**: path/to/file.java:3:-1 -- **In patch**: no -- **Infer check**: SOMETYPE -- **Publishable **: no -""" - ) - - -@pytest.mark.parametrize("version, nb", [("0.16.0", 9), ("0.17.0", 32)]) -def test_infer_artifact(version, nb, mock_revision, mock_hgmo): - """ - Test Infer artifact per version, comparing a raw artifact processed - and expected issues list - """ - with open(os.path.join(FIXTURES_DIR, f"infer_artifact_{version}.json")) as f: - artifact = json.load(f) - - status = {"task": {"metadata": {"name": "mock-infer"}}, "status": {}} - task = InferTask("someTaskId", status) - issues = task.parse_issues( - {"public/code-review/infer.json": artifact}, mock_revision - ) - - assert len(artifact) == len(issues) == nb - - issues_data = [issue.as_dict() for issue in issues] - - with open(os.path.join(FIXTURES_DIR, f"infer_issues_{version}.json")) as f: - assert issues_data == json.load(f) diff --git a/bot/tests/test_remote.py b/bot/tests/test_remote.py index 72ce5d63..4e746e59 100644 --- a/bot/tests/test_remote.py +++ b/bot/tests/test_remote.py @@ -7,7 +7,6 @@ import pytest import responses from libmozdata.phabricator import BuildState -from code_review_bot import Level from code_review_bot import stats @@ -688,180 +687,6 @@ def test_clang_format_task( assert mock_revision._state == BuildState.Pass -def test_coverity_task( - mock_config, mock_revision, mock_workflow, mock_backend, mock_hgmo -): - """ - Test a remote workflow with a clang-tidy analyzer - """ - from code_review_bot import Reliability - from code_review_bot.tasks.coverity import CoverityIssue - - mock_workflow.setup_mock_tasks( - { - "decision": { - "image": "taskcluster/decision:XXX", - "env": { - "GECKO_HEAD_REPOSITORY": "https://hg.mozilla.org/try", - "GECKO_HEAD_REV": "deadbeef1234", - }, - }, - "remoteTryTask": {"dependencies": ["coverity"]}, - "coverity": { - "name": "source-test-coverity-coverity", - "state": "completed", - "artifacts": { - "public/code-review/coverity.json": { - "files": { - "test.cpp": { - "warnings": [ - { - "line": 66, - "flag": "UNINIT", - "reliability": "high", - "message": 'Using uninitialized value "a".', - "extra": { - "category": "Memory - corruptions", - "stateOnServer": { - "presentInReferenceSnapshot": False - }, - "stack": [ - { - "line_number": 61, - "description": 'Condition "!target.oper…", taking false branch.', - "file_path": "dom/animation/Animation.cpp", - "path_type": "path", - } - ], - }, - } - ] - } - } - } - }, - }, - } - ) - issues = mock_workflow.run(mock_revision) - assert len(issues) == 1 - issue = issues[0] - assert isinstance(issue, CoverityIssue) - assert issue.path == "test.cpp" - assert issue.line == 66 - assert issue.check == "UNINIT" - assert issue.reliability == Reliability.High - assert ( - issue.message - == """Using uninitialized value "a". -The path that leads to this defect is: - -- //dom/animation/Animation.cpp:61//: --- `path: Condition \"!target.oper…", taking false branch.`.\n""" - ) - assert issue.is_local() - assert issue.validates() - - assert check_stats( - [ - ("code-review.analysis.files", None, 2), - ("code-review.analysis.lines", None, 2), - ("code-review.issues", "source-test-coverity-coverity", 1), - ("code-review.issues.publishable", "source-test-coverity-coverity", 0), - ("code-review.issues.paths", "source-test-coverity-coverity", 1), - ("code-review.analysis.issues.publishable", None, 0), - ("code-review.runtime.reports", None, "runtime"), - ] - ) - - assert mock_revision._state == BuildState.Pass - - -def test_infer_task(mock_config, mock_revision, mock_workflow, mock_hgmo, mock_backend): - """ - Test a remote workflow with an infer analyzer - """ - from code_review_bot.tasks.infer import InferIssue - - mock_workflow.setup_mock_tasks( - { - "decision": { - "image": "taskcluster/decision:XXX", - "env": { - "GECKO_HEAD_REPOSITORY": "https://hg.mozilla.org/try", - "GECKO_HEAD_REV": "deadbeef1234", - }, - }, - "remoteTryTask": {"dependencies": ["infer"]}, - "infer": { - "name": "source-test-infer-infer", - "state": "completed", - "artifacts": { - "public/code-review/infer.json": [ - { - "bug_class": "PROVER", - "kind": "error", - "bug_type": "THREAD_SAFETY_VIOLATION", - "qualifier": "Read/Write race.", - "severity": "HIGH", - "visibility": "user", - "line": 1196, - "column": -1, - "procedure": "void Bad.Function(Test,int)", - "procedure_id": "org.mozilla.geckoview.somewhere(, mock_workflow):void", - "procedure_start_line": 0, - "file": "mobile/android/geckoview/src/main/java/org/mozilla/test.java", - "bug_trace": [ - { - "level": 0, - "filename": "mobile/android/geckoview/src/main/java/org/mozilla/test.java", - "line_number": 1196, - "column_number": -1, - "description": "", - } - ], - "key": "GeckoSession.java|test|THREAD_SAFETY_VIOLATION", - "node_key": "9c5d6d9028928346cc4fb44cced5dea1", - "hash": "b008b0dd2b74e6036fa2105f7e54458e", - "bug_type_hum": "Thread Safety Violation", - "censored_reason": "", - "access": "reallyLongHash", - } - ] - }, - }, - } - ) - issues = mock_workflow.run(mock_revision) - assert len(issues) == 1 - issue = issues[0] - assert isinstance(issue, InferIssue) - assert issue.path == "mobile/android/geckoview/src/main/java/org/mozilla/test.java" - assert issue.line == 1196 - assert issue.column == -1 - assert issue.check == "THREAD_SAFETY_VIOLATION" - assert issue.level == Level.Warning - assert issue.message == "Read/Write race." - assert issue.nb_lines == 1 - assert issue.as_dict() == { - "analyzer": "source-test-infer-infer", - "check": "THREAD_SAFETY_VIOLATION", - "column": -1, - "in_patch": False, - "level": "warning", - "line": 1196, - "message": "Read/Write race.", - "nb_lines": 1, - "path": "mobile/android/geckoview/src/main/java/org/mozilla/test.java", - "publishable": False, - "validates": True, - "hash": "02353719655edb9ba07e0bd0cacd620b", - "fix": None, - } - - assert mock_revision._state == BuildState.Pass - - def test_no_tasks(mock_config, mock_revision, mock_workflow, mock_backend): """ Test a remote workflow with only a Gecko decision task as dep diff --git a/bot/tests/test_reporter_debug.py b/bot/tests/test_reporter_debug.py index 6e5b5c9b..90eb1e06 100644 --- a/bot/tests/test_reporter_debug.py +++ b/bot/tests/test_reporter_debug.py @@ -5,7 +5,7 @@ import json import os.path -from code_review_bot.tasks.infer import InferTask +from code_review_bot.tasks.clang_tidy import ClangTidyTask def test_publication(tmpdir, mock_issues, mock_revision): @@ -39,8 +39,8 @@ def test_publication(tmpdir, mock_issues, mock_revision): report_path = os.path.join(report_dir, "report.json") assert not os.path.exists(report_path) - status = {"task": {"metadata": {"name": "mock-infer"}}, "status": {}} - task = InferTask("someTaskId", status) + status = {"task": {"metadata": {"name": "mock-clang-tidy"}}, "status": {}} + task = ClangTidyTask("someTaskId", status) r = DebugReporter(report_dir) r.publish(mock_issues, mock_revision, [task], []) @@ -68,7 +68,7 @@ def test_publication(tmpdir, mock_issues, mock_revision): } assert "task_failures" in report - assert report["task_failures"] == [{"id": "someTaskId", "name": "mock-infer"}] + assert report["task_failures"] == [{"id": "someTaskId", "name": "mock-clang-tidy"}] assert "time" in report assert isinstance(report["time"], float) diff --git a/bot/tests/test_reporter_phabricator.py b/bot/tests/test_reporter_phabricator.py index f70ed18a..47b42e96 100644 --- a/bot/tests/test_reporter_phabricator.py +++ b/bot/tests/test_reporter_phabricator.py @@ -19,13 +19,9 @@ from code_review_bot.tasks.clang_tidy_external import ExternalTidyIssue from code_review_bot.tasks.clang_tidy_external import ExternalTidyTask from code_review_bot.tasks.coverage import CoverageIssue from code_review_bot.tasks.coverage import ZeroCoverageTask -from code_review_bot.tasks.coverity import CoverityIssue -from code_review_bot.tasks.coverity import CoverityTask from code_review_bot.tasks.default import DefaultIssue from code_review_bot.tasks.default import DefaultTask from code_review_bot.tasks.docupload import COMMENT_LINK_TO_DOC -from code_review_bot.tasks.infer import InferIssue -from code_review_bot.tasks.infer import InferTask from code_review_bot.tasks.lint import MozLintIssue from code_review_bot.tasks.lint import MozLintTask from code_review_bot.tasks.tgdiff import COMMENT_TASKGRAPH_DIFF @@ -43,16 +39,6 @@ If you see a problem in this automated review, [please report it here](https://b You can view these defects on [the code-review frontend](https://code-review.moz.tools/#/diff/42) and on [Treeherder](https://treeherder.mozilla.org/#/jobs?repo=try&revision=deadbeef1234). """ -VALID_COVERITY_MESSAGE = """ -Code analysis found 1 defect in the diff 42: - - 1 defect found by Coverity - ---- -If you see a problem in this automated review, [please report it here](https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox+Build+System&component=Source+Code+Analysis&short_desc=[Automated+review]+UPDATE&comment=**Phabricator+URL:**+https://phabricator.services.mozilla.com/...&format=__default__). - -You can view these defects on [the code-review frontend](https://code-review.moz.tools/#/diff/42) and on [Treeherder](https://treeherder.mozilla.org/#/jobs?repo=try&revision=deadbeef1234). -""" - VALID_BUILD_ERROR_MESSAGE = """ Code analysis found 1 defect in the diff 42: - 1 build error found by clang-tidy @@ -122,7 +108,7 @@ You can view these defects on [the code-review frontend](https://code-review.moz """ VALID_TASK_FAILURES_MESSAGE = """ -The analysis task [mock-infer](https://treeherder.mozilla.org/#/jobs?repo=try&revision=aabbccddee&selectedTaskRun=ab3NrysvSZyEwsOHL2MZfw-0) failed, but we could not detect any issue. +The analysis task [mock-clang-tidy](https://treeherder.mozilla.org/#/jobs?repo=try&revision=aabbccddee&selectedTaskRun=ab3NrysvSZyEwsOHL2MZfw-0) failed, but we could not detect any issue. Please check this task manually. --- @@ -501,7 +487,6 @@ def test_phabricator_clang_tidy_and_coverage( [ "mock-clang-format", "mock-clang-tidy", - "mock-infer", "mock-lint-flake8", "coverage", ], @@ -509,21 +494,20 @@ def test_phabricator_clang_tidy_and_coverage( "dummy", "mock-clang-tidy", "mock-clang-format", - "mock-infer", "mock-lint-flake8", ], ), # Skip clang-tidy ( ["mock-clang-tidy"], - ["mock-clang-format", "mock-infer", "mock-lint-flake8", "coverage"], - ["dummy", "mock-clang-format", "mock-infer", "mock-lint-flake8"], + ["mock-clang-format", "mock-lint-flake8", "coverage"], + ["dummy", "mock-clang-format", "mock-lint-flake8"], ), # Skip clang-tidy and mozlint ( ["mock-clang-format", "mock-clang-tidy"], - ["mock-infer", "mock-lint-flake8", "coverage"], - ["dummy", "mock-infer", "mock-lint-flake8"], + ["mock-lint-flake8", "coverage"], + ["dummy", "mock-lint-flake8"], ), ], ) @@ -574,18 +558,6 @@ def test_phabricator_analyzers( "modernize-use-nullptr", "dummy message", ), - InferIssue( - mock_task(InferTask, "mock-infer"), - { - "file": "test.cpp", - "line": 42, - "column": 1, - "bug_type": "dummy", - "kind": "WARNING", - "qualifier": "dummy message.", - }, - revision, - ), MozLintIssue( mock_task(MozLintTask, "mock-lint-flake8"), "test.cpp", @@ -618,9 +590,6 @@ def test_phabricator_analyzers( repr(revision), "Some lint fixes", ), - ImprovementPatch( - mock_task(InferTask, "mock-infer"), repr(revision), "Some java fixes" - ), ImprovementPatch( mock_task(MozLintTask, "mock-lint-flake8"), repr(revision), "Some js fixes" ), @@ -636,86 +605,6 @@ def test_phabricator_analyzers( assert [p.analyzer.name for p in patches] == valid_patches -def test_phabricator_coverity(mock_phabricator, phab, mock_try_task, mock_task): - """ - Test Phabricator Lint for a CoverityIssue - """ - - with mock_phabricator as api: - revision = Revision.from_try(mock_try_task, api) - revision.lines = { - # Add dummy lines diff - "test.cpp": [41, 42, 43] - } - revision.build_target_phid = "PHID-HMBD-deadbeef12456" - revision.repository = "https://hg.mozilla.org/try" - revision.repository_try_name = "try" - revision.mercurial_revision = "deadbeef1234" - - reporter = PhabricatorReporter({}, api=api) - - issue_dict = { - "line": 41, - "reliability": "medium", - "message": 'Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport".', - "flag": "NULL_RETURNS", - "build_error": False, - "extra": { - "category": "Null pointer dereferences", - "stateOnServer": { - "ownerLdapServerName": "local", - "stream": "Firefox", - "cid": 95687, - "cached": False, - "retrievalDateTime": "2019-05-13T10:20:22+00:00", - "firstDetectedDateTime": "2019-04-08T12:57:07+00:00", - "presentInReferenceSnapshot": False, - "components": ["js"], - "customTriage": {}, - "triage": { - "fixTarget": "Untargeted", - "severity": "Unspecified", - "classification": "Unclassified", - "owner": "try", - "legacy": "False", - "action": "Undecided", - "externalReference": "", - }, - }, - }, - } - - issue = CoverityIssue( - mock_task(CoverityTask, "mock-coverity"), revision, issue_dict, "test.cpp" - ) - assert issue.is_publishable() - - issues, patches = reporter.publish([issue], revision, [], []) - assert len(issues) == 1 - assert len(patches) == 0 - - # Check the callback has been used - assert phab.build_messages["PHID-HMBD-deadbeef12456"] == [ - { - "buildTargetPHID": "PHID-HMBD-deadbeef12456", - "lint": [ - { - "code": "NULL_RETURNS", - "description": 'WARNING: Dereferencing a pointer that might be "nullptr" ' - '"env" when calling "lookupImport".', - "line": 41, - "name": "Coverity", - "path": "test.cpp", - "severity": "warning", - } - ], - "type": "work", - "unit": [], - } - ] - assert phab.comments[51] == [VALID_COVERITY_MESSAGE] - - def test_phabricator_clang_tidy_build_error( mock_phabricator, phab, mock_try_task, mock_task ): @@ -855,7 +744,7 @@ def test_task_failures(mock_phabricator, phab, mock_try_task): reporter = PhabricatorReporter({"analyzers": ["clang-tidy"]}, api=api) status = { - "task": {"metadata": {"name": "mock-infer"}}, + "task": {"metadata": {"name": "mock-clang-tidy"}}, "status": {"runs": [{"runId": 0}]}, } task = ClangTidyTask("ab3NrysvSZyEwsOHL2MZfw", status) diff --git a/bot/tests/test_workflow.py b/bot/tests/test_workflow.py index e7fcf4e5..3bc467c1 100644 --- a/bot/tests/test_workflow.py +++ b/bot/tests/test_workflow.py @@ -13,9 +13,7 @@ from code_review_bot.revisions import Revision from code_review_bot.tasks.clang_format import ClangFormatTask from code_review_bot.tasks.clang_tidy import ClangTidyTask from code_review_bot.tasks.clang_tidy_external import ExternalTidyTask -from code_review_bot.tasks.coverity import CoverityTask from code_review_bot.tasks.default import DefaultTask -from code_review_bot.tasks.infer import InferTask from code_review_bot.tasks.lint import MozLintTask from code_review_bot.tasks.tgdiff import TaskGraphDiffTask @@ -97,10 +95,6 @@ def test_taskcluster_index(mock_config, mock_workflow, mock_try_task): ("source-test-mozlint-whatever", MozLintTask, True), ("source-test-clang-format", ClangFormatTask, False), ("source-test-clang-format", ClangFormatTask, True), - ("source-test-coverity-coverity", CoverityTask, False), - ("source-test-coverity-coverity", CoverityTask, True), - ("source-test-infer-infer", InferTask, False), - ("source-test-infer-infer", InferTask, True), ("source-test-taskgraph-diff", TaskGraphDiffTask, False), ("source-test-taskgraph-diff", TaskGraphDiffTask, True), ("source-test-unsupported", DefaultTask, False), diff --git a/docs/architecture.md b/docs/architecture.md index 1a6b0b09..934ea01c 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -50,8 +50,6 @@ We currently support several analyzers used in Mozilla central: - clang-format - clang-tidy -- coverity -- infer - mozlint (bringing a huge list of analyzers like flake8, eslint, rustfmt, ...) All those analyzers output a different JSON payload, hence the need for different implementations. But we also have a [default format](analysis_format.md) that is recommended if you want to add a new analyzer. diff --git a/docs/projects/bot.md b/docs/projects/bot.md index 87de92fb..083f6baf 100644 --- a/docs/projects/bot.md +++ b/docs/projects/bot.md @@ -56,18 +56,6 @@ This supports the `source-test-clang-format` task from mozilla-central, and pars It will output a list of `ClangFormatIssue`, reporting formatting warnings, with fixes provided to the developer. -### Coverity - -This supports the Coverity integration from mozilla-central and NSS, and parses the custom artifact `public/code-review/coverity.json`. - -It will output a list of `CoverityIssue`, reporting static analysis warnings and build errors. - -### Infer - -This supports the `source-test-infer` from mozilla-central, and parses the custom artifact `public/code-review/infer.json`. - -It will output a list of `InferIssue` reporting static analysis and formatting issues in Java source code. - ### Mozlint This supports all the Mozlint tasks (and there are a lot!) from mozilla-central. It parses the custom artifact and mozlint json format defined in `public/code-review/mozlint.json`. diff --git a/docs/publication.md b/docs/publication.md index eebfeb28..72aab0dd 100644 --- a/docs/publication.md +++ b/docs/publication.md @@ -2,7 +2,7 @@ ## Build analysis output -The code-review bot currently supports 6 different formats to report issues (clang-tidy, clang-format, zero coverage, coverity, infer and mozlint). +The code-review bot currently supports 6 different formats to report issues (clang-tidy, clang-format, zero coverage and mozlint). We are in the process of standardizing toward [a single format described here](analysis_format.md). ### Taskcluster artifacts diff --git a/docs/trigger.md b/docs/trigger.md index 0a995a96..1f40a9e3 100644 --- a/docs/trigger.md +++ b/docs/trigger.md @@ -21,4 +21,4 @@ Ask the [linter-reviewers group](https://phabricator.services.mozilla.com/projec [NSS](https://phabricator.services.mozilla.com/source/nss/) is already integrated in the code-review bot workflow, using its own decision task (no taskgraph here). -To add a new analyzer, you'll need to add a task in `automation/taskcluster/graph/src/extend.js`, with the tag `code-review`. You can lookup the `coverity` for a sample implementation. +To add a new analyzer, you'll need to add a task in `automation/taskcluster/graph/src/extend.js`, with the tag `code-review`. You can lookup the `clang-tidy` for a sample implementation.