Jest: Jest stalls after comparing to complex objects

[sebmarkbage]

(version v15.1.1)

In this PR, I can't use expect(a).toBe(b); on two complex and cyclic objects. It works fine if the test pass but if the test fails I think it tries to print/report the object structure or something. Interestingly enough it works fine on some of the other similar objects.

The end result is that jest stalls and doesn't print or proceed with anything else. I thought it might have been an infinite loop but if I run it with the Chrome debugger in Node 6 I can't stop. There's no break point so it seems like the event loop is just paused waiting for new messages that never arrive.

[chase]

@sebmarkbage It looks like the original commit that caused this failure was dropped during a history rewrite.

I checked out your fork and went to the next similar commit at sebmarkbage/react@f904163a6042b77ab82fa351ed459be26629ed88, then replaced the originally problematic line in src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js

    ReactNoop.flush();

    var instanceC = instance;

-    expect(instanceC === instanceA).toBe(true);
+    expect(instanceC).toBe(instanceA);
  });

When running jest at master (commit 2e36e259657236dc79484bcc54fb42d7e18bff62), the test fails but does not stall. I doubt that this is comparing the correct object, because the expected and received values are both null.

This is the output of the test: https://gist.github.com/chase/cc980da37005dc0d48bed7b3cdd9f4d5

It might be fixed in jest@master now, but I don't have access to the original commit where it broke.

[sighrobot]

(react 15.3, jest-cli 16.0)

This happens consistently when I expect(instance).toBeNull() when instance is in fact not null.

[gaearon]

I can reproduce this with a simple test:

it('hangs', function() {
  var elem = document.createElement('div');
  expect(elem).toBe(undefined);
});

It hangs forever on Jest 16.0.1.

[probablyup]

@cpojer any idea if you guys will be able to get to this soonish? Keeps coming up for my team and tracking down the culprit assertion is not always easy.

[cpojer]

It's faster if you send a PR! Unfortunately I've been pretty backed up with a ton of work.

[probablyup]

Hmm, ok I'll try and make time to look into this tomorrow.

[chase]

I did a little digging and it seems that @sebmarkbage's analysis was right.
Large or cyclic objects when passed to prettyFormat causes the cli to stall as the maximum depth is Infinity by default.

I will put in a basic PR tomorrow morning that at least deals with the symptoms mentioned, but I think that a long term solution would be to limit the verbosity or object depth being printed with a CLI option to adjust it.

[probablyup]

Fix submitted to pretty-print https://github.com/thejameskyle/pretty-format/pull/47

[probablyup]

Downgrade to jest-cli@15 if you want to avoid this bug until it's addressed.

[chase]

@yaycmyk, thanks for putting in the effort for HTML element printing. I'm sure your hours of work won't go to waste.

I got pulled for some crunch time tasks and ultimately got distracted from my pull request.
@cpojer, I forgot to ask: would the test be a better fit as an integration test? Seems like there's no easy option to use jsdom for a package's unit test as they all default to node for the testEnvironment.

[cpojer]

Sure, that's fine too.

[probablyup]

https://github.com/facebook/jest/pull/2078

[gaearon]

https://github.com/facebook/jest/pull/2078#issuecomment-260090774

[gaearon]

This still happens for us in React repo with the latest Jest.
Unfortunately I'm having troubles trying to reproduce it outside of React repo :-(

I made a repro case inside React repo.

git clone https://github.com/gaearon/react.git react-jest-bug-repro
cd react-jest-bug-repro
git checkout jest-bug
yarn
npm test -- JestBug

Expected: it doesn't hang.
Actual: it hangs.

Commit with the repro: https://github.com/gaearon/react/commit/8a0e694beee2a2b66eaff6ecd243063de85ce8da.

[aaronabramov]

should be fixed in https://github.com/facebook/jest/pull/2148
@gaearon thanks for the repro case! I think it was getting stuck trying to send a very long error message from a child process to the master jest process.

[sebmarkbage]

I have another case:

https://github.com/sebmarkbage/react/commit/32f450ca2753d0bbb78fb1af32fbc094d94c568f

I tried to patch in the fix in #2148 but it still stalls.

[cpojer]

@sebmarkbage can you send a PR to Jest with a failing test?

[sebmarkbage]

@cpojer I don't know how to reduce it yet. Pretty complex and don't know how to debug. Looks to be something with jest-diff when it calculates the diff.

[sebmarkbage]

The issues is that jest-diff returns a too big string: https://github.com/facebook/jest/blob/master/packages/jest-diff/src/index.js#L107

I tried to use the same fix as https://github.com/facebook/jest/pull/2148 but that makes it way too slow because running it over and over again at different depths is very slow when the tree is large.

Max depth is also not sufficient even for the first fix when you have a flat tree but many properties and large strings in each object. Even just a single long string at many characters (>10000) would probably be enough.

Gotta run but I guess that's another plausible unit test.

[ArtemGovorov]

Here's a simple test that hangs jest:

it('takes 37 seconds', () => {
  expect(location).toBe({});
});

It actually finishes in ~37 seconds on my machine, seems like producing a huge diff.

[thymikee]

Ok, so I finally found some time to test it and indeed thats the diff library we're using which causes problems.

We could consider changing our diff to utilise Google's diff-match-patch, which further optimises Meyer's algorithm with pre/post diff work (which is super fast for partially similar strings).

Although it still can choke on some bigger totally unrelated values (at least on default options), it can process values at least 3 orders of magnitude higher.

[cpojer]

Is there anybody who'd want to add Google's JS implementation for Jest 20? It seems like it should be fairly easy to switch over to a new implementation, no? :)

[alextes]

So.. Close..

Don't let my progress discourage anyone from taking on the issue 😉 .

[cpojer]

@AlexTes if you have already made progress, how about sending a PR? :) We should be able to fix these things up together.

[pedrottimark]

@cpojer @gaearon The comments about maxDepth brought me here, because the pretty-format plugin API which incompletely exposes current indentation also doesn’t expose current depth, therefore the maxDepth option:

• doesn’t apply to levels of structure that the plugin prints directly (for example, React elements)
• but does apply to its print callback arg (for example, if a prop value is an object)

Although depth of React elements wasn’t a problem here for <div>Hello</div> or <div>World</div> what are your general expectations for a message when a test fails?

If it’s hard to imagine limited depth for React elements, consider plugins for Immutable data.

To be clear, this comment isn’t about the difference from jest-diff (which is the subject of the preceding few comments) but about the message from jest-matchers

expect(received).toBe(expected)

    Expected value to be (using ===):
      tweedle dee pretty format options include min and maxDepth…
    Received:
      tweedle dum …but plugins cannot enforce maxDepth, nor refs

[cpojer]

Is this still a problem with the latest Jest 21.3 betas?

[gaearon]

Does 21.3.0-beta.4 count as latest? (I haven't checked yet, just asking)

[thymikee]

@cpojer AFAIK @pedrottimark is working on a proper fix for that (but it's probably going to be a big rewrite of our diffing algo, so it will take a while), so please keep this open for reference.
@gaearon FYI latest Jest is 21.3.0-beta.15

[pedrottimark]

Yes, I’m working on tests for new jest-diff-arrays package at this very moment :)

An update is overdue: if I came anywhere close to reproducing the original problem, when the test failed before implementing the feature:

• serialization of instanceA was about 500 lines
• serialization of instanceC was about 35000 lines

For this edge case of many instead of few differences, the diff dependency took about 5 minutes on Node.js 8 and 4 minutes on Node.js 9 because it allocates hundreds of millions of temporary objects. The replacement package for arrays has similar optimizations as diff-match-patch for strings to run in 0.025 seconds because its only heap storage is array of 35000 small integers and it short circuits zillions of loop iterations.

However, end result is that unusual looking assertion expect(instanceC === instanceA).toBe(true) remains if you don’t want 35000 lines of diff in console, no matter how fast Jest computes it.

[cpojer]

Got it, thanks for the update!

[carusology]

I got this to occur with image files. I was seeing if an image conversion tool I was using omitted an identical image as output by comparing buffers using Jest. If they were identical buffers, it passed without issue. If they weren't it hung. It seems to be just as @sebmarkbage described. Swapping to a simple boolean check or a hash was an easy workaround once I figured out this assertion was hanging.

I can't seem to get it to repro with any ol' test image file though, so you get to see my test pdf 😅. Maybe this will help @pedrottimark ?

const http = require('http');
const path = require('path');
const fs = require('fs');

test('this test will stall', async () => {
  let stallingImage1 = await download('stalled-image-1.png');
  let stallingImage2 = await download('stalled-image-2.png');

  let stallingBuffer1 = fs.readFileSync(stallingImage1);
  let stallingBuffer2 = fs.readFileSync(stallingImage2);

  expect(stallingBuffer1).toEqual(stallingBuffer2);
});

var download = (image) => {
  const bucket = 'http://storage.googleapis.com/gitlab-issue-downloads/';

  let localPath = path.resolve(__dirname, image);
  let file = fs.createWriteStream(localPath);

  return new Promise(
    (resolve, reject) => {
      http.get(bucket + image, function(response) {
        response.pipe(file);
        resolve(localPath);
      });
    }
  );
}

[davidlygagnon]

We're also seeing this issue with large snapshot files. Has anyone found a workaround?

[TUgonna]

Currently, experiencing the same issue. any fix?

[matthew-dean]

I've also ran into this, where a failure in a comparison of two deep objects (with circular references) will cause the script to appear to stall for nearly a minute. I just want it to fail and say, "No, it's not the correct object reference," not a deep comparison of the instance properties, which is not valuable at all.

[matthew-dean]

Seems like the fix is to change expect(a).toBe(b) to expect(a === b).toBe(true)

[pedrottimark]

@matthew-dean Yes, expect(a === b).toBe(true) avoids the problem.

Although expect(a).toBe(b) does test object identity instead of deep equality, the problem is if the assertion fails, then Jest displays the differences in the serialization of received and expected values. That output might not be relevant, even if we can speed it up.

@carusology Yeah, the serialization for buffers is huge, and I doubt that the diff is relevant to you.

What do you think about expect(stallingBuffer1.equals(stallingBuffer2)).toBe(true) as assertion?

@davidlygagnon @TUgonna Am back on this again. In jest-diff which displays results when an assertion fails, we will replace diff package with diff-sequences package. There is a huge performance gain when there is a big difference in number of lines between received and expected value.

Do you know if that is the situation when y’all see this issue?

[hisapy]

I'm having this problem with not so big snapshot files. In my case I got errors like

<--- Last few GCs --->

[65064:0x105800000]   242971 ms: Mark-sweep 1411.7 (1473.4) -> 1411.5 (1442.4) MB, 5512.1 / 0.2 ms  last resort GC in old space requested
[65064:0x105800000]   250116 ms: Mark-sweep 1411.5 (1442.4) -> 1411.5 (1442.4) MB, 7144.9 / 0.1 ms  last resort GC in old space requested


<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 0x184239f25879 <JSObject>
    1: indexOf(this=0x1842b7c3ec71 <Very long string[424567]>)
    2: /* anonymous */(aka /* anonymous */) [/Users/hisa/workspace/eljurista/node_modules/pretty-format/build/plugins/lib/markup.js:43] [bytecode=0x1842aac77119 offset=60](this=0x18425d9822d1 <undefined>,key=0x1842c4ab87a1 <String[5]: relay>)
    3: arguments adaptor frame: 3->1
    4: map(this=0x1842b7c79869 <JSArray[17]>)
    5: /*...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
 1: node::Abort() [/usr/local/Cellar/node@8/8.12.0/bin/node]
 2: node::FatalException(v8::Isolate*, v8::Local<v8::Value>, v8::Local<v8::Message>) [/usr/local/Cellar/node@8/8.12.0/bin/node]
 3: v8::Utils::ReportOOMFailure(char const*, bool) [/usr/local/Cellar/node@8/8.12.0/bin/node]
 4: v8::internal::V8::FatalProcessOutOfMemory(char const*, bool) [/usr/local/Cellar/node@8/8.12.0/bin/node]
 5: v8::internal::Factory::NewRawTwoByteString(int, v8::internal::PretenureFlag) [/usr/local/Cellar/node@8/8.12.0/bin/node]
 6: v8::internal::String::SlowFlatten(v8::internal::Handle<v8::internal::ConsString>, v8::internal::PretenureFlag) [/usr/local/Cellar/node@8/8.12.0/bin/node]
 7: v8::internal::String::IndexOf(v8::internal::Isolate*, v8::internal::Handle<v8::internal::String>, v8::internal::Handle<v8::internal::String>, int) [/usr/local/Cellar/node@8/8.12.0/bin/node]
 8: v8::internal::Runtime_StringIndexOfUnchecked(int, v8::internal::Object**, v8::internal::Isolate*) [/usr/local/Cellar/node@8/8.12.0/bin/node]
 9: 0x17f1435042fd
10: 0x17f1435683ba
error Command failed with signal "SIGABRT"

it hangs on

expect(tree).toMatchSpecificSnapshot(snapshotFilename);

//  tree is a mount wrapper from enzyme

Is there any progress or workaround for this for snapshot testing?

[hisapy]

I found that my snapshots were huge and were the cause of the out of memory error... luckily I also found mode: 'deep' option of enzyme-to-json and changed my snapshot strategy to split large files into multiple smaller files.

[siluri]

i detect at v23.6.0 following bug:

arrStatus[0] is a complex object

that expect is correct and the expect works in miliseconds

expect(arrStatus[0]).toEqual({
   property1: expect.any(Object),
   property2: expect.any(Object),
   stringValue: 'included'
});

but do the same expect, with wrong string which not expect to the object - never ends up and my pc fan running on full throttle

expect(arrStatus[0]).toEqual({
   property1: expect.any(Object),
   property2: expect.any(Object),
   stringValue: 'not_included'
});

edit
change toEqual to toBe - never ends up also

expect(arrStatus[0]).toBe({
   property1: expect.any(Object),
   property2: expect.any(Object),
   stringValue: 'not_included'
});

Got following Errorby stringify the complex Object

JSON.stringify(arrStatus[0].property1);
TypeError: Converting circular structure to JSON

so i guess the main problem is a infinite loop because we have a circular structure in the complex object. so we need a solution to prevent this:

• stop expecting after defined times runs up
• throw error by comparison of circular structure object
• or better solution ;)

[SimenB]

We have an open PR that should hopefully fix this, see #6961. Will land as part of Jest 24