Closed Bug 1316925 Opened 8 years ago Closed 8 years ago

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

Categories

(Developer Infrastructure :: Lint and Formatting, defect)

3 Branch
defect
Not set
normal

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
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: