GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — test: add http agent to executionAsyncResource by psj-tar-gz · Pull Request #34966 · nodejs/node

Conversation

@psj-tar-gz

@psj-tar-gz psj-tar-gz commented Aug 29, 2020

Copy link
Copy Markdown
Contributor

I added a testcase about executionAsyncResource with Http Agent.

Refs: https://github.com/nodejs/node/blob/master/test/async-hooks/test-async-exec-resource-http.js
Signed-off-by: psj-tar-gz [email protected]

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. test Issues and PRs related to Node.js core tests and test infrastructure. labels Aug 29, 2020
@psj-tar-gz
psj-tar-gz force-pushed the psj-dev branch from 79e0c09 to 4de4483 Compare August 29, 2020 09:06
Flarna
Flarna approved these changes Aug 29, 2020
View reviewed changes
@psj-tar-gz
psj-tar-gz force-pushed the psj-dev branch from 1fbcdc4 to 54c958c Compare September 4, 2020 11:50
@Flarna

Flarna commented Sep 4, 2020

Copy link
Copy Markdown
Member

Isn't it needed to execute at least two http requests to the same server to see the diff between use of the dedicated agent versus the global agent?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
addaleax
addaleax approved these changes Sep 5, 2020
View reviewed changes
Comment thread test/async-hooks/test-async-exec-resource-http-agent.js Outdated

@addaleax addaleax Sep 5, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can you wrap all the callbacks that are supposed to be called exactly once in common.mustCall(), i.e. the ones to http.createServer(), server.listen() and http.get()? That ensures that the assertions here are actually run, instead of the test just passing because something led to the callbacks not being called. (That’s probably not going to realistically happen because a ton of other tests would fail is these things were broken, but it’s a good practice to add those checks in our tests.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 5, 2020
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 5, 2020
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Nov 8, 2020
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 8, 2020
@nodejs-github-bot

nodejs-github-bot commented Nov 8, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/34193/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Flarna

Flarna commented Nov 9, 2020

Copy link
Copy Markdown
Member

@psj-tar-gz Linter complains about a few findings. A rebase to current master is maybe also a good idea.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95 aduh95 removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Nov 9, 2020
@nodejs-github-bot

nodejs-github-bot commented Jan 21, 2021

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/35650/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@guybedford
guybedford force-pushed the master branch from dc5a5da to 8e46568 Compare March 29, 2021 21:33
@Trott
Trott force-pushed the main branch from 2d76238 to ca3ed36 Compare November 12, 2022 01:49
@aduh95
aduh95 force-pushed the psj-dev branch 2 times, most recently from 4de4483 to 55f0830 Compare May 5, 2024 21:24
@aduh95 aduh95 added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label May 5, 2024
@nodejs-github-bot

nodejs-github-bot commented May 5, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58970/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 10, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59077/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@psj-tar-gz @aduh95
test: add http agent to executionAsyncResource
491855c 
Refs: https://github.com/nodejs/node/blob/HEAD/test/async-hooks/test-async-exec-resource-http.js
Signed-off-by: psj-tar-gz <[email protected]>
PR-URL: nodejs#34966
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@aduh95
aduh95 force-pushed the psj-dev branch from fdf899c to 491855c Compare May 11, 2024 08:45
@aduh95

aduh95 commented May 11, 2024

Copy link
Copy Markdown
Contributor

Landed in 491855c
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95
aduh95 merged commit 491855c into nodejs:main May 11, 2024
targos pushed a commit that referenced this pull request May 11, 2024
@psj-tar-gz @targos
test: add http agent to executionAsyncResource
0b073f8 
Refs: https://github.com/nodejs/node/blob/HEAD/test/async-hooks/test-async-exec-resource-http.js
Signed-off-by: psj-tar-gz <[email protected]>
PR-URL: #34966
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@targos targos mentioned this pull request May 13, 2024
v22.2.0 release proposal #52971
Merged
@marco-ippolito marco-ippolito mentioned this pull request Jun 17, 2024
v20.15.0 proposal #53486
Merged
marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
@psj-tar-gz @marco-ippolito
test: add http agent to executionAsyncResource
311bdc6 
Refs: https://github.com/nodejs/node/blob/HEAD/test/async-hooks/test-async-exec-resource-http.js
Signed-off-by: psj-tar-gz <[email protected]>
PR-URL: #34966
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024
@psj-tar-gz @soophoo
test: add http agent to executionAsyncResource
17ec6d1 
Refs: https://github.com/nodejs/node/blob/HEAD/test/async-hooks/test-async-exec-resource-http.js
Signed-off-by: psj-tar-gz <[email protected]>
PR-URL: nodejs#34966
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes
@Flarna Flarna Flarna approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

async_hooks Issues and PRs related to the async hooks subsystem. author ready PRs with CI started, the required approvals, and no outstanding review comments. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@psj-tar-gz @Flarna @nodejs-github-bot @aduh95 @addaleax
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.