Skip to content

Conversation

@ghost
Copy link

@ghostghost commented Sep 17, 2018

Ref: #22849.

In short: Some unit tests are using string literals to simply tell you a
conclusion what's right/wrong BUT not tell you what actually values are.
So it's necessary to print them out in the console.

  • 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]

Ref: #22849. In short: Some unit tests are using string literals to simply tell you a conclusion what's right/wrong BUT not tell you what actually values are. So it's necessary to print them out in the console.
@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Sep 17, 2018
@danbev
Copy link
Contributor

Copy link
Member

@lundibundilundibundi left a comment

Choose a reason for hiding this comment

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

LGTM.

@Trott
Copy link
Member

@addaleax
Copy link
Member

Landed in ee31c28, thanks for the PR!

addaleax pushed a commit that referenced this pull request Sep 22, 2018
In short: Some unit tests are using string literals to simply tell you a conclusion what's right/wrong BUT not tell you what actually values are. So it's necessary to print them out in the console. Refs: #22849 PR-URL: #22891 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
@ghost ghost deleted the RemovingStringLiteralErrors branch September 23, 2018 00:48
@ghost
Copy link
Author

Thanks all for you help!

targos pushed a commit that referenced this pull request Sep 23, 2018
In short: Some unit tests are using string literals to simply tell you a conclusion what's right/wrong BUT not tell you what actually values are. So it's necessary to print them out in the console. Refs: #22849 PR-URL: #22891 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
@targostargos mentioned this pull request Oct 7, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@danbev@Trott@addaleax@jasnell@thefourtheye@lundibundi@nodejs-github-bot