Skip to content

Conversation

@kt3k
Copy link
Contributor

@kt3kkt3k 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-botnodejs-github-bot added the test Issues and PRs related to the tests. label Nov 12, 2016
@shigeki
Copy link
Contributor

@mscdexmscdex added the stream Issues and PRs related to the stream subsystem. label Nov 12, 2016
Copy link
Contributor

@shigekishigeki left a comment

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@shigekishigeki left a comment

Choose a reason for hiding this comment

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

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

@kt3k
Copy link
ContributorAuthor

kt3k commented Nov 12, 2016

Sure, of course!

@kt3kkt3kforce-pushed the feature/improve-test-assertion-code-and-learn branch from 9ac5134 to d6819deCompareNovember 12, 2016 08:55
@kt3k
Copy link
ContributorAuthor

kt3k commented Nov 12, 2016

I added description of the change in commit message.

Copy link
Contributor

@shigekishigeki left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM again.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Agree. I'll change it.

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
@kt3kkt3kforce-pushed the feature/improve-test-assertion-code-and-learn branch from d6819de to 24514d7CompareNovember 12, 2016 14:01
Copy link
Member

@addaleaxaddaleax left a comment

Choose a reason for hiding this comment

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

LGTM

shigeki pushed a commit that referenced this pull request Nov 13, 2016
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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@shigeki
Copy link
Contributor

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

@shigekishigeki closed this Nov 13, 2016
@kt3kkt3k deleted the feature/improve-test-assertion-code-and-learn branch November 13, 2016 02:11
@kt3k
Copy link
ContributorAuthor

kt3k commented Nov 13, 2016

Thanks!

addaleax pushed a commit that referenced this pull request Nov 22, 2016
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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
addaleax pushed a commit to addaleax/node that referenced this pull request Dec 8, 2016
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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 20, 2016
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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
This was referenced Dec 21, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

streamIssues and PRs related to the stream subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@kt3k@shigeki@addaleax@lpinca@cjihrig@mscdex@MylesBorins@nodejs-github-bot