Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions bot/code_review_bot/report/phabricator.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ def __init__(self, configuration={}, *args, **kwargs):
self.mode = configuration.get("mode", MODE_COMMENT)
self.publish_build_errors = configuration.get("publish_build_errors", False)
assert self.mode in (MODE_COMMENT, MODE_HARBORMASTER), "Invalid mode"
logger.info("Will publish using", mode=self.mode)
logger.info(
"Will publish using", mode=self.mode, build_errors=self.publish_build_errors
)

def setup_api(self, api):
assert isinstance(api, PhabricatorAPI)
Expand Down Expand Up @@ -71,17 +73,20 @@ def publish(self, issues, revision):
for patch in revision.improvement_patches
if patch.analyzer in analyzers_available
]
# List of issues without possible build errors
issues_only = [issue for issue in issues if not issue.is_build_error()]
Comment thread
abpostelnicu marked this conversation as resolved.
build_errors = [issue for issue in issues if issue.is_build_error()]

if issues:
if issues_only or build_errors:
if self.mode == MODE_COMMENT:
self.publish_comment(revision, issues, patches)
self.publish_comment(revision, issues_only, patches)
Comment thread
abpostelnicu marked this conversation as resolved.
elif self.mode == MODE_HARBORMASTER:
self.publish_harbormaster(revision, issues)
self.publish_harbormaster(revision, issues_only)
else:
raise Exception("Unsupported mode {}".format(self.mode))
# Also publish build issues
if self.publish_build_errors:
Comment thread
abpostelnicu marked this conversation as resolved.
self.publish_harbormaster_build_errors(revision, issues)
self.publish_harbormaster_build_errors(revision, build_errors)
else:
logger.info("No issues to publish on phabricator")

Expand All @@ -91,17 +96,13 @@ def publish_harbormaster_build_errors(self, revision, issues):
"""
Publish build errors through Phabricator UnitResult
"""
build_errors = [
issue.as_phabricator_unitresult()
for issue in issues
if issue.is_build_error()
]

if not build_errors:
if not issues:
logger.info("No build errors encountered")
return
self.api.update_build_target(
revision.build_target_phid, BuildState.Fail, unit=build_errors
revision.build_target_phid,
BuildState.Fail,
unit=[issue.as_phabricator_unitresult() for issue in issues],
)

def publish_comment(self, revision, issues, patches):
Expand All @@ -124,7 +125,6 @@ def publish_comment(self, revision, issues, patches):
self.comment_inline(revision, issue, existing_comments)
for issue in issues
if issue.ANALYZER not in ANALYZERS_WITHOUT_INLINES
and not issue.is_build_error()
],
)
)
Expand Down
6 changes: 4 additions & 2 deletions bot/code_review_bot/tasks/coverity.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def validates(self):
"""
Publish only local Coverity issues
"""
return self.is_local() and not self.is_clang_error()
return self.is_local()

def as_text(self):
"""
Expand Down Expand Up @@ -188,11 +188,13 @@ def as_phabricator_unitresult(self):
if not self.build_error:
raise Exception("Current issue is not a build error: {}".format(self))

message = f"Code review bot found a **build error**: \n{self.message}"

return UnitResult(
namespace="code-review",
name="general",
result=UnitResultState.Fail,
details=self.message,
details=message,
format="remarkup",
)

Expand Down
2 changes: 1 addition & 1 deletion bot/tests/test_remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ def test_coverity_task(mock_config, mock_revision, mock_workflow, mock_stats):
namespace="code-review",
name="general",
result=UnitResultState.Fail,
details=issue.message,
details=f"Code review bot found a **build error**: \n{issue.message}",
format="remarkup",
)

Expand Down
14 changes: 2 additions & 12 deletions bot/tests/test_reporter_phabricator.py
Original file line number Diff line number Diff line change
Expand Up @@ -559,17 +559,7 @@ def _check_message(request):
details = json.loads(payload["params"][0])
assert details == {
"buildTargetPHID": "PHID-HMBD-deadbeef12456",
"lint": [
{
"code": "coverity.NULL_RETURNS",
"name": "Checker reliability is medium, meaning that the false "
"positive ratio is medium.\n"
'Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport".',
"line": 41,
"path": "test.cpp",
"severity": "error",
}
],
Comment thread
abpostelnicu marked this conversation as resolved.
"lint": [],
"unit": [],
"type": "work",
"__conduit__": {"token": "deadbeef"},
Expand All @@ -594,7 +584,7 @@ def _check_unitresult(request):
"lint": [],
"unit": [
{
"details": 'Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport".',
"details": 'Code review bot found a **build error**: \nDereferencing a pointer that might be "nullptr" "env" when calling "lookupImport".',
"format": "remarkup",
"name": "general",
"namespace": "code-review",
Expand Down