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

deepEqual doesn't compare the constructor (class types) #482

Closed
Istador opened this issue Sep 4, 2019 · 3 comments
Closed

deepEqual doesn't compare the constructor (class types) #482

Istador opened this issue Sep 4, 2019 · 3 comments

Comments

@Istador
Copy link

Istador commented Sep 4, 2019

When comparing objects created with new, I'd expect deepEqual and notDeepEqual to compare the class type / prototype to be equal or not (the constructor property of the objects).

I'm not sure if this issue belongs here or into deep-equal. I can also think of this being working as intended. Then there should be other methods, arguments or options added to tape to support this use-case.

lodash's isEqual seems to do this correctly.


Test Code:

const test = require('tape')
const deepEqual = require('deep-equal')
const isEqual = require('lodash.isequal')

class A {
  constructor(data) { this.data = data }
}
class B {
  constructor(data) { this.data = data }
}

const a1 = new A('c')
const a2 = new A('c')
const a3 = new A('d')
const b1 = new B('c')

const aa1 = new A(a1)
const aa2 = new A(a2)
const aa3 = new A(a3)
const ab1 = new A(b1)

test('tape deepEqual', t => {
  t.deepEqual(    a1,  a1,  'the same class should be equal')
  t.notDeepEqual( a1,  b1,  'different classes should not be equal')
  t.notDeepEqual( a1,  a3,  'different instance variables should not be equal')
  // deeper
  t.deepEqual(    aa1, aa2, 'the same class should be equal inside a deeper structure')
  t.notDeepEqual( aa1, ab1, 'different classes should not be equal inside a deeper structure')
  t.notDeepEqual( aa1, aa3, 'different instance variables should not be equal inside a deeper structure')
  t.end()
})

test('deep-equal', t => {
  t.ok(   deepEqual(a1, a2),   'the same class should be equal')
  t.notOk(deepEqual(a1, b1),   'different classes should not be equal')
  t.notOk(deepEqual(a1, a3),   'different instance variables should not be equal')
  // deeper
  t.ok(   deepEqual(aa1, aa2), 'the same class should be equal inside a deeper structure')
  t.notOk(deepEqual(aa1, ab1), 'different classes should not be equal inside a deeper structure')
  t.notOk(deepEqual(aa1, aa3), 'different instance variables should not be equal inside a deeper structure')
  t.end()
})

test('lodash.isequal', t => {
  t.ok(   isEqual(a1, a2),   'the same class should be equal')
  t.notOk(isEqual(a1, b1),   'different classes should not be equal')
  t.notOk(isEqual(a1, a3),   'different instance variables should not be equal')
  // deeper
  t.ok(   isEqual(aa1, aa2), 'the same class should be equal inside a deeper structure')
  t.notOk(isEqual(aa1, ab1), 'different classes should not be equal inside a deeper structure')
  t.notOk(isEqual(aa1, aa3), 'different instance variables should not be equal inside a deeper structure')
  t.end()
})

Output:

$ node test_tap.js
TAP version 13
# tape deepEqual
ok 1 the same class should be equal
not ok 2 different classes should not be equal
  ---
    operator: notDeepEqual
    expected: |-
      { data: 'c' }
    actual: |-
      { data: 'c' }
    at: Test.<anonymous> ([..]:24:5)
    [...]
  ...
ok 3 different instance variables should not be equal
ok 4 the same class should be equal inside a deeper structure
not ok 5 different classes should not be equal inside a deeper structure
  ---
    operator: notDeepEqual
    expected: |-
      { data: { data: 'c' } }
    actual: |-
      { data: { data: 'c' } }
    at: Test.<anonymous> ([..]:28:5)
    [...]
  ...
ok 6 different instance variables should not be equal inside a deeper structure
# deep-equal
ok 7 the same class should be equal
not ok 8 different classes should not be equal
  ---
    operator: notOk
    expected: false
    actual:   true
    at: Test.<anonymous> ([..]:35:5)
    [...]
  ...
ok 9 different instance variables should not be equal
ok 10 the same class should be equal inside a deeper structure
not ok 11 different classes should not be equal inside a deeper structure
  ---
    operator: notOk
    expected: false
    actual:   true
    at: Test.<anonymous> ([..]:39:5)
    [...]
  ...
ok 12 different instance variables should not be equal inside a deeper structure
# lodash.isequal
ok 13 the same class should be equal
ok 14 different classes should not be equal
ok 15 different instance variables should not be equal
ok 16 the same class should be equal inside a deeper structure
ok 17 different classes should not be equal inside a deeper structure
ok 18 different instance variables should not be equal inside a deeper structure

1..18
# tests 18
# pass  14
# fail  4
@ljharb
Copy link
Collaborator

ljharb commented Sep 4, 2019

deep-equal v1 matches the way node’s assert.deepEqual worked in the past; the upcoming v2, which tape will update to use in a semver-major, matches how it works now.

@ljharb
Copy link
Collaborator

ljharb commented Dec 23, 2019

This should be resolved in the v5.0.0-next.0 prerelease of tape.

@ljharb ljharb closed this as completed Dec 23, 2019
@Istador
Copy link
Author

Istador commented Dec 23, 2019

Yep, it is. Thank you.

$ npm install tape@5.0.0-next.0
+ tape@5.0.0-next.0
added 14 packages from 2 contributors, updated 3 packages and audited 242 packages in 3.232s
found 0 vulnerabilities

$ node test_tap.js
TAP version 13
# tape deepEqual
ok 1 the same class should be equal
ok 2 different classes should not be equal
ok 3 different instance variables should not be equal
ok 4 the same class should be equal inside a deeper structure
ok 5 different classes should not be equal inside a deeper structure
ok 6 different instance variables should not be equal inside a deeper structure
[...]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants