Ava: Add an option for strict deepEqual (order of properties important)

Created on 6 Feb 2017  路  6Comments  路  Source: avajs/ava

Description

The ECMAScript 6 specification defines in which order the properties of an object should be traversed (See: http://www.2ality.com/2015/10/property-traversal-order-es6.html).

Therefore, in ES6, the following ought to be false:

const result = {
  stderr: '',
  stdout: 'Hello, World!\n',
  exitCode: 0
};

const expectedResult = {
  exitCode: 0,
  stderr: '',
  stdout: 'Hello, World!\n'
};

t.deepEqual(result, expectedResult);

The current ([email protected]) behaviour is that deepEqual ignores the order of properties.

I suggest that deepEqual would perform strict (order aware) deepEqual by default and ask user to sort order properties of result object if property order is not important for the test.

Relevant Links

  • Related discussion on ESLint (https://github.com/eslint/eslint/issues/7714)
  • Previous discussion where this suggestion has been declined on the (false) grounds that order is not important (https://github.com/avajs/ava/issues/899)

Environment

Tell us which operating system you are using, as well as which versions of Node.js, npm, and AVA. Run the following to get it quickly:

Node.js v7.5.0
darwin 16.3.0
0.18.1
4.1.2

Most helpful comment

I agree with @novemberborn and @vadimdemedes. I'm starting to understand why the order can be important, but I think this only matters in pretty rare cases. I would prefer this to be implemented in a new package.

All 6 comments

cc @jfmengels

This is primarily about comparing the order in which string and symbol keys are defined, right? E.g. in your example you'd still expect this to pass, based on http://www.2ality.com/2015/10/property-traversal-order-es6.html#traversing_the_own_keys_of_an_object:

const result = {
  stderr: '',
  stdout: 'Hello, World!\n',
  exitCode: 0,
  [Symbol.for('foo')]: 'foo',
  [Symbol.for('bar')]: 'bar'
};

t.deepEqual(result, {
  stderr: '',
  [Symbol.for('foo')]: 'foo',
  stdout: 'Hello, World!\n',
  exitCode: 0,
  [Symbol.for('bar')]: 'bar'
})

I hear ya that order could be important, but I don't think it's important enough to make t.deepEqual super finicky. If you want to make key order a part of your API you should test for it explicitly.

I'd rather have a t.keyOrder(result, ['stderr', 'stdout', 'exitCode', Symbol.for('foo'), Symbol.for('bar')]) assertion than a t.strictDeepEqual(), but even that could be done through a third party library with the existing assertions: t.deepEqual(keyOrder(result), [鈥).

I wouldn't want t.deepEqual to check for the order of keys. As for t.keyOrder, this feels like a "nice-to-have" feature, which generally we are against from. If someone absolutely has to check for the order of keys: t.deepEqual(Object.keys(obj)).

I agree with @novemberborn and @vadimdemedes. I'm starting to understand why the order can be important, but I think this only matters in pretty rare cases. I would prefer this to be implemented in a new package.

I'd rather have a t.keyOrder(result, ['stderr', 'stdout', 'exitCode', Symbol.for('foo'), Symbol.for('bar')]) assertion than a t.strictDeepEqual()

This assumes {[key: string]: boolean | number | string | null} array result, i.e. no nesting. Assuming nesting...

but even that could be done through a third party library with the existing assertions: t.deepEqual(keyOrder(result), [鈥).

It cannot.

How are you going to deep equal two objects where result object property order is important?

but I think this only matters in pretty rare cases.

I agree that it is a pretty rare case, though. Furthermore, breaking the object into primitive value objects and comparing Object.keys values could be used as a way to check that property order is correct.

Regardless, I am raising this issue because I (a) ran into such a case and (b) because the previous issue was closed on false grounds.

As long as everyone is agreement that this feature is non-essential to the core, it is safe to close the issue.

As long as everyone is agreement that this feature is non-essential to the core, it is safe to close the issue.

There's agreement on that, yes. Thanks for bringing this up, it's an important distinction to be aware of.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ehmicky picture ehmicky  路  4Comments

fregante picture fregante  路  3Comments

nickjanssen picture nickjanssen  路  4Comments

niftylettuce picture niftylettuce  路  4Comments

leegee picture leegee  路  4Comments