FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: improve assert message for debugging by Trott · Pull Request #6581 · nodejs/node · GitHub

/ node Public

test: improve assert message for debugging - #6581

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:msg
ConversationCommits (1)ChecksFiles changed
Closed

test: improve assert message for debugging#6581
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:msg

Conversation

Trott commented May 4, 2016

Copy link
Copy Markdown
Member
Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

Previously, the assertion changed in this commit would print the same
message no matter which of the four test cases failed. The assert has
been changed so that it will indicate which test case failed.

Refs: #6577

test: improve assert message for debugging
2268660 
Previously, the assertion changed in this commit would print the same
message no matter which of the four test cases failed. The assert has
been changed so that it will indicate which test case failed.

Refs: nodejs#6577
Trott added the test Issues and PRs related to the tests. label May 4, 2016
Trott mentioned this pull request May 4, 2016
Closed

addaleax commented May 4, 2016

Copy link
Copy Markdown
Member

LGTM

2 similar comments

santigimeno commented May 4, 2016

Copy link
Copy Markdown
Member

LGTM

jasnell commented May 4, 2016

Copy link
Copy Markdown
Member

LGTM

mscdex added the dgram Issues and PRs related to the dgram subsystem / UDP. label May 4, 2016

mcollina commented May 5, 2016

Copy link
Copy Markdown
Member

LGTM

mcollina commented May 5, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

@Trott while you are at it, you should update the allocation of the buffers to the new method, like what was done for the ipv4 equivalent: https://github.com/nodejs/node/blob/master/test/parallel/test-dgram-send-default-host.js#L9-L12

JacksonTian commented May 5, 2016

Copy link
Copy Markdown
Contributor

LGTM

mcollina commented May 5, 2016

Copy link
Copy Markdown
Member

I think the commit message needs to be a little more explicative
Il giorno gio 5 mag 2016 alle 11:44 Jackson Tian notifications@github.com
ha scritto:

LGTM

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#6581 (comment)

cjihrig commented May 5, 2016

Copy link
Copy Markdown
Contributor

LGTM

jasnell commented May 6, 2016

Copy link
Copy Markdown
Member

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

addaleax commented May 6, 2016

Copy link
Copy Markdown
Member

If @Trott is okay with that, this should probably be not be landed in favour of #6607. This PR served its purpose well enough. :)

jasnell commented May 6, 2016

Copy link
Copy Markdown
Member

SGTM

Trott commented May 6, 2016

Copy link
Copy Markdown
Member Author

If @Trott is okay with that, this should probably be not be landed in favour of #6607.

I agree. Closing this one.

Trott closed this May 6, 2016
Trott deleted the msg branch January 13, 2022 22:43
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

dgram Issues and PRs related to the dgram subsystem / UDP. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL