1428585 - Many lint tools do not check exit code

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement)

Description

[Sam Sneddon [:gsnedders]]

Found via bug 1428582, which investigating found that the wpt lint has been exiting abnormally (with a failing assertion) for who-knows-how-long.

It seems that the wpt lint, and several others, don't seem to check the exit code of the called subprocess, which seems bad.

Comment 1

[Sam Sneddon [:gsnedders]]

I wonder if this (in the WPT case) is the explanation for https://github.com/w3c/web-platform-tests/issues/8271 (Gecko exported tests sometimes contain lint errors)? We have bug 1428582 and quite probably other bugs where the lint throws an exception and hence doesn't run to completion, and nobody notices that this happens?

Comment 2

[James Graham [:jgraham]]

Do you have some example pushes where this happens? I can try to add a check for the lint exit code, but having some revision that fails to work against would help to ensure I atually fixed the prblem fully.

Comment 3

[Sam Sneddon [:gsnedders]]

If you don't have the patch for bug 1428582 and wpt lint has any files to lint it will raise an AssertionError.

Comment 4

[James Graham [:jgraham]]

Attachment: Bug 1428585 - Fail the wpt lint if the lint process has a non-zero exit code,

Review commit: https://reviewboard.mozilla.org/r/211334/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/211334/

Comment 5

[Andrew Halberstadt [:ahal]]

Comment on attachment 8941037 [details]
Bug 1428585 - Fail the wpt lint if the lint process has a non-zero exit code,

https://reviewboard.mozilla.org/r/211334/#review217102

::: tools/lint/wpt/wpt.py:31
(Diff revision 1)
> +    if files == [tests_dir]:
> +        print >> sys.stderr, "No files specified, running the full wpt lint (this is slow)"
> +        files = ["--all"]

Was this case actually happening? I think mozlint should prevent running this linter in the first place if there were no wpt related paths being linted.

::: tools/lint/wpt/wpt.py:42
(Diff revision 1)
> +        if proc.returncode != 0:
> +            results.append(result.from_config(config, message="Lint process failed"))

Alternatively you could return the returncode and mozlint will pick that up as a failure, but then if you had legit results to show, they would be lost. So this is probably better.

Maybe at least log the return code in the message.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/849d7fde04aa
Fail the wpt lint if the lint process has a non-zero exit code, r=ahal

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/83bfdce958e7
Fixup flake8 error, r=me ON A CLOSED TREE

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/849d7fde04aa
https://hg.mozilla.org/mozilla-central/rev/83bfdce958e7