test: Promisify basic tests #1018
Conversation
albertpurnama
force-pushed
the
tests/util-promisify
branch
from
6370f91
to
37b507f
Compare
| ); | ||
| }) | ||
|
|
||
| // TODO: common.mustCall |
There was a problem hiding this comment.
Up to this point I decided to stop working on this test for now since it requires common.mustCall to be implemented for further testing.
Should I do this first?
There was a problem hiding this comment.
I think it will be valuable to land more tests into one PR and address the gaps as a follow up issue.
We will get a better understanding of what are the gaps by that point of time.
There was a problem hiding this comment.
Another idea is to use similar strategy as used here
bun/test/bun.js/setTimeout.test.js
Line 38 in 463f2b7
as an alternative of
common.mustCall
There was a problem hiding this comment.
hmm I see, I'll see what I can do with that. Thank you for the reference!
| ); | ||
| }) | ||
|
|
||
| // TODO: common.mustCall |
There was a problem hiding this comment.
I think it will be valuable to land more tests into one PR and address the gaps as a follow up issue.
We will get a better understanding of what are the gaps by that point of time.
| @@ -0,0 +1,81 @@ | |||
| import { describe, it } from "bun:test"; | |||
There was a problem hiding this comment.
Should we add the license header from the node tests into this?
|
Hey @soneymathew, I've requested review again! The error object returned from invalid input (see last test case) doesn't match from NodeJS's |
General Description
This PR is supposed to cover tests in here .
Link to discussion
TODOs