Skip to content
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

iterables and toHaveBeenCalledWith result in RangeError: Maximum call stack size exceeded #6830

Closed
sorawee opened this issue Aug 11, 2018 · 8 comments · Fixed by #8160
Closed

Comments

@sorawee
Copy link

sorawee commented Aug 11, 2018

🐛 Bug Report

If toHaveBeenCalledWith is used with an iterable, RangeError: Maximum call stack size exceeded will be thrown.

To Reproduce

Steps to reproduce the behavior:

Run

class Blah {
  *[Symbol.iterator]() {
    yield this;
  }
}

const video = {
  play(x) {
    console.log('received', x);
    return true;
  },
};

test('plays video', () => {
  const val = new Blah();
  const spy = jest.spyOn(video, 'play');
  const isPlaying = video.play(val);

  expect(spy).toHaveBeenCalledWith(val);
  expect(isPlaying).toBe(true);
});

This results in:

 FAIL  __tests__/test.js
  ● Console

    console.log __tests__/test.js:9
      received Blah {}

  ● plays video

    RangeError: Maximum call stack size exceeded

      17 |   const isPlaying = video.play(val);
      18 | 
    > 19 |   expect(spy).toHaveBeenCalledWith(val);
         |               ^
      20 |   expect(isPlaying).toBe(true);
      21 | });
      22 | 

      at Object.toHaveBeenCalledWith (__tests__/test.js:19:15)

Test Suites: 1 failed, 1 skipped, 1 of 2 total
Tests:       1 failed, 17 skipped, 18 total
Snapshots:   0 total
Time:        1.401s, estimated 2s
Ran all test suites.

Expected behavior

The test passes.

Link to repl or repo (highly encouraged)

Code given above

Run npx envinfo --preset jest

Paste the results here:

It results in an error for me. I don't think it's what you want, but I will paste it here anyway.

(node:27012) UnhandledPromiseRejectionWarning: TypeError: Cannot read property '1' of null
    at a.run.then.e (/home/user/.npm/_npx/27012/lib/node_modules/envinfo/dist/cli.js:2:96634)
(node:27012) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:27012) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Here's the version in package.json

"jest": "^23.5.0",
@sorawee
Copy link
Author

sorawee commented Aug 11, 2018

Note that yield this is crucial here. Yielding something else like 0 won't cause a problem.

@SimenB
Copy link
Member

SimenB commented Aug 13, 2018

Minimum reproduction in Jest's own test suite:

diff --git i/packages/expect/src/__tests__/utils.test.js w/packages/expect/src/__tests__/utils.test.js
index 6a5e7d909..8ba302d89 100644
--- i/packages/expect/src/__tests__/utils.test.js
+++ w/packages/expect/src/__tests__/utils.test.js
@@ -13,6 +13,7 @@ const {
   emptyObject,
   getObjectSubset,
   getPath,
+  iterableEquality,
   subsetEquality,
 } = require('../utils');
 
@@ -150,3 +151,17 @@ describe('subsetEquality()', () => {
     expect(subsetEquality(undefined, {foo: 'bar'})).not.toBeTruthy();
   });
 });
+
+describe('iterableEquality()', () => {
+  test('self-returning iterator does not explode', () => {
+    class Iter {
+      *[Symbol.iterator]() {
+        yield this;
+      }
+    }
+
+    const val = new Iter();
+
+    expect(iterableEquality(val, val)).toBe(true);
+  });
+});

From what I can understand it recurses forever here: https://github.com/facebook/jest/blob/c9893d2f1cdbc98655ca446d2a49f8c77a42b4be/packages/expect/src/utils.js#L183-L188

Not sure how to handle it. Bail after say 1000 iterations? If val.next() returns val? If it returns itself n amounts of times?

@SimenB
Copy link
Member

SimenB commented Aug 13, 2018

@thymikee @rickhanlonii thoughts?

@sorawee
Copy link
Author

sorawee commented Aug 13, 2018

I'm no expert about this, but would checking reference equality helps?

@SimenB
Copy link
Member

SimenB commented Aug 13, 2018

Yeah, I think so. But for how many iterations should we do that before bailing? Maybe somebody has an iterator delivering fibonacci numbers - there are cases for infinite generators. Not sure what would give the best DX here

@rickhanlonii
Copy link
Member

I think bailing after 1000 makes sense, we can up it if necessary

@gamaamaral98
Copy link

Hi, I am a student of computer and software engineering and I choose this issue to work on in my software engineering subject so I'm no expert on this. As I was trying to understand this issue I looked into the comments and saw that the problem may be fixed if the cycle referenced above is changed. Would you give me some tips and help? Thanks a lot!

@github-actions
Copy link

This issue 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants