-
Notifications
You must be signed in to change notification settings - Fork 909
test: move hooks.test.js to the node test runner #2185
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
Conversation
test/hooks.test.js
Outdated
function match (t, obj, expected) { | ||
const checkObj = Object.keys(expected).reduce((acc, key) => { | ||
acc[key] = obj[key] | ||
return acc | ||
}, {}) | ||
|
||
t.assert.deepStrictEqual(checkObj, expected) | ||
} |
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.
It's not available in v20 yet, but I would prefer we use nodejs/node#54630. In the interim, we can copy https://github.com/newrelic/node-newrelic/blob/ebf3aa946be1e6802b567f2bfeef0b0c13613375/test/lib/custom-assertions/match.js into a file like test/assert-match.js
. I'm sure it will be used in more suites than this one.
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.
@jsumners done. Other PRs need this match
assertion. I'll wait for this one to be merged before refactoring them.
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.
Aw hell. I just realized the match function is licensed Apache-2.0 and Pino is MIT. I think we can include it, but we might need to update the licenses file. @mcollina do you know?
Can we get match published as an npm module? Then it’d be less problematic. (This code is only used in tests and not really a problem anyway). |
That's a fair solution. I can do it with my account after work. |
@Puppo please use https://www.npmjs.com/package/@jsumners/assert-match npm install -D @jsumners/assert-match |
Done! |
This PR moves
hooks.test.js
to the node test runner