deepStrictEqual comparison output is unhelpful on prototype-only differences

[damianobarbati]

$ node -v
v10.8.0
$ node test.js

//test.js
const assert = require('assert').strict;

const DEBUG = process.env.DEBUG_ROUTER;

const pathnameToMatches = (pathname, template) => {
    DEBUG && console.log(pathname, '~', template);

    const regexp = templateToRegexp(template);
    const matches = pathname.match(regexp);

    DEBUG && console.log(pathname, '~', regexp, '=>', !matches ? false : (matches.groups || {}));

    if (!matches)
        return false;

    const result = matches.groups || {};
    for (const key in result)
        result[key] = result[key] || null;
    return result;
};

const templateToRegexp = template => {
    let pattern = template;
    DEBUG && console.log('processing =>', template);
    pattern = pattern.replace(/\/:(?<param>[\w\d]+?)(?=\/|$)/gi, '\\/(?<$<param>>[\\w\\d]+?)(?=\\/|$)');
    DEBUG && console.log('after required =>', pattern);
    pattern = pattern.replace(/\/:(?<param>[\w\d]+?)\?/gi, '(\\/(?<=\\/)(?<$<param>>[\\w\\d]+?)(?=\\/|$))?');
    DEBUG && console.log('after optional =>', pattern);
    pattern = '^' + pattern + '$';
    DEBUG && console.log('after boundaries =>', pattern);
    const regexp = new RegExp(pattern, 'i');
    DEBUG && console.log(template, '=>', pattern, '=>', regexp);
    return regexp;
};

try {
    assert.deepStrictEqual(pathnameToMatches('/user', '/user'), {});
    assert.deepStrictEqual(pathnameToMatches('/not-user', '/user'), false);
    assert.deepStrictEqual(pathnameToMatches('/user/123', '/user/:id'), { id: '123' });
    assert.deepStrictEqual(pathnameToMatches('/user/123/456', '/user/:id'), false);
    assert.deepStrictEqual(pathnameToMatches('/user', '/user/:id'), false);
    assert.deepStrictEqual(pathnameToMatches('/user', '/user/:id?'), { id: null });
    assert.deepStrictEqual(pathnameToMatches('/user/123', '/user/:id?'), { id: '123' });
    assert.deepStrictEqual(pathnameToMatches('/user/123/male', '/user/:id/:sex'), { id: '123', sex: 'male' });
    assert.deepStrictEqual(pathnameToMatches('/user/123', '/user/:id/:sex?'), { id: '123', sex: null });
    assert.deepStrictEqual(pathnameToMatches('/user/123', '/user/:id/:sex'), false);
    assert.deepStrictEqual(pathnameToMatches('/user/123', '/user/:id?/:sex?'), { id: '123', sex: null });
    assert.deepStrictEqual(pathnameToMatches('/user/123/female', '/user/:id?/:sex?'), { id: '123', sex: 'female' });
    assert.deepStrictEqual(pathnameToMatches('/user', '/user/:id?/:sex?'), { id: null, sex: null });

    console.log('🎉🎉🎉');
}
catch (error) {
    console.error(error);//.message, { actual: error.actual, expected: error.expected });
}

Throws a misterious:

{ AssertionError [ERR_ASSERTION]: Input objects not identical:

{
  id: '123'
}

    at Object.<anonymous> (/Users/damz/Desktop/react-router-xs/specs/pathnameToMatches.spec.js:41:12)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
    at startup (internal/bootstrap/node.js:266:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:596:3)
  generatedMessage: true,
  name: 'AssertionError [ERR_ASSERTION]',
  code: 'ERR_ASSERTION',
  actual: { id: '123' },
  expected: { id: '123' },
  operator: 'deepStrictEqual' }

What does this mean? Where's the difference? 🤨

  actual: { id: '123' },
  expected: { id: '123' },

[ChALkeR]

@damianobarbati in your test, their prototypes differ. The object that is returned from the regexp in matches.groups does not have a prototype, { id: '123' } does.

A simplified version of your testcase:

const assert = require('assert').strict;

function split(pathname) {
  const regexp = new RegExp("^/(?<id>\\d+?)$", 'i');
  const matches = pathname.match(regexp);
  return matches.groups;
}

try {
  assert.deepStrictEqual(
    split('/123', '/:id'),
    { id: '123' },
    'with prototype'
  );
} catch (e) {
  console.error(e);
}

try {
  assert.deepStrictEqual(
    split('/123', '/:id'),
    Object.assign(Object.create(null), { id: '123' }),
    'without prototype'
  );
} catch (e) {
  console.error(e);
}

Note that only the first of those two tests fails.

[ChALkeR]

