Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Make tests more clear #8999

Closed
wants to merge 8 commits into from
+13 −12
Closed

Make tests more clear #8999

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
af9e4d0
replace ==, !=, some instances of var, and assert.equal()
Sep 25, 2016
f04c78c
change var to const
Oct 10, 2016
3d27c89
change 'var' to 'const' for clarity
Oct 10, 2016
055e479
trying to complete git pull upstream master
jennabelle Oct 10, 2016
95b1fcd
completing merge
jennabelle Oct 11, 2016
be11d1a
Change last `var` to `const`, add `assert.strictEqual()`
Oct 11, 2016
393a059
add semicolon after `assert.strictEqual()`
Oct 11, 2016
e02413c
Refactor for readability and change `var` to `const`
Oct 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions test/parallel/test-file-read-noexist.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var path = require('path');
var fs = require('fs');
const common = require('../common');
const assert = require('assert');
const path = require('path');
const fs = require('fs');

var filename = path.join(common.fixturesDir, 'does_not_exist.txt');
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Since you're in here already, can you make filename a constant too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

fs.readFile(filename, 'latin1', common.mustCall(function(err, content) {
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 we add a line after assert.ok(err) like assert.strictEqual(err.code, 'ENOENT') or whatever the right error code is? I assume it will be the same across platform, but correction welcome.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-file-write-stream2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
'use strict';
var common = require('../common');
var assert = require('assert');
const common = require('../common');
const assert = require('assert');

var path = require('path');
var fs = require('fs');
const path = require('path');
const fs = require('fs');


var filepath = path.join(common.tmpDir, 'write.txt');
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 we make this filepath a constant too?

Copy link
Member

Choose a reason for hiding this comment

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

Nit: I think the var in line 84 can also be a const, no?

Copy link
Member

Choose a reason for hiding this comment

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

Nit: While we're in here refactoring anyway, maybe line 79 can be converted from assert.ok(err.message.indexOf('write after end') >= 0); to something more like assert.ok(err.message.includes('write after end'))?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Expand Down