Skip to content
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

lib: added isNativeError check to assert.js #51241

Closed
wants to merge 2 commits into from

Conversation

NiharPhansalkar
Copy link
Contributor

Added the function for compliance with
frameworks such as Jest

Fixes: #50780

I am not sure if what my eslint did is okay in the commit diff.
The actual changes are in lines 75, 424 and 425.

Added the function for compliance with
frameworks such as Jest
@nodejs-github-bot nodejs-github-bot added assert Issues and PRs related to the assert subsystem. needs-ci PRs that need a full CI run. labels Dec 20, 2023
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are A LOT of unrelated changes, and no tests

@NiharPhansalkar
Copy link
Contributor Author

NiharPhansalkar commented Dec 21, 2023

@aduh95 I would like to ask, the code is working fine normally, it starts having issues with frameworks like Jest. So what kinds of tests can be expected to be added?

Till then I will undo the unrelated changes.

@NiharPhansalkar NiharPhansalkar deleted the assert-ok-fix branch December 21, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

assert.ok() throwing AssertionError instead of provided Error object
3 participants