Skip to content
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

dns: fix trace_events name for resolveCaa() #35979

Merged
merged 1 commit into from Nov 9, 2020
Merged

Conversation

Trott
Copy link
Member

@Trott Trott commented Nov 5, 2020

A test was added for this in 6f34498 but because it was a test in the
internet directory, it was not run on CI and it was not noticed that
the test was failing. This fixes the error that was causing the test to
fail.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. labels Nov 5, 2020
@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 5, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 5, 2020
@Trott
Copy link
Member Author

Trott commented Nov 5, 2020

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member Author

Trott commented Nov 7, 2020

Internet test: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/17369/

Whoops, that was against master and shows the failure. Here's against this PR to show success:

Internet CI: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/17425/

@Trott
Copy link
Member Author

Trott commented Nov 8, 2020

@nodejs/testing @nodejs/dns This fixes the node-daily-master CI job. One more review/approval would be helpful.

A test was added for this in 6f34498 but because it was a test in the
`internet` directory, it was not run on CI and it was not noticed that
the test was failing. This fixes the error that was causing the test to
fail.

PR-URL: nodejs#35979
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott Trott merged commit 642174a into nodejs:master Nov 9, 2020
@Trott
Copy link
Member Author

Trott commented Nov 9, 2020

Landed in 642174a

@Trott Trott deleted the fix-dns-trace branch November 9, 2020 13:58
danielleadams pushed a commit that referenced this pull request Nov 9, 2020
A test was added for this in 6f34498 but because it was a test in the
`internet` directory, it was not run on CI and it was not noticed that
the test was failing. This fixes the error that was causing the test to
fail.

PR-URL: #35979
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@danielleadams danielleadams mentioned this pull request Nov 9, 2020
targos pushed a commit that referenced this pull request May 1, 2021
A test was added for this in 6f34498 but because it was a test in the
`internet` directory, it was not run on CI and it was not noticed that
the test was failing. This fixes the error that was causing the test to
fail.

PR-URL: #35979
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants