MithrilJS / ospec

Noiseless testing framework
MIT License
48 stars 13 forks source link

deepEquals always fails on objects with arrays created with seamless-immutable #24

Closed glantucan closed 2 years ago

glantucan commented 4 years ago

deepEquals always fails on objects with arrays created with seamless-immutable, (or any other library that adds methods to the arrays) ospec version: 4.1.1

Code:

The following test fails when it shouldn't

let a = Immutable({
    tasks: {
        tasksList: [
            {
                due_date: 1595808000,
                dueMessage: 'Due today',
                daysLeft: 0,
            }
        ],
        computed: false
    }
})
, b = Immutable({
    tasks: {
        tasksList: [
            {
                due_date: 1595808000,
                dueMessage: 'Due today',
                daysLeft: 0,
            }
        ],
        computed: false
    }
})

o('test', () => {
    o(a).deepEquals(b)
})

o.run()

The problem seems to be here: https://github.com/MithrilJS/ospec/blob/master/ospec.js#L543

Ospec is using Object.getOwnPropertyNames on the array so it's finding non-enumerable functions and trying to compare them as well, and so it's finding non-enumerable functions and trying to compare them as well, rather than just ignoring them like it should.

pygy commented 2 years ago

@glantucan Not sure how we'll fix this, but in the mean time, you can write your own validator using the .satisfies() assertion.

Schematically:

const  compareImmutable = (reference) => (candidate) => {
  const discrepancies = []
  // actually compare `reference` and `candidate`, populate `discrepancies` if needed
  return {pass: discrepancies.length === 0, message: discrepancies.join("\n")}
}
o("test", () => {
  o(candidate).satisfies(compareImmutable(reference))
})

You'll probably want a recursive helper of the form

_compareImmutable(reference, candidate, discrepancies) {}
pygy commented 2 years ago

So, this is not something we can fix in core, as these objects are not deepEquals according to our definition, and out API leaves no room for adding an exception list.

Here's a rudimentary port of deepEquals to support seamless-immutable arrays (a slightly modified version of ospec's deepEquals). Feel free to embellish it to your fit needs.

Edit: better demo

pygy commented 2 years ago

Rethinking about this, I may have gotten this wrong... Non-enumerable properties are not compared on objects, we should skip them on arrays too for consistency.

pygy commented 2 years ago

This is fixed in v4.1.6

glantucan commented 2 years ago

Thanks for taking care of this :)