Debugger node test failures should be shown in phabricator
Categories
(DevTools :: Debugger, task, P3)
Tracking
(Not tracked)
People
(Reporter: jlast, Unassigned)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
Bug 1574285 added support for kicking off debugger unit tests on phabricator pushes and reporting pass/fail. We should update the reviewbot reporter so that issues are shown in context
Reporter | ||
Comment 1•5 years ago
|
||
Notes from #go-node
slack discussion
the 2 requirements to integrate with code review bot are:
- have a task in CI taskgraph using the code-review attribute
- that task should apply the given revision to an MC checkout, run any analyzer on it, then publish a JSON list of issues as a Taskcluster artifact (that means writing a file on the FS)
FYI here is a random clang-tidy.json: https://taskcluster-artifacts.net/KYbwiuG9SDyXYGeTfRfzKQ/0/public/code-review/clang-tidy.json
then the last part is on my side, in the code review bot. Basically we need to add 2 Python classes that represent the analyzer & the issues you produce
here is the implementation for mozlint https://github.com/mozilla/code-review/blob/master/bot/code_review_bot/tasks/lint.py
hash is the md5 of the file at that revision
we do not use that
unfortunately we do not have an official format yet
the best one is the mozlint format
which you could reuse
Comment 2•5 years ago
|
||
The mozlint format has the following fields for each issue:
path
relative to the repositorycolumn
&lineno
where the issue is happening in the filelevel
(warning | error) of the issuerule
describing the issue detected (often a unique shorthand code)message
with all the details to provide to the developerlinter
if you have multiple analyzers using the same format
Of course you can add other relevant fields to your needs, but these should at least be present (maybe except linter)
Reporter | ||
Updated•5 years ago
|
Comment 3•5 years ago
|
||
This is currently crashing some code reviews as the publication is triggered but does not support yet your task's output.
I'll make a simple patch to silently avoid your task until you produce a JSON output.
Comment 4•5 years ago
|
||
I just released a new version on production, those crashes won't happen anymore. Instead we'll see a log message and no actions on these extra analyzers.
Reporter | ||
Comment 5•5 years ago
|
||
thanks
Comment 6•5 years ago
|
||
Shouldn't this be blocked by bug 1561292? If not, I'm curious as to why...
Reporter | ||
Comment 7•5 years ago
|
||
Yep
Comment 8•5 years ago
|
||
Switching this bug to depend on 1561292 rather than block it, which is I think what :jlast intended to do. :-)
Updated•2 years ago
|
Description
•