assert: introduce `deepStrictEqual` #639

vkurchatkin · 2015-01-28T17:09:17Z

deepStrictEqual works the same way as strictEqual, but uses === to compare primitives and requires prototypes of equal objects to be the same object.

Fixes: nodejs/node-v0.x-archive#7161

R=@iojs/collaborators

Fishrock123 · 2015-01-28T17:56:33Z

There was a large discussion in IRC a couple weeks ago on why this didn't make semantic sense.

Basically strict and deep are mutually exclusive; having them both together means having a deepStrictlyQucksLike() with duck-typing for only some things.

While such a structure can still be useful, I'm not sure if it is actually good.

See @othiym23's deeper and deepest.

mscdex · 2015-01-28T18:05:46Z

What about primitives? Think about if you're trying to do a assert.deepEqual() on two objects, where one object has foo: 5.2 and the other has foo: '5.2'. assert.deepEqual() would consider those two objects equal, whereas a assert.deepStrictEqual() would not.

vkurchatkin · 2015-01-28T19:11:11Z

Basically strict and deep are mutually exclusive

Why? I don't get it.

Fishrock123 · 2015-01-28T22:15:31Z

See the conversation following: http://logs.libuv.org/io.js/2014-12-15#19:43:36.553

I'll re-read the conversation again tonight as I would like this kind of feature.

vkurchatkin · 2015-01-28T22:30:55Z

what I want is feedback from people. I personally expect asserts to be as strict as possible in tests

Fishrock123 · 2015-02-04T19:53:23Z

lib/assert.js

  // if one is a primitive, the other must be same
  if (util.isPrimitive(a) || util.isPrimitive(b))
    return a === b;
+  if (strict && Object.getPrototypeOf(a) !== Object.getPrototypeOf(b))
+    return false;


This is still going to suffer from the things discussed in #621

Are we sure this shouldn't just check primitives? Otherwise we might as well just do a regular strictEquals().

not sure what you mean. recursively call deepStrictEqual on prototypes?

Looked at the tests and it seems as I would expect. I think I mis-understood points made in the previous thread.

Fishrock123 · 2015-02-04T20:22:35Z

LGTM

vkurchatkin · 2015-02-04T20:52:33Z

Ok, so the PR has been approved by TC. I'm not sure it's ok to merge it now since we don't have a branch for next minor release and I don't know if want to do 1.2.0 right after 1.1.0 /cc @chrisdickinson

vkurchatkin · 2015-02-06T12:47:29Z

rebased the PR. can anyone take a final look? @bnoordhuis maybe

tellnes · 2015-02-06T15:16:10Z

test/parallel/test-assert.js

