Conversation
|
@nodejs/dns @nodejs/testing |
|
@KeeReal Welcome and thanks for the pull request. Are you able to point to the lines of code that are currently uncovered that this will cover? Or at least what file those lines of code would be in? |
|
Hi @Trott. |
|
@joyeecheung (just in case you want to test on any particularly restrictive networks--I tested with the network disconnected entirely and the test worked fine, which I suppose is a good indication that the monkey-patching has the intended effect, but just in case....) |
|
@KeeReal were you able to run coverage locally to make sure that you hit the target lines: NODE_V8_COVERAGE=./cov ./node test/parallel/test-dns-lookup-promises.js
./node_modules/.bin/c8 report --reporter=html \
--temp-directory=./cov --omit-relative=false \
--resolve=./lib --exclude="benchmark/" --exclude="deps/" --exclude="test/" --exclude="tools/" \
--wrapper-length=0
open coverage/index.html |
bcoe
left a comment
There was a problem hiding this comment.
this looks good to me, conditional on confirming that this test exercises the expected code.
|
Thx, @bcoe
|
Adding tests covering promises-related code paths.
beb6e46 to
494f32d
Compare
BridgeAR
left a comment
There was a problem hiding this comment.
RSLGTM
Mocking is definitely not ideal but this part can probably not be tested properly otherwise.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
CI: https://ci.nodejs.org/job/node-test-pull-request/27587/ ✅ (yellow build with two typical Windows flakes) |
Adding tests covering promises-related code paths. PR-URL: #30777 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
|
Landed in 2d13896 🎉 |
Adding tests covering promises-related code paths. PR-URL: #30777 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Adding tests covering promises-related code paths. PR-URL: #30777 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Adding tests covering promises-related code paths. PR-URL: #30777 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Adding tests covering promises-related code paths.
missing coverage
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes