Fix subsetEquality: same referenced object on same level node of tree is regarded as circular reference

[gejimayu]

Summary

This fixes #9089
This bug is introduced by this PR #8687
So basically, there is a flaw on the circular reference logic within a function called subsetEquality.
The logic is that, to avoid circular reference, it stores the references of nodes that it has been through within one singleton variable seenReferences. Whenever we have seen the same node (using the reference), we skip the subset equality checking (which is done recursively) and do deep equality checking instead.
The implication is that nodes within the same level of tree will use the same reference seenReferences. It's a problem because then it will cause an invalid handling on this case :

const referencedObj = {abc: 'def'};
const object = {
  a: {abc: 'def'},
  b: {abc: 'def', zzz: 'zzz'},
};
const thisIsNotCircular = {
  a: referencedObj,
  b: referencedObj,
};
expect(subsetEquality(object, thisIsNotCircular)).toBeTruthy(); // this test fails while it should succeed

On current logic, above case is considered circular referenced which isn't supposed to be. It will result in incorrect handling because circular reference does not check for subset equality and only check for plain equality instead. It makes sense for circular referenced object but not for normal object.

What we should do is to keep the seenReferences only for parent node and its children. The solution is quite simple, we only need to remove the reference immediately after the recursive function is done called for all of a node's children.

Test plan

I have added one test case using example above which would fail previously.
After the fix, now it succeeds.

[codecov-io]

Codecov Report

Merging #9322 into master will increase coverage by 0.23%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #9322      +/-   ##
==========================================
+ Coverage   64.76%   64.99%   +0.23%     
==========================================
  Files         280      278       -2     
  Lines       11985    11913      -72     
  Branches     2956     2935      -21     
==========================================
- Hits         7762     7743      -19     
+ Misses       3591     3539      -52     
+ Partials      632      631       -1

Impacted Files	Coverage Δ
packages/expect/src/utils.ts	94.96% <100%> (-1.24%)	⬇️
packages/jest-transform/src/ScriptTransformer.ts	68.49% <0%> (-0.15%)	⬇️
packages/jest-config/src/index.ts	11.76% <0%> (ø)	⬆️
packages/jest-config/src/normalize.ts	77.74% <0%> (ø)	⬆️
e2e/v8-coverage/no-sourcemap/x.css
e2e/v8-coverage/no-sourcemap/Thing.js
packages/jest-runner/src/runTest.ts	2.43% <0%> (+0.24%)	⬆️
packages/jest-runtime/src/index.ts	67.34% <0%> (+1.9%)	⬆️
packages/jest-reporters/src/coverage_reporter.ts	61.83% <0%> (+10.61%)	⬆️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89c151b...cb4e2ea. Read the comment docs.

[SimenB]

Thanks! This change seems reasonable to me, but I'm not too familiar with this part of the code base.

Mind updating the changelog as well?

[jeysal]

LGTM, thanks a lot!

[gejimayu]

You're welcome. Thanks for a very swift response !
I'm really glad that i could help and contribute. Looking forward to contribute more ! This is my very first open source contribution by the way hahah

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.