Skip to content

test: improve test coverage of dns/promises #41133

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Dec 19, 2021

Conversation

kuriyosh
Copy link
Contributor

This improves a test coverage in lib/internal/dns/promises.
It tests emitting warning when the options types of lookup method are invalid.

ref: https://coverage.nodejs.org/coverage-18ff5832501b66b4/lib/internal/dns/promises.js.html#L116

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Dec 10, 2021
@Mesteery Mesteery added the dns Issues and PRs related to the dns subsystem. label Dec 10, 2021
@jasnell
Copy link
Member

jasnell commented Dec 10, 2021

Looks like there are some relevant test failures that need to be looked at! :-)

@kuriyosh kuriyosh force-pushed the dns-options-test branch 2 times, most recently from 0c67e16 to a197004 Compare December 11, 2021 01:59
});

assert.throws(() => {
dnsPromises.lookup('127.0.0.1', { hints: '-1' });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test only check whether DeprecationWarning is emitted when hints is not a number.
Since the valid string value of hints depends on the OS environment, give an invalid string value here and assert the error.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

@Ayase-252 Ayase-252 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 13, 2021
@aduh95 aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Dec 19, 2021
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 19, 2021
@nodejs-github-bot nodejs-github-bot merged commit 9b903d1 into nodejs:master Dec 19, 2021
@nodejs-github-bot
Copy link
Collaborator

Landed in 9b903d1

targos pushed a commit that referenced this pull request Jan 14, 2022
PR-URL: #41133
Refs: https://coverage.nodejs.org/coverage-18ff5832501b66b4/lib/internal/dns/promises.js.html#L116
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Qingyu Deng <i@ayase-lab.com>
@danielleadams
Copy link
Contributor

@kuriyosh This caused a failed test when pulling into the v16.x release. Do you mind creating a backport PR?

Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
PR-URL: nodejs#41133
Refs: https://coverage.nodejs.org/coverage-18ff5832501b66b4/lib/internal/dns/promises.js.html#L116
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Qingyu Deng <i@ayase-lab.com>
@kuriyosh
Copy link
Contributor Author

kuriyosh commented Feb 4, 2022

@danielleadams
Of course. Please ping me if I have any necessary actions.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. dns Issues and PRs related to the dns subsystem. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants