-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Change assert module's deprecated methods in testing files #4435
Change assert module's deprecated methods in testing files #4435
Conversation
I'm working on test cases 😢 |
Please mark this PR as ready when ready. I know there's at least one place in our tests that must use |
Thank you for your feedback. I modified diffs.spec.js so that only one of the two outputs can be printed. also i changed Please let me know what needs to be modified. Thanks! I marked this PR. |
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. I think maybe better to just detect Node.js v10.x and skip the tests. So revert the changes to test/integration/diffs.spec.js
and use the same strategy as in test/integration/no-diff.spec.js
. do it in a before()
hook within the top-level describe()
.
it's worth mentioning (in the docs, maybe?) that Mocha's "no diff" behavior does not work if the diff is embedded in the message
prop of an Error
.
I reverted the changes to test/integration/diffs.spec.js and use skip tests in the file as in test/integration/no-diff.spec.js.
I'm considering mentioning this part of the docs that you told, so can I proceed with it in another PR? please let me know if it's okay to proceed like that. Thank you for your comments. |
3dc4489
to
00063b8
Compare
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.
LGTM
Netlify build failure seems not related to these changes. But I didn't find why Netlify build failed yet.
lgtm, thanks |
Requirements
Description of the Change
I removed deprecated methods and tested it.
equal, deepEqual
methods are deprecated since v.9.9.0. Assert ReferenceI used
strictEqual, deepStrictEqual
instead.Alternate Designs
deprecated
changed
For now, PR's just changed methods that was deprecated. I wonder if i can change code using unexpected library as shown below. I found this issue about migrating assert to Unexpected assertion library. I wonder if there are still
assert
left.assert
unexpected assertion library
Why should this be in core?
N/A
Benefits
Users can view the modified version of the function within code.
Possible Drawbacks
N/A
Applicable issues
patch release