Skip to content

test: move benchmark-dgram to sequential - #21144

Closed
apapirovski wants to merge 1 commit into
nodejs:masterfrom
apapirovski:fix-move-dgram-to-sequential
Closed

test: move benchmark-dgram to sequential#21144
apapirovski wants to merge 1 commit into
nodejs:masterfrom
apapirovski:fix-move-dgram-to-sequential

Conversation

@apapirovski

Copy link
Copy Markdown
Contributor

This test uses hardcoded ports, it should not be located in parallel.

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

This test uses hardcoded ports, it should not be located in parallel.
@apapirovskiapapirovski added the test Issues and PRs related to the tests. label Jun 5, 2018
@apapirovski

Copy link
Copy Markdown
ContributorAuthor

@apapirovski

Copy link
Copy Markdown
ContributorAuthor

Please 👍 here to approve fast-tracking.

@trivikrtrivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 6, 2018
@TrottTrott added the fast-track PRs that do not need to wait for 48 hours to land. label Jun 6, 2018

construnBenchmark=require('../common/benchmark');

// Because the dgram benchmarks use hardcoded ports, this should be in

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: Maybe mention common.PORT?

@TrottTrott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM with or without my nit addressed. +1 to fast-tracking.

@Trott

Trott commented Jun 6, 2018

Copy link
Copy Markdown
Member

Landed in d0943bc

@TrottTrott closed this Jun 6, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Jun 6, 2018
This test uses hardcoded ports, it should not be located in parallel.
PR-URL: nodejs#21144
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 6, 2018
This test uses hardcoded ports, it should not be located in parallel.
PR-URL: #21144
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Jun 6, 2018
@apapirovski
apapirovski deleted the fix-move-dgram-to-sequential branch June 7, 2018 13:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.fast-trackPRs that do not need to wait for 48 hours to land.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@apapirovski@Trott@targos@cjihrig@joyeecheung@trivikr