Skip to content

Commit

Permalink
fix: data comparison with undefined object values (#2049)
Browse files Browse the repository at this point in the history
Updates the `dataEqual` utility and underlying `deepEqual` recursive functions to treat explicit and implicit `undefined` object values as equal.
Also ensures that `expand` is only called on top-level data if they're plain objects, thus avoiding accidentally equating an object and an array if the object's keys happen to be integers.
  • Loading branch information
mastermatt committed Jul 27, 2020
1 parent a0a7fdf commit ac6ebbb
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 9 deletions.
28 changes: 19 additions & 9 deletions lib/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,15 @@ function urlToOptions(url) {
* - The expected data can use regexp to compare values
* - JSON path notation and nested objects are considered equal
*/
const dataEqual = (expected, actual) =>
deepEqual(expand(expected), expand(actual))
const dataEqual = (expected, actual) => {
if (isPlainObject(expected)) {
expected = expand(expected)
}
if (isPlainObject(actual)) {
actual = expand(actual)
}
return deepEqual(expected, actual)
}

/**
* Converts flat objects whose keys use JSON path notation to nested objects.
Expand All @@ -573,17 +580,20 @@ function deepEqual(expected, actual) {
return expected.test(actual)
}

if (Array.isArray(expected) || isPlainObject(expected)) {
if (actual === undefined) {
if (Array.isArray(expected) && Array.isArray(actual)) {
if (expected.length !== actual.length) {
return false
}

const expKeys = Object.keys(expected)
if (expKeys.length !== Object.keys(actual).length) {
return false
}
return expected.every((expVal, idx) => deepEqual(expVal, actual[idx]))
}

if (isPlainObject(expected) && isPlainObject(actual)) {
const allKeys = Array.from(
new Set(Object.keys(expected).concat(Object.keys(actual)))
)

return expKeys.every(key => deepEqual(expected[key], actual[key]))
return allKeys.every(key => deepEqual(expected[key], actual[key]))
}

return expected === actual
Expand Down
22 changes: 22 additions & 0 deletions tests/test_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,28 @@ describe('`normalizeClientRequestArgs()`', () => {
})
})

describe('`dataEqual()`', () => {
it('treats explicit and implicit undefined object values as equal', () => {
const result = common.dataEqual({ a: 'a', b: undefined }, { a: 'a' })
expect(result).to.equal(true)
})
it('does not conflate object and array keys', () => {
const result = common.dataEqual(['a', 'b'], { '0': 'a', '1': 'b' })
expect(result).to.equal(false)
})
it('treats JSON path notated and nested objects as equal', () => {
const result = common.dataEqual(
{ 'foo[bar][0]': 'baz' },
{ foo: { bar: ['baz'] } }
)
expect(result).to.equal(true)
})
it('does not equate arrays of different length', () => {
const result = common.dataEqual(['a'], ['a', 'b'])
expect(result).to.equal(false)
})
})

it('testing timers are deleted correctly', done => {
const timeoutSpy = sinon.spy()
const intervalSpy = sinon.spy()
Expand Down

0 comments on commit ac6ebbb

Please sign in to comment.