1892731 - Consider connecting AC/Fenix/Focus Lint Checks to Reviewbot in Phabricator

Status: RESOLVED DUPLICATE of bug 1907129

(Fenix :: Tooling, enhancement)

Description

[Olivia Hall [:olivia]]

Lint failures in AC/Fenix/Focus currently don't show up in Phabricator. It would be nice to have this check via reviewbot, since it is another safety layer to prevent developers from inadvertently pushing code that will get backed out. Failures do show up in manual trys, but it is easy to overlook or have a "one line update" that introduces a lint issue.

(For example, like in this GeckoView Patch, reviewbot catches lint failures. However, this patch did not, except through manually noticing in try.)

Comment 1

[Olivia Hall [:olivia]]

(Tagging to monorepo-enhancements for triage, please move if out of scope.)

Comment 2

[Mark Banner (:standard8)]

I think there's potentially a couple of issues here. Taking this phabricator revision as a random example, with its associated try run:

• I'm not sure why the review bot isn't picking these up as "unknown failures" as the task runs are failing. They appear to be flagged as code-review (e.g. detekt). Marco, do you have any ideas here?
• If you look at the preview of the file-whitespace on treeherder, you'll notice the preview says TEST-UNEXPECTED-ERROR | ....
  • That means the output processor has found the error lines.
  • No preview appears on the detekt & ktlint failures (example

I believe the first part should get code review bot reporting that there's issues, I think the second part would be what's required to get it supporting the specific details of those issues - it would certainly make it easier when looking at treeherder.

For the second part, the error summary lines would need to match this regexp

Comment 3

[Marco Castelluccio [:marco]]

The bot doesn't know how to parse the detekt task artifacts.

There are two ways to go about it:

• (PREFERRED) Make the task itself output the issues in the standard format used by the code review bot (see https://github.com/mozilla/code-review/blob/master/docs/analysis_format.md and https://github.com/mozilla/code-review/blob/084750a1ba5896db9a9c5787f857ad1b4683e1aa/bot/code_review_bot/tasks/default.py#L58).
• (STRONGLY DISCOURAGED) Add support for the code review bot to parse detekt artifacts (similarly to https://github.com/mozilla/code-review/blob/084750a1ba5896db9a9c5787f857ad1b4683e1aa/bot/code_review_bot/tasks/clang_tidy.py).