-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
assert: fix internal logic judgment #27743
Changes from all commits
b87a772
4eb0f24
a9f7f54
cd85b5b
21a9f93
24e9212
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -68,6 +68,17 @@ const meta = [ | |||||
const escapeFn = (str) => meta[str.charCodeAt(0)]; | ||||||
|
||||||
let warned = false; | ||||||
function showMulParamDeprecation() { | ||||||
if (warned === false) { | ||||||
warned = true; | ||||||
process.emitWarning( | ||||||
'assert.fail() with more than one argument is deprecated. ' + | ||||||
'Please use assert.strictEqual() instead or only pass a message.', | ||||||
'DeprecationWarning', | ||||||
'DEP0094' | ||||||
); | ||||||
} | ||||||
} | ||||||
|
||||||
// The assert module provides functions that throw | ||||||
// AssertionError's when particular conditions are not met. The | ||||||
|
@@ -100,25 +111,23 @@ function fail(actual, expected, message, operator, stackStartFn) { | |||||
message = actual; | ||||||
actual = undefined; | ||||||
} else { | ||||||
if (warned === false) { | ||||||
warned = true; | ||||||
process.emitWarning( | ||||||
'assert.fail() with more than one argument is deprecated. ' + | ||||||
'Please use assert.strictEqual() instead or only pass a message.', | ||||||
'DeprecationWarning', | ||||||
'DEP0094' | ||||||
); | ||||||
} | ||||||
if (argsLen === 2) | ||||||
operator = '!='; | ||||||
showMulParamDeprecation(); | ||||||
} | ||||||
|
||||||
if (message instanceof Error) throw message; | ||||||
|
||||||
if (operator === undefined) { | ||||||
if (message) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
operator = 'fail'; | ||||||
} else { | ||||||
operator = '!='; | ||||||
} | ||||||
} | ||||||
BridgeAR marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
const errArgs = { | ||||||
actual, | ||||||
expected, | ||||||
operator: operator === undefined ? 'fail' : operator, | ||||||
operator, | ||||||
stackStartFn: stackStartFn || fail, | ||||||
message | ||||||
}; | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -61,3 +61,16 @@ assert.throws( | |||||||||||||||||||||||||||||||||
function foo() { assert.fail('first', 'second', 'message', '!==', foo); }, | ||||||||||||||||||||||||||||||||||
(err) => !/^\s*at\sfoo\b/m.test(err.stack) | ||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
// Actual undefined = Error | ||||||||||||||||||||||||||||||||||
assert.throws(() => { | ||||||||||||||||||||||||||||||||||
assert.fail(undefined); | ||||||||||||||||||||||||||||||||||
}, { | ||||||||||||||||||||||||||||||||||
code: 'ERR_ASSERTION', | ||||||||||||||||||||||||||||||||||
name: 'AssertionError', | ||||||||||||||||||||||||||||||||||
message: 'Failed', | ||||||||||||||||||||||||||||||||||
operator: 'fail', | ||||||||||||||||||||||||||||||||||
actual: undefined, | ||||||||||||||||||||||||||||||||||
expected: undefined, | ||||||||||||||||||||||||||||||||||
Comment on lines
+67
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Ideally, another identical test could be added that sets |
||||||||||||||||||||||||||||||||||
generatedMessage: true | ||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test currently passes without the other changes in this PR. Can we please add a test case that currently fails? |
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.
I suggest to change the
operator
description above to explain when it's set to'fail'
and when to'!='
.I would also move the
stackStartFn
description to the part above. That way the total description below concentrates on the general behavior instead of describing each individual option setting.Right now we also miss to document that
assert.fail(1, 2, new Error()
throws the passed through error. That could be described in themessage
property above.