+  toString: function() { return this.first + ' ' + this.last; }
+};
+
+function nameBuilder(first, last) {


Shouldn't constructor name start with an uppercase letter?

to be honest, I just copied all the test for deepEqual and changed doesNotThrow to throws and vice versa where appropriate. This test doesn't make much sense, but I decided not to remove anything.

tellnes · 2015-02-06T15:19:58Z

Except for the naming thing in the test, it LGTM.

vkurchatkin · 2015-02-06T15:36:05Z

tweaked tests to make a bit more sense and removed copy-paste artifacts

bnoordhuis · 2015-02-06T16:17:44Z

doc/api/assert.markdown

@@ -23,11 +23,12 @@ Tests shallow, coercive non-equality with the not equal comparison operator ( `!

 ## assert.deepEqual(actual, expected[, message])

-Tests for deep equality.
+Tests for deep equality. Primitive values are compared with the equal comparison operator ( `==` ).


Can you make lines wrap at 80 columns?

bnoordhuis · 2015-02-06T16:28:43Z

test/parallel/test-assert.js

+assert.throws(makeBlock(a.deepStrictEqual, new String('a'), ['a']), a.AssertionError);
+assert.throws(makeBlock(a.deepStrictEqual, new String('a'), {0: 'a'}), a.AssertionError);
+assert.throws(makeBlock(a.deepStrictEqual, new Number(1), {}), a.AssertionError);
+assert.throws(makeBlock(a.deepStrictEqual, new Boolean(true), {}), a.AssertionError);


Can you fix the long lines in this file?

`deepStrictEqual` works the same way as `strictEqual`, but uses `===` to compare primitives and requires prototypes of equal objects to be the same object. Fixes: nodejs/node-v0.x-archive#7161

vkurchatkin · 2015-02-07T14:57:03Z

@bnoordhuis Wrapped some lines, reworked weird test to just compare arrays and added some tests for primitives. PTAL

bnoordhuis · 2015-02-07T15:49:29Z

LGTM. CI: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/156/

rvagg · 2015-02-09T10:58:34Z

just land it @vkurchatkin, it'll hit 1.2.0 and I think we have reached a point of acceptance on this (re TC meeting specifically).

lgtm from me

`deepStrictEqual` works the same way as `strictEqual`, but uses `===` to compare primitives and requires prototypes of equal objects to be the same object. Fixes: nodejs/node-v0.x-archive#7161 Fixes: #620 PR-URL: #639 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-by: Rod Vagg <[email protected]>

vkurchatkin · 2015-02-09T11:18:10Z

landed in 3f473ef

@sonewman

Notable changes: * stream: - Simpler stream construction, see nodejs/readable-stream#102 for details. This extends the streams base objects to make their constructors accept default implementation methods, reducing the boilerplate required to implement custom streams. An updated version of readable-stream will eventually be released to match this change in core. (@sonewman) * dns: - `lookup()` now supports an `'all'` boolean option, default to `false` but when turned on will cause the method to return an array of *all* resolved names for an address, see, #744 (@silverwind) * assert: - Remove `prototype` property comparison in `deepEqual()`, considered a bugfix, see #636 (@vkurchatkin) - Introduce a `deepStrictEqual()` method to mirror `deepEqual()` but performs strict equality checks on primitives, see #639 (@vkurchatkin) * **tracing**: - Add LTTng (Linux Trace Toolkit Next Generation) when compiled with the `--with-lttng` option. Trace points match those available for DTrace and ETW. #702 (@thekemkid) * npm upgrade to 2.5.1 * **libuv** upgrade to 1.4.0 * Add new collaborators: - Aleksey Smolenchuk (@lxe) - Shigeki Ohtsu (@shigeki)

vkurchatkin force-pushed the assert-strict-deep branch from 3b56e2d to 6a73421 Compare January 28, 2015 17:09

vkurchatkin changed the title ~~assert: introduce deepStictEqual~~ assert: introduce deepStrictEqual Jan 28, 2015

Fishrock123 added the semver-minor PRs that contain new features and should be released in the next minor version. label Jan 28, 2015

vkurchatkin added the assert Issues and PRs related to the assert subsystem. label Jan 28, 2015

vkurchatkin mentioned this pull request Feb 1, 2015

assert: don't compare object prototype property #636

Closed

vkurchatkin added the tc-agenda label Feb 3, 2015

vkurchatkin mentioned this pull request Feb 3, 2015

io.js TC Meeting 2015-02-04 #701

Closed

Fishrock123 reviewed Feb 4, 2015
View reviewed changes

vkurchatkin removed the tc-agenda label Feb 4, 2015

vkurchatkin force-pushed the assert-strict-deep branch from 6a73421 to 147acd6 Compare February 6, 2015 12:42

tellnes reviewed Feb 6, 2015
View reviewed changes

vkurchatkin force-pushed the assert-strict-deep branch from 147acd6 to a45a104 Compare February 6, 2015 15:34

bnoordhuis reviewed Feb 6, 2015
View reviewed changes

vkurchatkin force-pushed the assert-strict-deep branch from a45a104 to c91cec8 Compare February 6, 2015 16:28

bnoordhuis reviewed Feb 6, 2015
View reviewed changes

assert: introduce deepStrictEqual

fc2bbe5

`deepStrictEqual` works the same way as `strictEqual`, but uses `===` to compare primitives and requires prototypes of equal objects to be the same object. Fixes: nodejs/node-v0.x-archive#7161

vkurchatkin force-pushed the assert-strict-deep branch from c91cec8 to fc2bbe5 Compare February 7, 2015 14:55

vkurchatkin mentioned this pull request Feb 9, 2015

Release proposal: v1.2.0 #768

Closed

vkurchatkin closed this Feb 9, 2015

Fishrock123 added this to the 1.2.0 milestone Feb 9, 2015

bnb mentioned this pull request Feb 12, 2015

Evangelism: Weekly Update Feb 13th nodejs/iojs.org#122

Closed

takashi mentioned this pull request Feb 14, 2015

io.js weekly 2015-02-13 nodejs/nodejs-ja#26

Merged

adoyle-h mentioned this pull request Feb 14, 2015

2015.2.13 周报翻译 nodejs/nodejs-zh-CN#87

Merged

Schmoopiie mentioned this pull request Feb 15, 2015

Traduction de IO.js week of February 13th 2015 nodejs/nodejs-fr#24

Closed

bnoordhuis mentioned this pull request Apr 7, 2015

assert.deepEqual treats RegExp literals equal to {} #1362

Closed

twada mentioned this pull request May 5, 2015

deepEqual does not descriminate by type mochajs/mocha#1685

Closed

nunofgs mentioned this pull request Aug 20, 2015

Fix empty objects not being declared nemtsov/json-mask#9

Merged

kellyselden mentioned this pull request Feb 14, 2016

Copy deepStrictEqual and notDeepStrictEqual from node. qunitjs/qunit#930

Closed

ChALkeR mentioned this pull request May 2, 2016

API stability level «Locked» is inaccurate #6528

Closed

ChALkeR mentioned this pull request Feb 8, 2017

Changing Definition of "Locked" for Stability Index in API Docs #11200

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

assert: introduce `deepStrictEqual` #639

assert: introduce `deepStrictEqual` #639

vkurchatkin commented Jan 28, 2015

Fishrock123 commented Jan 28, 2015

mscdex commented Jan 28, 2015

vkurchatkin commented Jan 28, 2015

Fishrock123 commented Jan 28, 2015

vkurchatkin commented Jan 28, 2015

Fishrock123 Feb 4, 2015

vkurchatkin Feb 4, 2015

Fishrock123 Feb 4, 2015

Fishrock123 commented Feb 4, 2015

vkurchatkin commented Feb 4, 2015

vkurchatkin commented Feb 6, 2015

tellnes Feb 6, 2015

vkurchatkin Feb 6, 2015

tellnes commented Feb 6, 2015

vkurchatkin commented Feb 6, 2015

bnoordhuis Feb 6, 2015

bnoordhuis Feb 6, 2015

vkurchatkin commented Feb 7, 2015

bnoordhuis commented Feb 7, 2015

rvagg commented Feb 9, 2015

vkurchatkin commented Feb 9, 2015

assert: introduce deepStrictEqual #639

assert: introduce deepStrictEqual #639

Conversation

vkurchatkin commented Jan 28, 2015

Fishrock123 commented Jan 28, 2015

mscdex commented Jan 28, 2015

vkurchatkin commented Jan 28, 2015

Fishrock123 commented Jan 28, 2015

vkurchatkin commented Jan 28, 2015

Fishrock123 Feb 4, 2015

Choose a reason for hiding this comment

vkurchatkin Feb 4, 2015

Choose a reason for hiding this comment

Fishrock123 Feb 4, 2015

Choose a reason for hiding this comment

Fishrock123 commented Feb 4, 2015

vkurchatkin commented Feb 4, 2015

vkurchatkin commented Feb 6, 2015

tellnes Feb 6, 2015

Choose a reason for hiding this comment

vkurchatkin Feb 6, 2015

Choose a reason for hiding this comment

tellnes commented Feb 6, 2015

vkurchatkin commented Feb 6, 2015

bnoordhuis Feb 6, 2015

Choose a reason for hiding this comment

bnoordhuis Feb 6, 2015

Choose a reason for hiding this comment

vkurchatkin commented Feb 7, 2015

bnoordhuis commented Feb 7, 2015

rvagg commented Feb 9, 2015

vkurchatkin commented Feb 9, 2015

assert: introduce `deepStrictEqual` #639

assert: introduce `deepStrictEqual` #639