Skip to content

test: replace port in dgram send address types test - #13007

Closed
arturgvieira-zz wants to merge 1 commit into
nodejs:masterfrom
arturgvieira-zz:commonPort09-branch
Closed

test: replace port in dgram send address types test#13007
arturgvieira-zz wants to merge 1 commit into
nodejs:masterfrom
arturgvieira-zz:commonPort09-branch

Conversation

@arturgvieira-zz

@arturgvieira-zz arturgvieira-zz commented May 12, 2017 •
edited
Loading

Copy link
Copy Markdown

Replaced common.PORT in the following test.
test-dgram-send-address-types.js

Ref: #12376

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

test dgram

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to Node.js core tests and test infrastructure. label May 12, 2017
@hiroppy hiroppy added the dgram Issues and PRs related to UDP and the dgram module. label May 13, 2017
lpinca
lpinca reviewed May 13, 2017
View reviewed changes
Comment thread test/parallel/test-dgram-send-address-types.js Outdated

@lpinca lpinca May 13, 2017

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: can you please move this at the top of the file? before creating the socket.

lpinca
lpinca reviewed May 13, 2017
View reviewed changes
Comment thread test/parallel/test-dgram-send-address-types.js Outdated

@lpinca lpinca May 13, 2017

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.

Ditto.

@vieiraarturg
test: replace port in dgram send address types test
72dc6ae 
Replaced common.PORT in the following test.
test-dgram-send-address-types.js

Ref: #12376
@arturgvieira-zz

arturgvieira-zz commented May 13, 2017

Copy link
Copy Markdown
Author

@lpinca All done, I moved those two items towards the top of the file.

@lpinca

lpinca commented May 14, 2017

Copy link
Copy Markdown
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/8068/

@lpinca

lpinca commented May 22, 2017

Copy link
Copy Markdown
Member

Landed in 281de19.

@lpinca lpinca closed this May 22, 2017
lpinca pushed a commit that referenced this pull request May 22, 2017
@vieiraarturg @lpinca
test: use dynamic port in test-dgram-send-address-types
281de19 
Remove common.PORT from test-dgram-send-address-types to eliminate
possibility that a dynamic port used in another test will collide
with common.PORT.

PR-URL: #13007
Refs: #12376
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott Trott mentioned this pull request May 22, 2017
Closed
64 tasks
jasnell pushed a commit that referenced this pull request May 23, 2017
@vieiraarturg @jasnell
test: use dynamic port in test-dgram-send-address-types
d202407 
Remove common.PORT from test-dgram-send-address-types to eliminate
possibility that a dynamic port used in another test will collide
with common.PORT.

PR-URL: #13007
Refs: #12376
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
jasnell pushed a commit that referenced this pull request May 23, 2017
@vieiraarturg @jasnell
test: use dynamic port in test-dgram-send-address-types
9fc47de 
Remove common.PORT from test-dgram-send-address-types to eliminate
possibility that a dynamic port used in another test will collide
with common.PORT.

PR-URL: #13007
Refs: #12376
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@jasnell jasnell mentioned this pull request May 28, 2017
Closed
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpinca lpinca lpinca approved these changes
@cjihrig cjihrig cjihrig approved these changes

Assignees

No one assigned

Labels

dgram Issues and PRs related to UDP and the dgram module. 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.

7 participants

@arturgvieira-zz @lpinca @cjihrig @MylesBorins @hiroppy @nodejs-github-bot @vieiraarturg