Closed Bug 1316925 Opened 3 years ago Closed 3 years ago

"./mach eslint --setup" should not print out "0 errors, 0 warnings", if it fails & prints a warning/error about failing

Categories

(Firefox Build System :: Lint and Formatting, defect)

3 Branch
defect
Not set

Tracking

(firefox53 fixed)

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: dholbert, Assigned: ahal)

Details

Attachments

(1 file)

STR:
 (1) Don't have nodejs installed
 (2) Run ./mach eslint --setup

ACTUAL RESULTS:
Something like the following:
> nodejs v4.2.3 is either not installed or is installed to
> a non-standard path.
> Please install nodejs from https://nodejs.org and try again.
> 
> Valid installation paths:
>   - /usr/bin/nodejs
> ✖ 0 problems (0 errors, 0 warnings)

Note the last line in particular, which is very confusing in light of the error/warning directly before it.

EXPECTED RESULTS:
That "0 problems (0 errors, 0 warnings)" line should not be printed if we failed to find node (or failed to run/setup eslint due to other reasons).
Summary: "./mach eslint --setup" should not print out "0 errors, 0 warnings" if it fails due to an error → "./mach eslint --setup" should not print out "0 errors, 0 warnings", if it fails & prints a warning/error about failing
Agree it's confusing. Fwiw, it's a bit more nuanced because you could be running multiple linters at a time, some of which error out, and others which run successfully. I think a good solution is to count a non-zero return code as a "problem".

So in the case something goes wrong you might see:
✖ 1 problem (0 errors, 0 warnings, 1 exception)
That output would've made more sense to me, yeah.
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Comment on attachment 8810583 [details]
Bug 1316925 - Keep track of failed linters in stylish formatter summary,

https://reviewboard.mozilla.org/r/92866/#review94176

::: python/mozlint/mozlint/cli.py:109
(Diff revision 1)
>  
>      # Explicitly utf-8 encode the output as some of the formatters make
>      # use of unicode characters. This will prevent a UnicodeEncodeError
>      # on environments where utf-8 isn't the default
> -    print(formatter(results).encode('utf-8', 'replace'))
> -    return lint.return_code
> +    print(formatter(results, failed=lint.failed).encode('utf-8', 'replace'))
> +    return 1 if results or lint.failed else 0

I might be tempted to have different return codes for the two cases (failure vs lint errors) but not that important.
Attachment #8810583 - Flags: review?(james) → review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/127e45e5ba1d
Keep track of failed linters in stylish formatter summary, r=jgraham
https://hg.mozilla.org/mozilla-central/rev/127e45e5ba1d
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.