Refactor test expectations to align with potential changes #1142
Conversation
The Node core assert module will likely soon change some error messages by combining individual error messages with an auto-generated one. This makes sure all tests pass in all cases.
| t.fail('should throw') | ||
| } catch (err) { | ||
| t.is(err.message, `The decorator 'plugin' is not present in Request`) | ||
| t.ok(/The decorator 'plugin' is not present in Request/.test(err.message)) |
There was a problem hiding this comment.
Because it was not used in other tests (they used this pattern). If requested, I can change that.
There was a problem hiding this comment.
I think it would be better to use the framework provided assertion. It will indicate that a test failed because the string does not match the provided regex. Using t.ok it will only report that a value was false instead of true.
|
What willl be the new message in this case? |
|
I do not understand how that PR in core could cause such a breakage. |
| t.plan(2) | ||
| try { | ||
| const f = require('..')('lol') // eslint-disable-line | ||
| require('..')('lol') // eslint-disable-line |
There was a problem hiding this comment.
I think we can remove the eslint comment
mcollina
left a comment
mcollina
left a comment
There was a problem hiding this comment.
I think this should change these lines instead:
Not the assertion.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Any updates here? |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
5 participants
The Node core assert module will likely soon change some error
messages by combining individual error messages with an auto-generated
one. This makes sure all tests pass in all cases.
Checklist
npm run testandnpm run benchmark