The Wayback Machine - https://web.archive.org/web/20171129044519/https://github.com/nodejs/node/pull/9565
Skip to content
Learn more
Please note that GitHub no longer supports old versions of Internet Explorer.

We recommend upgrading to the latest Microsoft Edge, Google Chrome, or Firefox.

If you are using IE 11, make sure you turn off "Compatibility View".

Sign in or Sign up

/node

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

test: improve test-stream2-objects.js. #9565

Closed
wants to merge 1 commit into
from

Conversation

Reviewers

@shigeki shigeki

@addaleax addaleax

@lpinca lpinca

@cjihrig cjihrig

Assignees
No one assigned
Labels
land-on-v4.x land-on-v6.x stream test
Projects
None yet
Milestone
No milestone
8 participants
@kt3k @shigeki @addaleax @lpinca @cjihrig @nodejs-github-bot @mscdex @MylesBorins
@kt3k
Contributor

kt3k commented Nov 12, 2016

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

test, stream

Description of change

This commit improves the assertions of
test-stream2-objects.js.

This is a part of Code And Learn at NodeFest 2016 Challenge
nodejs/code-and-learn#58

@nodejs-github-bot nodejs-github-bot added the test label Nov 12, 2016

@shigeki
Contributor

shigeki commented Nov 12, 2016

CI is running on https://ci.nodejs.org/job/node-test-pull-request/4826/

@mscdex mscdex added the stream label Nov 12, 2016

@shigeki

shigeki approved these changes Nov 12, 2016
View changes

One CI error is in Centos7-64 due to missing g++ in the build environment. Others are fine.
LGTM.

@shigeki

shigeki requested changes Nov 12, 2016
View changes

Could you revise your commit message to describe your improvement of using strict assert checks?

@kt3k
Contributor

kt3k commented Nov 12, 2016

Sure, of course!
@kt3k
Contributor

kt3k commented Nov 12, 2016

I added description of the change in commit message.
@shigeki

shigeki approved these changes Nov 12, 2016
View changes

Thanks. LGTM again.

test/parallel/test-stream2-objects.js
@@ -232,7 +232,7 @@ test('high watermark push', function(t) {
r._read = function(n) {};
for (var i = 0; i < 6; i++) {
var bool = r.push(i);
- assert.equal(bool, i === 5 ? false : true);
+ assert.strictEqual(bool, i === 5 ? false : true);
@cjihrig

cjihrig Nov 12, 2016

Contributor

Could this just be assert.strictEqual(bool, i !== 5);?

@kt3k

kt3k Nov 12, 2016

Contributor

Agree. I'll change it.

@kt3k
test: improve test-stream2-objects.js 
This commit improves the test cases in
test-stream2-objects.js by using assert.strictEqual
instead of assert.equal.

This is a part of Code And Learn at NodeFest 2016
nodejs/code-and-learn#58
24514d7
@addaleax

addaleax approved these changes Nov 12, 2016
View changes

LGTM

shigeki pushed a commit that referenced this pull request Nov 13, 2016

@kt3k
test: improve test-stream2-objects.js 
This commit improves the test cases in
test-stream2-objects.js by using assert.strictEqual
instead of assert.equal.

This is a part of Code And Learn at NodeFest 2016

Fixes: nodejs/code-and-learn#58
PR-URL: #9565
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
8ca322d
@shigeki
Contributor

shigeki commented Nov 13, 2016

Thanks. Landed in 8ca322d.
Congrats on your first contribution to Node.

@shigeki shigeki closed this Nov 13, 2016

@kt3k kt3k deleted the kt3k:feature/improve-test-assertion-code-and-learn branch Nov 13, 2016

@kt3k
Contributor

kt3k commented Nov 13, 2016

Thanks!

addaleax added a commit that referenced this pull request Nov 22, 2016

@kt3k @addaleax
test: improve test-stream2-objects.js 
This commit improves the test cases in
test-stream2-objects.js by using assert.strictEqual
instead of assert.equal.

This is a part of Code And Learn at NodeFest 2016

Fixes: nodejs/code-and-learn#58
PR-URL: #9565
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This commit was signed with a verified signature.
@addaleax addaleax Anna Henningsen
GPG key ID: D8B9F5AEAE84E4CF Learn about signing commits
e7eb9cc

addaleax added a commit to addaleax/node that referenced this pull request Dec 8, 2016

@kt3k @addaleax
test: improve test-stream2-objects.js 
This commit improves the test cases in
test-stream2-objects.js by using assert.strictEqual
instead of assert.equal.

This is a part of Code And Learn at NodeFest 2016

Fixes: nodejs/code-and-learn#58
PR-URL: nodejs#9565
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This commit was signed with a verified signature.
@addaleax addaleax Anna Henningsen
GPG key ID: D8B9F5AEAE84E4CF Learn about signing commits
92f8073

@addaleax addaleax added the land-on-v6.x label Dec 8, 2016

MylesBorins added a commit that referenced this pull request Dec 20, 2016

@kt3k @MylesBorins
test: improve test-stream2-objects.js 
This commit improves the test cases in
test-stream2-objects.js by using assert.strictEqual
instead of assert.equal.

This is a part of Code And Learn at NodeFest 2016

Fixes: nodejs/code-and-learn#58
PR-URL: #9565
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
eca12d4

@MylesBorins MylesBorins added the land-on-v4.x label Dec 20, 2016

MylesBorins added a commit that referenced this pull request Dec 21, 2016

@kt3k @MylesBorins
test: improve test-stream2-objects.js 
This commit improves the test cases in
test-stream2-objects.js by using assert.strictEqual
instead of assert.equal.

This is a part of Code And Learn at NodeFest 2016

Fixes: nodejs/code-and-learn#58
PR-URL: #9565
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
90027c3

MylesBorins added a commit that referenced this pull request Dec 21, 2016

@kt3k @MylesBorins
test: improve test-stream2-objects.js 
This commit improves the test cases in
test-stream2-objects.js by using assert.strictEqual
instead of assert.equal.

This is a part of Code And Learn at NodeFest 2016

Fixes: nodejs/code-and-learn#58
PR-URL: #9565
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
56677b6

This was referenced Dec 21, 2016

Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can't perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.
Morty Proxy This is a proxified and sanitized view of the page, visit original site.