feat: control fail severity and result display #540
Conversation
philsturgeon
force-pushed
the
feat/error-only-error
branch
2 times, most recently
from
6a9e2c6
to
b452e9d
Compare
philsturgeon
force-pushed
the
feat/error-only-error
branch
2 times, most recently
from
df663cd
to
f817006
Compare
philsturgeon
force-pushed
the
feat/error-only-error
branch
from
f817006
to
b20865b
Compare
| --version Show version number [boolean] | ||
| --help Show help [boolean] | ||
| --encoding, -e text encoding to use [string] [default: "utf8"] | ||
| --format, -f formatter to use for outputting results [string] [default: "stylish"] |
There was a problem hiding this comment.
Is the lack of available formats expected?
There was a problem hiding this comment.
it's not no. damn where'd they go.
There was a problem hiding this comment.
This PR did not remove them, the test harness shows that develop branch is missing them already. Let's carry on with this merge and fix them in another PR.
| --skip-rule, -s ignore certain rules if they are causing trouble [string] | ||
| --fail-severity, -F results of this level or above will trigger a failure exit code | ||
| [string] [choices: "error", "warn", "info", "hint"] [default: "hint"] | ||
| --display-only-failures, -D only output results equal to or greater than --fail-severity |
There was a problem hiding this comment.
greater sounds a bit unfortunate, given DiagnosticSeverity.Error equals 0, Warn 1 etc.
I know what the intention is, but perhaps there is a way to make it more clear, i.e. more severe or higher/lower?
There was a problem hiding this comment.
greater and higher are synonomous and i dont think anyone knows what the internal ints are? This is probably find, if anyone gets confused we can ask them for suggestions.
| }, | ||
| 'display-only-failures': { | ||
| alias: 'D', | ||
| description: 'only output results equal to or greater than --fail-severity', |
| .then(results => { | ||
| if (results.length) { | ||
| process.exitCode = 1; | ||
| process.exitCode = severeEnoughToFail(results, failSeverity) ? 1 : 0; |
There was a problem hiding this comment.
| process.exitCode = severeEnoughToFail(results, failSeverity) ? 1 : 0; | |
| process.exitCode = isSevereEnoughToFail(results, failSeverity) ? 1 : 0; |
| const expectedStatus = replaceVars(scenario.status.trim(), replacements); | ||
| const expectedStdout = replaceVars(scenario.stdout.trim(), replacements); | ||
| const expectedStderr = replaceVars(scenario.stderr.trim(), replacements); | ||
| const status = commandHandle.status; |
There was a problem hiding this comment.
let's maybe differentiate between expected and actual to be consistent? here, actualStatus would tell more than just status
There was a problem hiding this comment.
The actualStatus is the status. I think t's ok.
Co-Authored-By: lag-of-death <[email protected]> Co-Authored-By: Jakub Rożek <[email protected]>
philsturgeon
dismissed stale reviews from lag-of-death and P0lip
via
449551e
Fixes #368
Let folks control what results are displayed, and what level off severity constitutes a failure. This PR respects the existing behavior and does not break BC, but we should raise the default
--fail-severitytowarnin v5.0.Checklist
Does this PR introduce a breaking change?
Screenshots