Skip to content

update test/parallel/test-fs-read-stream-inherit.js to use ES6 variable declarations and assert.strictEqual #9894

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

Closed

Conversation

jmdarling
Copy link
Contributor

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

Updates test/parallel/test-fs-read-stream-inherit.js by:

  • Converts instances of var to const / let
  • Converts instances of asset.equal to assert.strictEqual

Jonathan Darling added 2 commits December 1, 2016 10:13
converts all instances of 'var' to const / let
converts instances of assert.equal to assert.strictEqual
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 1, 2016
@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Dec 1, 2016
@imyller imyller added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Dec 1, 2016
@Trott
Copy link
Member

Trott commented Dec 1, 2016

@@ -49,18 +49,18 @@ file.on('end', function(chunk) {
file.on('close', function() {
callbacks.close++;

//assert.equal(fs.readFileSync(fn), fileContent);
//assert.strictEqual(fs.readFileSync(fn), fileContent);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just remove this?


file.on('open', function(fd) {
file.length = 0;
callbacks.open++;
assert.equal('number', typeof fd);
assert.strictEqual('number', typeof fd);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: can you please swap the arguments here and throughout the rest of the file? The function signature is assert.strictEqual(actual, expected[, message]);

Jonathan Darling added 2 commits December 3, 2016 11:56
Removes an assert that has been commented out.
Fixes the parameter order to some calls of assert.strictEqual to
be consistent with documentation and the rest of the file.
console.error('ok');
});

var file4 = fs.createReadStream(rangeFile, Object.create({bufferSize: 1,
const file4 = fs.createReadStream(rangeFile, Object.create({bufferSize: 1,
start: 1, end: 2}));
Copy link
Member

Choose a reason for hiding this comment

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

Nit: would you mind to align start with rangeFile?

});

var file5 = fs.createReadStream(rangeFile, Object.create({bufferSize: 1,
const file5 = fs.createReadStream(rangeFile, Object.create({bufferSize: 1,
start: 1}));
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

});

// https://github.com/joyent/node/issues/2320
var file6 = fs.createReadStream(rangeFile, Object.create({bufferSize: 1.23,
const file6 = fs.createReadStream(rangeFile, Object.create({bufferSize: 1.23,
start: 1}));
Copy link
Member

@lpinca lpinca Dec 3, 2016

Choose a reason for hiding this comment

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

Ditto, there are also a couple more below.

Fixes spacing that become improperly alligned after changing var
declarations to const.
@lpinca
Copy link
Member

lpinca commented Dec 4, 2016

@jmdarling awesome, thank you!

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM! thank you for the PR and for participating in the code-and-learn!

jasnell pushed a commit that referenced this pull request Dec 4, 2016
* convert assert.equal to assert.strictEqual
* convert var to const/let

PR-URL: #9894
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Dec 4, 2016

Landed in e00c4bf. thank you!
fyi, I squashed the commits down into a single commit when landing

@jasnell jasnell closed this Dec 4, 2016
addaleax pushed a commit that referenced this pull request Dec 5, 2016
* convert assert.equal to assert.strictEqual
* convert var to const/let

PR-URL: #9894
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Dec 5, 2016
2 tasks
addaleax pushed a commit to addaleax/node that referenced this pull request Dec 8, 2016
* convert assert.equal to assert.strictEqual
* convert var to const/let

PR-URL: nodejs#9894
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jmdarling pushed a commit to jmdarling/node that referenced this pull request Dec 8, 2016
* convert assert.equal to assert.strictEqual
* convert var to const/let

PR-URL: nodejs#9894
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 20, 2016
* convert assert.equal to assert.strictEqual
* convert var to const/let

PR-URL: #9894
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
* convert assert.equal to assert.strictEqual
* convert var to const/let

PR-URL: #9894
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
* convert assert.equal to assert.strictEqual
* convert var to const/let

PR-URL: #9894
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This was referenced Dec 21, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. fs Issues and PRs related to the fs subsystem / file system. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants