Skip to content

test: removed string literal on assert.strictEqual#16014

Closed
Saeed-Navarik wants to merge 1 commit into
nodejs:masterfrom
Saeed-Navarik:test-tls-pfx-gh-5100-regr
Closed

test: removed string literal on assert.strictEqual#16014
Saeed-Navarik wants to merge 1 commit into
nodejs:masterfrom
Saeed-Navarik:test-tls-pfx-gh-5100-regr

Conversation

@Saeed-Navarik
Copy link
Copy Markdown
Contributor

@Saeed-Navarik Saeed-Navarik commented Oct 6, 2017 •
edited by tniessen
Loading

removed string literal argument to function call strictEqual
so that default error message is printed

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

test
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 6, 2017
@Saeed-Navarik Saeed-Navarik changed the title test: removed msg on assert.strictEqual on test-tls-pfx-gh-5100-regr test: removed msg on assert.strictEqual Oct 6, 2017
@Saeed-Navarik Saeed-Navarik changed the title test: removed msg on assert.strictEqual test: removed string literal on assert.strictEqual Oct 6, 2017
@mscdex mscdex added the tls Issues and PRs related to the tls subsystem. label Oct 6, 2017
@joyeecheung
Copy link
Copy Markdown
Member

joyeecheung commented Oct 6, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/10435/
@Trott Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 7, 2017
gibfahn
gibfahn approved these changes Oct 7, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

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

One suggestion, but LGTM either way.
Comment thread test/parallel/test-tls-pfx-gh-5100-regr.js
null,
'authorizationError must be null'
null
);
Copy link
Copy Markdown
Member

@gibfahn gibfahn Oct 7, 2017

Choose a reason for hiding this comment

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

I think this could be reformatted to be on one line like:

assert.strictEqual(c.authorizationError, null);

If you could push a commit to your branch that makes this change then that would be great.
@joyeecheung
Copy link
Copy Markdown
Member

joyeecheung commented Oct 7, 2017 •
edited
Loading

Fixed @gibfahn 's comments and landed in 5843b2c, thanks for your contribution and welcome to Node.js!
@joyeecheung joyeecheung closed this Oct 7, 2017
joyeecheung pushed a commit that referenced this pull request Oct 7, 2017
@Saeed-Navarik @joyeecheung
test: remove messages in assert.strictEqual
5843b2c 
PR-URL: #16014
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this pull request Oct 11, 2017
@Saeed-Navarik @MylesBorins
test: remove messages in assert.strictEqual
1d7e1c0 
PR-URL: #16014
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@MylesBorins MylesBorins mentioned this pull request Oct 11, 2017
Merged
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 12, 2017
@Saeed-Navarik @addaleax
test: remove messages in assert.strictEqual
81844b7 
PR-URL: nodejs/node#16014
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this pull request Oct 17, 2017
@Saeed-Navarik @MylesBorins
test: remove messages in assert.strictEqual
c81b425 
PR-URL: #16014
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@MylesBorins MylesBorins mentioned this pull request Oct 17, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Oct 18, 2017
@Saeed-Navarik @MylesBorins
test: remove messages in assert.strictEqual
c308a30 
PR-URL: #16014
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
@Saeed-Navarik @MylesBorins
test: remove messages in assert.strictEqual
968cc44 
PR-URL: #16014
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@MylesBorins MylesBorins mentioned this pull request Nov 3, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. test Issues and PRs related to the tests. tls Issues and PRs related to the tls subsystem.

10 participants

@Saeed-Navarik @joyeecheung @benjamingr @cjihrig @tniessen @gibfahn @mscdex @MylesBorins @Trott @nodejs-github-bot