New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: Manage severity of 1 with TAP reporter (fixes #11110) #11221
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing! I've identified one typo.
I also have two other thoughts:
- The string "not ok" does have the string "ok" in it, so do we want to be more specific about asserting that the formatter does not say "not ok"? (If a message says "not ok", then
assert.include(message, "ok")
will still be true, so any regression might not be caught!) - Are there tests which show results for all errors, or a mix of warnings and errors? For the latter, I think I would expect a change (ok on warnings, not ok on errors), so it seems a new test should be added.
Let me know if you have any questions. Thanks again for working on this!
tests/lib/formatters/tap.js
Outdated
}] | ||
}]; | ||
|
||
it("should return a an warning string", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: a an
When using TAP reporter, both messages with severity of 1 or 2 were listed as errors (`not ok`). This changes aims at reporting warnings as `ok` messages.
1952cd5
to
5908433
Compare
Thanks for your feedback! First point must be fixed :) |
@platinumazure Friendly ping; has your feedback been addressed? |
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[X] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
As reported in #11110, result with severity of 1 (ie. warning) were reported as errors (not ok)
This will allow:
ok
not ok
Is there anything you'd like reviewers to focus on?
Nothing!