1956871 - Phabricator did not surface a lint job that both failed and did not produce a `mozlint.json`

Status: NEW

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Nick Alexander :nalexander [he/him]]

A developer on mobile had a patch backed out due to a failing lint on autoland. I investigated why that was and it's a known issue: until we address Bug 1891723, we won't parse the messages into the correct output artifact.

However: Phab could do a lot more to expose this issue! First, the lint job failed, and second, it did not produce a mozlint.json. In some way shape or form, Phab needs to expose this misconfiguration to the developer.

Comment 1

[Nick Alexander :nalexander [he/him]]

ahal: assuming that it's cheaper to change stuff in-tree than make Phab better, is it possible to make the mozlint harness produce a mozlint.json talking about some systematic failure?

Although, looking deeper, I see that https://searchfox.org/mozilla-central/rev/ac605820636c3b964542a2c0589af04a02235d00/taskcluster/kinds/source-test/android-lint.yml doesn't use mozlint at all -- and that's probably the root issue. But my question still stands -- is it possible to remove this failure mode from our Phab/lint integration entirely?

Comment 2

[Andrew Halberstadt [:ahal]]

It sounds like bug 1891723 tracks the proper fix here already.

It would be possible to add tasks to reviewbot without going through mozlint if that was necessary for some reason, but seems better to just make these standard lint tasks.

But for completeness, you'd have to create a something like:
https://github.com/mozilla/code-review/blob/master/bot/code_review_bot/tasks/lint.py

If going this route, there would be no need to implement a "mozlint.json", you could invent whatever artifact you wanted or even just parse the log.

Comment 3

[Aaditya Dhingra [:adhingra]]

Hey, just found another weird case which needs your help!

Linting task shows it ran successfully when some tasks failed. Example: https://treeherder.mozilla.org/jobs?repo=try&revision=a4f8f46907b1f08163d42d8fc1ca193fd1a8b4b9&selectedTaskRun=NhUDxQJ8TjedT1xzv5fADg.0 failed three times. Log: https://treeherder.mozilla.org/logviewer?job_id=501334839&repo=try&lineNumber=3271-3291

Before even getting to reporting the specific error, is there a way that this atleast turns orange ? How can we fix this? Thanks!

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Andrew Halberstadt [:ahal] from comment #2)

It sounds like bug 1891723 tracks the proper fix here already.

It would be possible to add tasks to reviewbot without going through mozlint if that was necessary for some reason, but seems better to just make these standard lint tasks.

But for completeness, you'd have to create a something like:
https://github.com/mozilla/code-review/blob/master/bot/code_review_bot/tasks/lint.py

If going this route, there would be no need to implement a "mozlint.json", you could invent whatever artifact you wanted or even just parse the log.

Thanks, :ahal. I agree that we should just go through mozlint. But I've also filed https://github.com/mozilla/code-review/issues/2684 to make the code-review bot complain when it doesn't get the expected mozlint.json artifact to consume.

Comment 5

[Nick Alexander :nalexander [he/him]]

(In reply to Aaditya Dhingra [:adhingra] from comment #3)

Hey, just found another weird case which needs your help!

Linting task shows it ran successfully when some tasks failed. Example: https://treeherder.mozilla.org/jobs?repo=try&revision=a4f8f46907b1f08163d42d8fc1ca193fd1a8b4b9&selectedTaskRun=NhUDxQJ8TjedT1xzv5fADg.0 failed three times. Log: https://treeherder.mozilla.org/logviewer?job_id=501334839&repo=try&lineNumber=3271-3291

Before even getting to reporting the specific error, is there a way that this atleast turns orange ? How can we fix this? Thanks!

I think the issue here is in our configuration, which is unusual -- we run multiple lints in a single job, via gradle, and that returns an exit code that is ignored. If that raised a Python exception or similar, you'd get more feedback. I'm not sure exactly how mozlint wants to handle this, but if we want failing to build to be an error, we need to change something :)

Comment 6

[Andrew Halberstadt [:ahal]]

I added a comment to that issue:
https://github.com/mozilla/code-review/issues/2684#issuecomment-2776057870

Fundamentally it seems like the questions are, "Are these tasks actually lints?" and "Is it better to shoe-horn them into mozlint, or treat them as their own task type?". I'm not sure of the answer to these questions.

I'll note that reviewbot is hardcoded to only parse mozlint.json if the task label starts with source-test-mozlint:
https://github.com/mozilla/code-review/blob/357a657c65b3b3c8910d6ee042052ff395417190/bot/code_review_bot/workflow.py#L649

I recommend we don't do something like manually generate mozlint.json for tasks that aren't actually running through mozlint.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:andi, could you have a look please?

For more information, please visit BugBot documentation.