Skip to content
New issue

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

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

test: swap arguments in strictEqual() for parallel/test-buffer-copy #23204

Closed
wants to merge 1 commit into from

Conversation

DeltaEvo
Copy link
Contributor

@DeltaEvo DeltaEvo commented Oct 1, 2018

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

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 1, 2018
@DeltaEvo DeltaEvo changed the title test: swap arguments in strictEqual() test: swap arguments in strictEqual() for parallel/test-buffer-copy Oct 1, 2018
@@ -23,7 +23,7 @@ let cntr = 0;
b.fill(++cntr);
c.fill(++cntr);
const copied = c.copy(b, 0, 0);
assert.strictEqual(c.length, copied);
assert.strictEqual(copied, c.length);
for (let i = 0; i < c.length; i++) {
assert.strictEqual(c[i], b[i]);
Copy link
Member

Choose a reason for hiding this comment

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

I think these should be swapped as well (we copied c into b, so c holds the expected values?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@@ -34,7 +34,7 @@ let cntr = 0;
b.fill(++cntr);
c.fill(++cntr);
const copied = c.copy(b, 0);
assert.strictEqual(c.length, copied);
assert.strictEqual(copied, c.length);
for (let i = 0; i < c.length; i++) {
assert.strictEqual(c[i], b[i]);
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@targos targos added the buffer Issues and PRs related to the buffer subsystem. label Oct 1, 2018
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

Thank you!

@targos targos added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 1, 2018
@targos
Copy link
Member

targos commented Oct 1, 2018

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Thank you for doing this and welcome :)

targos pushed a commit to targos/node that referenced this pull request Oct 6, 2018
Swap arguments in strictEqual() for parallel/test-buffer-copy.

PR-URL: nodejs#23204
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos
Copy link
Member

targos commented Oct 6, 2018

Landed in 635161a 🎉. Thank you @DeltaEvo for the contribution!

@targos targos closed this Oct 6, 2018
targos pushed a commit that referenced this pull request Oct 6, 2018
Swap arguments in strictEqual() for parallel/test-buffer-copy.

PR-URL: #23204
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Oct 7, 2018
Swap arguments in strictEqual() for parallel/test-buffer-copy.

PR-URL: #23204
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Oct 17, 2018
Swap arguments in strictEqual() for parallel/test-buffer-copy.

PR-URL: #23204
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. buffer Issues and PRs related to the buffer subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants