-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Correct highlighting for asymmetric matchers #7893
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
Changes from all commits
59937bc
e26d9cd
fb70d79
ea15b24
91c8b7c
9308f69
5b1daef
391fbfa
16d7243
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
**/__mocks__/** | ||
**/__tests__/** | ||
src | ||
tsconfig.json | ||
tsconfig.tsbuildinfo |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
# Jest New Diff | ||
|
||
I have not thought about the name at all. If there are good suggestions, you are more than welcome. | ||
|
||
## Motivation | ||
|
||
`jest-diff` package works by serializing the values and then diffing the serializations. This approach works for most of the scenarios but has a few edge cases. New Diff tries to address these limitations by first by diffing the values and then serializing the differences. | ||
|
||
## Understanding design and codebase | ||
|
||
Note: Consider this as my attempt to write something that works to improve my understanding of the problem space. | ||
|
||
API is almost identical to `jest-diff`. There are two extra fields added to the options but the rest is the same. For now, not options are partially implemented. | ||
|
||
There are two major components in the current implementation. | ||
|
||
1. `diff` function which returns DiffObject(Representation of difference between two values as a JS object) | ||
2. `format` function which returns a formatted string based on DiffObject | ||
|
||
There is support for plugins. I have made ReactElement and AsymmetricMatcher Plugin, but they are not fully functional. | ||
|
||
## `diff` | ||
|
||
### supported types: | ||
|
||
- [x] primitives (String is the only primitve with childDiffs) | ||
- [x] Date | ||
- [x] RegExp | ||
- [x] Function | ||
- [x] PrimitiveWrapper | ||
- [x] Array | ||
- [x] Object | ||
- [x] Circular | ||
- [x] Map (Does not checks equality of complex value keys) | ||
- [ ] Set | ||
- [ ] DOM Node | ||
- [ ] React | ||
- [x] Asymmetric Any | ||
- [x] Asymmetric Object | ||
|
||
I am quite happy with this module. It's clear to me what it does. It recursively marks values as Inserted, Updated, Deleted, Equal or TypeUnequal and returns an object which represents the differences between 2 values. | ||
|
||
Note: Diff will also traverse and mark all the children of complex values | ||
|
||
## `format` | ||
|
||
- [x] primitives | ||
- [x] Function | ||
- [x] Array | ||
- [x] Object | ||
- [x] Circular | ||
- [ ] Date | ||
- [ ] RegExp | ||
- [x] Map | ||
- [ ] Set | ||
- [ ] DOM Node | ||
- [ ] React | ||
- [x] Asymmetric Any | ||
- [x] Asymmetric Object | ||
|
||
`format` has two parts, `diffFormat`(desperately needs a name) and `print`. The first one returns an array of `Line` objects and the second serializes this array based on options. | ||
|
||
### `Line` type | ||
|
||
`Line` is a representation of a terminal line that will be printed on the screen. It has `type` which can be Inserted, Common or Deleted. The type determines the color of the content and diff indicator('-', '+'). `val` is a value that will be serialized. `prefix` and `suffix` are added to the serialized value, as well as `indent`. | ||
|
||
`Line` also has `skipSerialize` field which can be set to `true` if you want to the full control of `val` serialization. | ||
|
||
Note: `Line` can be printed as multiple lines too, but that is up to print function. | ||
|
||
Example: | ||
|
||
```ts | ||
const line: Line = {type: Inserted, prefix: '"a": ', sufix: ',', val: {b: 1}}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, when reading this example (in particular the To express what I understand in my words and make sure we're on the same page: At the risk of suggesting something you had already thought about and know is problematic:
or tree 2:
If we have tree 2 (we still go all the way into an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's an interesting approach. I am investigating these days whenever I have time. From a high-level view, this approach would change the responsibilities of From the implementation view, all nested structure diff functions require two values. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the responsibility is still fairly clear, it marks the parts that were different with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some updates:
export function diffObjects(
a: Record<Key, unknown>,
b: Record<Key, unknown>,
path: Path,
diff: Function,
flatten: Function,
): DiffObject<Record<Key, unknown>> {
// omited
if (otherIndex >= 0) {
childDiffs.push(diffFunc(a[key], b[key], key));
bKeys[otherIndex] = null;
} else {
childDiffs.push(
flatten({val: a[key], path: key, kind: Kind.DELETED, flatten}),
);
}
// omited
} So it with extra step it would be diff -> flatten -> format -> print. Not sure if it's worth it, but something I would be willing to try.
const insertedDiff: DiffObject = {
b: diffObj.b,
childDiffs: diffObj.bChildDiffs,
kind: Kind.INSERTED,
path: diffObj.path,
};
const deletedDiff: DiffObject = {
a: diffObj.a,
childDiffs: diffObj.aChildDiffs,
kind: Kind.DELETED,
path: diffObj.path,
};
return [
...originalFormat(deletedDiff, context, options),
...originalFormat(insertedDiff, context, options),
]; In the future, formatter might just show the name of the constructor only when types are unequal or be smart about it in other ways.
if (test(a)) {
return a.asymmetricMatch(b)
? diff(b, b, path, memos) // Hacky way to get flattened equal object
: flatten({ a, b, path, kind: TYPE_UNEQUAL });
}
Meanwhile, I will work to clean up the code(mostly types) and push it. But hopefully, this is enough to get the idea of some problems. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm, I'm not sure I follow. You are raising problems that occur when trying to flatten a diff object tree, but what I was trying to say is I would want to avoid even doing a flatten at all costs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are couple of problems with that: For now it's not a problem, it might be but we can worry about it later. None of this traversal changes are catastrophic and can always be reverted. I will try to push code ASAP now so you also can get clearer idea There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, couldn't it still be represented differently to make the strict equality const childDiffs = [];
let commonKeys /* = ... */;
let keysOnlyInB /* = ... */;
for (commonKey of commonKeys) {
childDiffs.push(diff(a[commonKey], b[commonKey]))
}
for (keyOnlyInB of keysOnlyInB) {
childDiffs.push({
...diff(undefined, b[keyOnlyInB]),
kind: 'INSERTED',
})
} This is a really interesting conversation by the way 👌 in general assertions and diffing are such an interesting topic 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree it's very interesting and it's great to hear your thought process about it too. I have been working on this whenever I had time. Pushing the newest changes so you can see them, but it's not final work. Additionally improved typing and rebased, which was some work considering how much changed. I am planning to work on it more actively this week. Do you have any remarks/questions regarding overall architecture? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The union type does look cool indeed 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have tried and it has some quirks. Not saying no to it, but I wanted to implement some other types and see some diffs first. Since diff function API is mostly agreed on, how we implement it can be improved down the line. |
||
``` | ||
|
||
can be rendered as | ||
|
||
``` | ||
....more diff | ||
|
||
- "a": Object { | ||
"b": 1, | ||
}, | ||
|
||
...more diff | ||
``` | ||
|
||
or | ||
|
||
``` | ||
....more diff | ||
|
||
- "a": Object {...}, | ||
|
||
...more diff | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
{ | ||
"name": "jest-deep-diff", | ||
"version": "29.0.2", | ||
"main": "./build/index.js", | ||
"types": "./build/index.d.ts", | ||
"exports": { | ||
".": { | ||
"types": "./build/index.d.ts", | ||
"default": "./build/index.js" | ||
}, | ||
"./package.json": "./package.json" | ||
}, | ||
"repository": { | ||
"type": "git", | ||
"url": "https://github.com/facebook/jest", | ||
"directory": "packages/jest-deep-diff" | ||
}, | ||
"engines": { | ||
"node": "^14.15.0 || ^16.10.0 || >=18.0.0" | ||
}, | ||
"bugs": { | ||
"url": "https://github.com/facebook/jest/issues" | ||
}, | ||
"dependencies": { | ||
"chalk": "^4.0.0", | ||
"diff-sequences": "workspace:^", | ||
"expect": "workspace:^", | ||
"pretty-format": "workspace:^" | ||
}, | ||
"devDependencies": { | ||
"fast-check": "^1.23.0", | ||
"jest-diff": "workspace:^", | ||
"strip-ansi": "^6.0.0" | ||
}, | ||
"homepage": "https://jestjs.io/", | ||
"license": "MIT" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,208 @@ | ||
/** | ||
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
// adapted from pretty-format/perf/test.js | ||
const chalk = require('chalk'); | ||
const oldDiff = require('jest-diff').default; | ||
const diff = require('../build').default; | ||
const bigJSON = require('./world.geo.json'); | ||
|
||
const NANOSECONDS = 1000000000; | ||
let TIMES_TO_RUN = 10000; | ||
const LIMIT_EXECUTION_TIME = 40 * NANOSECONDS; | ||
|
||
const deepClone = obj => JSON.parse(JSON.stringify(obj)); | ||
|
||
function testCase(name, fn) { | ||
let error, time, total, timeout; | ||
|
||
try { | ||
fn(); | ||
} catch (err) { | ||
error = err; | ||
} | ||
|
||
if (!error) { | ||
const start = process.hrtime(); | ||
|
||
let i = 0; | ||
let currentTotal; | ||
for (; i < TIMES_TO_RUN; i++) { | ||
const diff = process.hrtime(start); | ||
currentTotal = diff[0] * 1e9 + diff[1]; | ||
if (currentTotal > LIMIT_EXECUTION_TIME) { | ||
timeout = true; | ||
break; | ||
} | ||
fn(); | ||
} | ||
|
||
total = currentTotal; | ||
|
||
time = Math.round(total / TIMES_TO_RUN); | ||
} | ||
|
||
return { | ||
error, | ||
name, | ||
time, | ||
timeout, | ||
total, | ||
}; | ||
} | ||
|
||
function test(name, a, b) { | ||
const oldDiffResult = testCase('Old diff', () => oldDiff(a, b)); | ||
|
||
const diffResult = testCase('Deep diff', () => diff(a, b)); | ||
|
||
const results = [oldDiffResult, diffResult].sort((a, b) => a.time - b.time); | ||
|
||
const winner = results[0]; | ||
|
||
results.forEach((item, index) => { | ||
item.isWinner = index === 0; | ||
item.isLoser = index === results.length - 1; | ||
}); | ||
|
||
function log(current) { | ||
let message = current.name; | ||
|
||
if (current.timeout) { | ||
message += ` Could not complete ${TIMES_TO_RUN} iterations under ${ | ||
LIMIT_EXECUTION_TIME / NANOSECONDS | ||
}s`; | ||
// eslint-disable-next-line no-console | ||
console.log(' ' + chalk.bgRed.black(message)); | ||
return; | ||
} | ||
|
||
if (current.time) { | ||
message += ' - ' + String(current.time).padStart(6) + 'ns'; | ||
} | ||
if (current.total) { | ||
message += | ||
' - ' + | ||
current.total / NANOSECONDS + | ||
's total (' + | ||
TIMES_TO_RUN + | ||
' runs)'; | ||
} | ||
if (current.error) { | ||
message += ' - Error: ' + current.error.message; | ||
} | ||
|
||
message = ' ' + message + ' '; | ||
|
||
if (current.error) { | ||
message = chalk.dim(message); | ||
} | ||
|
||
const diff = current.time - winner.time; | ||
|
||
if (diff > winner.time * 0.85) { | ||
message = chalk.bgRed.black(message); | ||
} else if (diff > winner.time * 0.65) { | ||
message = chalk.bgYellow.black(message); | ||
} else if (!current.error) { | ||
message = chalk.bgGreen.black(message); | ||
} else { | ||
message = chalk.dim(message); | ||
} | ||
|
||
// eslint-disable-next-line no-console | ||
console.log(' ' + message); | ||
} | ||
|
||
// eslint-disable-next-line no-console | ||
console.log(name + ': '); | ||
results.forEach(log); | ||
// eslint-disable-next-line no-console | ||
console.log(); | ||
} | ||
|
||
const equalPrimitives = [ | ||
['boolean', true, true], | ||
['string', 'a', 'a'], | ||
['number', 24, 24], | ||
['null', null, null], | ||
['undefined', undefined, null], | ||
]; | ||
|
||
for (const [type, a, b] of equalPrimitives) { | ||
test(`equal ${type}`, a, b); | ||
} | ||
|
||
const unequalPrimitives = [ | ||
['boolean', true, false], | ||
['string', 'a', 'A'], | ||
['number', 24, 42], | ||
['null and undefined', null, undefined], | ||
]; | ||
|
||
for (const [type, a, b] of unequalPrimitives) { | ||
test(`unequal ${type}`, a, b); | ||
} | ||
|
||
const smallJSON = { | ||
features: { | ||
a: 1, | ||
b: 3, | ||
c: { | ||
key: 'string', | ||
}, | ||
}, | ||
topLevel: 3, | ||
}; | ||
|
||
const smallJSONDeepEqual = deepClone(smallJSON); | ||
|
||
test('deep equal small objects', smallJSON, smallJSONDeepEqual); | ||
|
||
const smallJSONUpdated = { | ||
features: { | ||
a: 1, | ||
b: 4, | ||
c: { | ||
key2: 'string', | ||
}, | ||
}, | ||
topLevel: 4, | ||
}; | ||
|
||
test('updated small objects', smallJSON, smallJSONUpdated); | ||
|
||
TIMES_TO_RUN = 100; | ||
|
||
const mediumJSON = { | ||
...bigJSON, | ||
features: bigJSON.features.slice(10), | ||
}; | ||
|
||
const changedMediumJSON = { | ||
...bigJSON, | ||
features: deepClone(bigJSON.features.slice(4, 14)), | ||
}; | ||
test('Medium object with diff', mediumJSON, changedMediumJSON); | ||
|
||
const mediumJSONDeepEqual = deepClone(mediumJSON); | ||
|
||
test('Medium object with deep equality', mediumJSON, mediumJSONDeepEqual); | ||
|
||
const objectWithXKeys1 = {}; | ||
const objectWithXKeys2 = {}; | ||
|
||
const keyNumber = 20; | ||
|
||
for (let i = 0; i < keyNumber; i++) { | ||
objectWithXKeys1['key' + i] = Math.round(Math.random()); | ||
objectWithXKeys2['key' + i] = Math.round(Math.random()); | ||
objectWithXKeys1[Math.random().toString(36)] = i; | ||
objectWithXKeys2[Math.random().toString(36)] = i; | ||
} | ||
|
||
test('Object with a lot of keys', objectWithXKeys1, objectWithXKeys2); |
Uh oh!
There was an error while loading. Please reload this page.