That is documented: https://nodejs.org/api/assert.html#assert_comparison_details

Moreover, that is what the main difference between assert.deepStrictEqual/assert.strict.deepEqual and the legacy deprecated assert.deepEqual is.

[ChALkeR]

Replacing return result; with return Object.assign({}, result); fixes your tests.

[damianobarbati]

Oh I see now, groups has a prototype attached!

The 2 misleading factors:

• docs stating Only enumerable "own" properties are considered. but here prototype is considered for matching
• useless actual/expected messages in error

Thanks @ChALkeR!

[ChALkeR]

@damianobarbati No, matches.groups doesn't have a prototype (has null), { id: '123' } does.
assert.strict.deepEqual(Object.create(null), {}) throws because the prototypes are not equal.

[ChALkeR]

$ node
> const x = {}; x.id = '123'; x
{ id: '123' }
> const y = Object.create(null); y.id = '123'; y
{}
> assert.strict.deepEqual(x, y)
AssertionError [ERR_ASSERTION]: Input objects not identical:

{}

> Object.getPrototypeOf(x)
{}
> Object.getPrototypeOf(y)
null
> x.toString()
'[object Object]'
> y.toString()
TypeError: y.toString is not a function

matches.groups also wouldn't have a .toString() method, but { id: '123' } would, because its prototype has it.

[BridgeAR]

As far as I understand the main issue it is not about the comparison details but about the output and that is indeed not ideal. The main difficulty here is how to visualize the difference between such objects. I struggled with this as well and I would like to fix this. I just could not yet come up with a great solution so far.

I am going to look into improving that though.

[BridgeAR]

@damianobarbati

docs stating Only enumerable "own" properties are considered. but here prototype is considered for matching

The docs list all the comparison details. See https://nodejs.org/api/assert.html#assert_assert_deepstrictequal_actual_expected_message.

useless actual/expected messages in error

Those values contain the actual input values and what you see is the inspected value as a string. However, the output is only as "good" or "bad" as the inspection tool you use. util.inspect will not show visual differences between different prototypes in objects if there is nothing else different.

[ChALkeR]

@BridgeAR

As far as I understand the main issue it is not about the comparison details but about the output and that is indeed not ideal.

Ok, that makes sense. I renamed the issue then.

[antsmartian]

Hey @BridgeAR @ChALkeR , thought of giving a crack at this. I was seeing through the source code, and I found the issue is we are not showing up the error message properly when two objects are compared and one of those object's prototype is null.

The fix should be straight forward , if we have a object whose prototype is null, we need to build up the braces object so that it can catch up the data about prototype being null(so that we can use these data to print relevant information on failure). But definitely we need to come up with some text that says it has empty prototype (I had kept it as proto-null while I was building locally). I did those changes and I ran the example given by the OP and got the below message:

{ AssertionError [ERR_ASSERTION]: Expected inputs to be strictly deep-equal:
+ actual - expected

+ proto-null {
- {
    id: '123'
  }
    at Object.<anonymous> (/Users/anto/programs/node/node-hack/22141.js:83:12)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
    at startup (internal/bootstrap/node.js:266:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:596:3)
  generatedMessage: true,
  name: 'AssertionError [ERR_ASSERTION]',
  code: 'ERR_ASSERTION',
  actual: proto-null { id: '123' },
  expected: { id: '123' },
  operator: 'deepStrictEqual' }

The actual,expected looked:

  actual: proto-null { id: '123' },
  expected: { id: '123' },

which seems good to me -- clearly indicates prototype is not equal. Well, of course, if we do these changes, we need to work on utils.inspect test case as well (on the text that we might add), ex : https://github.com/nodejs/node/blob/master/test/parallel/test-util-inspect.js#L256

Let me know your thoughts on this. Thanks!

[BridgeAR]

@antsmartian I think it is actually a good idea to change the output in util.inspect() to visualize the null prototype. My first thought was to only fix this in assert but it should be useful in general to know if the entry has a prototype or not.

If the inspect output provides the necessary information the output is automatically fixed for assert. The question is how it should be visualized. proto-null seems sub-par. [Object: null prototype] would work. The same as e.g., [Promise: null prototype].

[antsmartian]

@BridgeAR

If the inspect output provides the necessary information the output is automatically fixed for assert.

Yes, as I have shown above.

The question is how it should be visualized. proto-null seems sub-par.[Object: null prototype] would work. The same as e.g., [Promise: null prototype].

Yes, using [Object: null prototype] should be good enough as we will get the following assert error:

actual: [Object: null prototype] { id: '123' },
expected: { id: '123' },

I will work on this and raise a PR.

Note: You have actually mentioned the wrong person in your reply , instead of me ;)