chore: migrate from tap to node:test and c8#117
Conversation
test/index.test.js
Outdated
|
|
||
| await fastify.ready() | ||
| t.equal(fastify.elastic.name, 'elasticsearch-js') | ||
| assert.equal(fastify.elastic.name, 'elasticsearch-js') |
There was a problem hiding this comment.
you should use t.assert.equal etc. and not directly assert
There was a problem hiding this comment.
Accessing t.assert doesn't work as it ends up giving TypeError [Error]: Cannot read properties of undefined (reading 'equal').
Any idea on how to solve it?
There was a problem hiding this comment.
Done with 85ac08f, but I'm not able to run tests locally anymore because of the error reported previously.
There was a problem hiding this comment.
you should use t.assert.equal etc. and not directly assert
Hey @Uzlopak, thanks for the review 😁 Could you explain the rationale behind using t.assert instead of assert?
There was a problem hiding this comment.
btw,
I've just tested, and the context assert is not present in Node 20.12 but is present in 20.17.
It is also not exposed in Node 18.
I suggest using import assert from 'node:assert' if we want to maintain CI compatibility for Node 18, 20, etc.
There was a problem hiding this comment.
I see that Node 18 was removed from plugins-ci-elasticsearch.yml few months back.
Maybe we should move from the v4.2.0 to a more recent one? v5.0.0 seems to contain the Node 18 removal.
There was a problem hiding this comment.
The difference between t.assert and assert is that t.assert works with t.plan for assertion counting.
I think in this case does not make any difference.
gurgunday
left a comment
gurgunday
left a comment
There was a problem hiding this comment.
Thanks!
Please see: #117 (comment)
With the very same error I got locally |
gurgunday
left a comment
gurgunday
left a comment
There was a problem hiding this comment.
You should be able to rebase
Co-Authored-By: Pietro Marchini <[email protected]>
Co-Authored-By: Pietro Marchini <[email protected]>
tpreviero
force-pushed
the
tap-to-nodetest
branch
from
85ac08f to
d502a5b
Compare
|
FYI Node has native support for code coverage thresholds nodejs/node#54429 from Node 22 (which will become LTS the 29th of October) |
|
FYI Node has native support for code coverage threshold nodejs/node#54429 from Node 22 (which will become LTS the 29th of October) |
Then, it is not an option for us until it backported to |
It will not be backported unless its already in 20.18.0 |
I am only saying we cannot use this feature because it is not in the By that time, |
7 participants
Migrate from
taptonode:testandc8.Co-Authored-By: Pietro Marchini [email protected]
Checklist
npm run testandno benchmarksnpm run benchmarktests and/or benchmarks are includedno benchmarksand the Code of conduct