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.

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

test, eslintrc: s/assert.equal/assert.strictEqual/ #10698

Merged
merged 2 commits into from
Jan 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
28 changes: 16 additions & 12 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,21 @@ rules:
no-new-require: 2
no-path-concat: 2
no-restricted-modules: [2, sys, _linklist]
no-restricted-properties: [2, {
object: assert,
property: deepEqual,
message: Please use assert.deepStrictEqual().
}, {
property: __defineGetter__,
message: __defineGetter__ is deprecated.
}, {
property: __defineSetter__,
message: __defineSetter__ is deprecated.
}]
no-restricted-properties:
- 2
- object: assert
property: deepEqual
message: Use assert.deepStrictEqual().
- object: assert
property: equal
message: Use assert.strictEqual() rather than assert.equal().
- object: assert
property: notEqual
message: Use assert.notStrictEqual() rather than assert.notEqual().
- property: __defineGetter__
message: __defineGetter__ is deprecated.
- property: __defineSetter__,
message: __defineSetter__ is deprecated.

# Stylistic Issues
# http://eslint.org/docs/rules/#stylistic-issues
Expand All @@ -86,7 +90,7 @@ rules:
func-name-matching: 2
indent: [2, 2, {ArrayExpression: first,
CallExpression: {arguments: first},
MemberExpression: 1,
MemberExpression: 1,
ObjectExpression: first,
SwitchCase: 1}]
key-spacing: [2, {mode: minimum}]
Expand Down
3 changes: 1 addition & 2 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ assert.ok = ok;
// The equality assertion tests shallow, coercive equality with
// ==.
// assert.equal(actual, expected, message_opt);

/* eslint-disable no-restricted-properties */
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Maybe // eslint-disable-next-line no-restricted-properties would be better here so that the rule isn't disabled for the rest of the file. (I'm assuming it's intended to only be disabled for this line, rather than for the rest of the file.)

Copy link
Member Author

@gibfahn gibfahn Jan 10, 2017

Choose a reason for hiding this comment

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

This line was already in use for deepEqual and notDeepEqual below, it's now being used for 4 functions (it's re-enabled below in L136). I think we'd need 8 eslint-disable-next-line lines if we did it individually.

So I left it as it was, let me know if you disagree.

Copy link
Member

Choose a reason for hiding this comment

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

I'd slightly prefer 8 eslint-disable-next-line calls. The more targeted and explicit the disabling, the better. But I don't oppose doing it the way you did it either.

