1575295 - Debugger node test failures should be shown in phabricator

Status: NEW

(DevTools :: Debugger, task, P3)

Description

[Jason Laster [:jlast]]

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

Comment 1

[Jason Laster [:jlast]]

Notes from #go-node slack discussion

the 2 requirements to integrate with code review bot are:

1. have a task in CI taskgraph using the code-review attribute
2. 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

[Bastien Abadie [:bastien]]

The mozlint format has the following fields for each issue:

• path relative to the repository
• column & lineno where the issue is happening in the file
• level (warning | error) of the issue
• rule describing the issue detected (often a unique shorthand code)
• message with all the details to provide to the developer
• linter 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)

Comment 3

[Bastien Abadie [:bastien]]

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

[Bastien Abadie [:bastien]]

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.

Comment 5

[Jason Laster [:jlast]]

thanks

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Shouldn't this be blocked by bug 1561292? If not, I'm curious as to why...

Comment 7

[Jason Laster [:jlast]]

Yep

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Switching this bug to depend on 1561292 rather than block it, which is I think what :jlast intended to do. :-)