assert.equal = function equal(actual, expected, message) {
if (actual != expected) fail(actual, expected, message, '==', assert.equal);
};
Expand All @@ -127,7 +127,6 @@ assert.notEqual = function notEqual(actual, expected, message) {
// The equivalence assertion tests a deep equality relation.
// assert.deepEqual(actual, expected, message_opt);

/* eslint-disable no-restricted-properties */
assert.deepEqual = function deepEqual(actual, expected, message) {
if (!_deepEqual(actual, expected, false)) {
fail(actual, expected, message, 'deepEqual', assert.deepEqual);
Expand Down
8 changes: 4 additions & 4 deletions test/disabled/test-fs-largefile.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,22 @@ const path = require('path'),
message = 'Large File';

fs.truncateSync(fd, offset);
assert.equal(fs.statSync(filepath).size, offset);
assert.strictEqual(fs.statSync(filepath).size, offset);
var writeBuf = Buffer.from(message);
fs.writeSync(fd, writeBuf, 0, writeBuf.length, offset);
var readBuf = Buffer.allocUnsafe(writeBuf.length);
fs.readSync(fd, readBuf, 0, readBuf.length, offset);
assert.equal(readBuf.toString(), message);
assert.strictEqual(readBuf.toString(), message);
fs.readSync(fd, readBuf, 0, 1, 0);
assert.equal(readBuf[0], 0);
assert.strictEqual(readBuf[0], 0);

var exceptionRaised = false;
try {
fs.writeSync(fd, writeBuf, 0, writeBuf.length, 42.000001);
} catch (err) {
console.log(err);
exceptionRaised = true;
assert.equal(err.message, 'Not an integer');
assert.strictEqual(err.message, 'Not an integer');
}
assert.ok(exceptionRaised);
fs.close(fd);
Expand Down
2 changes: 1 addition & 1 deletion test/disabled/test-http-abort-stream-end.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ s.listen(common.PORT, function() {

process.on('exit', function() {
assert(aborted);
assert.equal(size, maxSize);
assert.strictEqual(size, maxSize);
console.log('ok');
});
14 changes: 7 additions & 7 deletions test/disabled/test-readline.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ var rl = readlineFakeStream();
var written_bytes_length, refreshed;

rl.write('foo');
assert.equal(3, rl.cursor);
assert.strictEqual(3, rl.cursor);
[key.xterm, key.rxvt, key.gnome, key.putty].forEach(function(key) {
rl.write.apply(rl, key.home);
assert.equal(0, rl.cursor);
assert.strictEqual(0, rl.cursor);
rl.write.apply(rl, key.end);
assert.equal(3, rl.cursor);
assert.strictEqual(3, rl.cursor);
});

rl = readlineFakeStream();
Expand All @@ -76,9 +76,9 @@ rl.write.apply(rl, key.xterm.home);
].forEach(function(action) {
written_bytes_length = rl.written_bytes.length;
rl.write.apply(rl, action.key);
assert.equal(action.cursor, rl.cursor);
assert.strictEqual(action.cursor, rl.cursor);
refreshed = written_bytes_length !== rl.written_bytes.length;
assert.equal(true, refreshed);
assert.strictEqual(true, refreshed);
});

rl = readlineFakeStream();
Expand All @@ -93,7 +93,7 @@ rl.write.apply(rl, key.xterm.home);
''
].forEach(function(expectedLine) {
rl.write.apply(rl, key.xterm.metad);
assert.equal(0, rl.cursor);
assert.equal(expectedLine, rl.line);
assert.strictEqual(0, rl.cursor);
assert.strictEqual(expectedLine, rl.line);
});
rl.close();
10 changes: 5 additions & 5 deletions test/disabled/test-sendfd.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ var logChild = function(d) {
// validate any data sent back by the child. We send the write end of the
// pipe to the child and close it off in our process.
var pipeFDs = netBinding.pipe();
assert.equal(pipeFDs.length, 2);
assert.strictEqual(pipeFDs.length, 2);

var seenOrdinals = [];

Expand All @@ -72,8 +72,8 @@ pipeReadStream.on('data', function(data) {
data.toString('utf8').trim().split('\n').forEach(function(d) {
var rd = JSON.parse(d);

assert.equal(rd.pid, cpp);
assert.equal(seenOrdinals.indexOf(rd.ord), -1);
assert.strictEqual(rd.pid, cpp);
assert.strictEqual(seenOrdinals.indexOf(rd.ord), -1);

seenOrdinals.unshift(rd.ord);
});
Expand Down Expand Up @@ -119,8 +119,8 @@ cp.on('exit', function(code, signal) {
srv.close();
// fs.unlinkSync(SOCK_PATH);

assert.equal(code, 0);
assert.equal(seenOrdinals.length, 2);
assert.strictEqual(code, 0);
assert.strictEqual(seenOrdinals.length, 2);
});

// vim:ts=2 sw=2 et
2 changes: 1 addition & 1 deletion test/disabled/test-setuidgid.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ assert.notStrictEqual(newuid, olduid, 'uids expected to be different');
try {
process.setuid('nobody1234');
} catch (e) {
assert.equal(e.message,
assert.strictEqual(e.message,
'failed to resolve group',
'unexpected error message'
);
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/GH-892-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ var options = {
};

var req = https.request(options, function(res) {
assert.equal(200, res.statusCode);
assert.strictEqual(200, res.statusCode);
gotResponse = true;
console.error('DONE');
res.resume();
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/b/c.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const assert = require('assert');

const package = require('./package');

assert.equal('world', package.hello);
assert.strictEqual('world', package.hello);

console.error('load fixtures/b/c.js');

Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/node_modules/baz/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/fixtures/node_modules/foo.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions test/internet/test-dns-txt-sigsegv.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ const assert = require('assert');
const dns = require('dns');

dns.resolveTxt('www.microsoft.com', function(err, records) {
assert.equal(err, null);
assert.equal(records.length, 0);
assert.strictEqual(err, null);
assert.strictEqual(records.length, 0);
});
2 changes: 1 addition & 1 deletion test/message/error_exit.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ process.on('exit', function(code) {
console.error('Exiting with code=%d', code);
});

assert.equal(1, 2);
assert.strictEqual(1, 2);
2 changes: 1 addition & 1 deletion test/message/error_exit.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Exiting with code=1
assert.js:*
throw new assert.AssertionError({
^
AssertionError: 1 == 2
AssertionError: 1 === 2
at Object.<anonymous> (*test*message*error_exit.js:*:*)
at Module._compile (module.js:*:*)
at Object.Module._extensions..js (module.js:*:*)
Expand Down
38 changes: 19 additions & 19 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,9 @@ try {
threw = true;
assert.ok(e instanceof TypeError, 'type');
}
assert.equal(true, threw,
'a.throws with an explicit error is eating extra errors',
a.AssertionError);
assert.strictEqual(true, threw,
'a.throws with an explicit error is eating extra errors',
a.AssertionError);
threw = false;

// doesNotThrow should pass through all errors
Expand All @@ -345,8 +345,8 @@ try {
threw = true;
assert.ok(e instanceof TypeError);
}
assert.equal(true, threw,
'a.doesNotThrow with an explicit error is eating extra errors');
assert.strictEqual(true, threw, 'a.doesNotThrow with an explicit error is ' +
'eating extra errors');

// key difference is that throwing our correct error makes an assertion error
try {
Expand All @@ -355,8 +355,8 @@ try {
threw = true;
assert.ok(e instanceof a.AssertionError);
}
assert.equal(true, threw,
'a.doesNotThrow is not catching type matching errors');
assert.strictEqual(true, threw,
'a.doesNotThrow is not catching type matching errors');

assert.throws(function() { assert.ifError(new Error('test error')); });
assert.doesNotThrow(function() { assert.ifError(null); });
Expand Down Expand Up @@ -461,10 +461,10 @@ circular.x = circular;

function testAssertionMessage(actual, expected) {
try {
assert.equal(actual, '');
assert.strictEqual(actual, '');
} catch (e) {
assert.equal(e.toString(),
['AssertionError:', expected, '==', '\'\''].join(' '));
assert.strictEqual(e.toString(),
['AssertionError:', expected, '===', '\'\''].join(' '));
assert.ok(e.generatedMessage, 'Message not marked as generated');
}
}
Expand Down Expand Up @@ -499,24 +499,24 @@ try {
});
} catch (e) {
threw = true;
assert.equal(e.message, 'Missing expected exception..');
assert.strictEqual(e.message, 'Missing expected exception..');
}
assert.ok(threw);

// #5292
try {
assert.equal(1, 2);
assert.strictEqual(1, 2);
} catch (e) {
assert.equal(e.toString().split('\n')[0], 'AssertionError: 1 == 2');
assert.strictEqual(e.toString().split('\n')[0], 'AssertionError: 1 === 2');
assert.ok(e.generatedMessage, 'Message not marked as generated');
}

try {
assert.equal(1, 2, 'oh no');
assert.strictEqual(1, 2, 'oh no');
} catch (e) {
assert.equal(e.toString().split('\n')[0], 'AssertionError: oh no');
assert.equal(e.generatedMessage, false,
'Message incorrectly marked as generated');
assert.strictEqual(e.toString().split('\n')[0], 'AssertionError: oh no');
assert.strictEqual(e.generatedMessage, false,
'Message incorrectly marked as generated');
}

// Verify that throws() and doesNotThrow() throw on non-function block
Expand All @@ -527,8 +527,8 @@ function testBlockTypeError(method, block) {
method(block);
threw = false;
} catch (e) {
assert.equal(e.toString(),
'TypeError: "block" argument must be a function');
assert.strictEqual(e.toString(),
'TypeError: "block" argument must be a function');
}

assert.ok(threw);
Expand Down
Loading