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

Expect holds reference to value, generates error string at the end of test (after mutation) #429

Closed
jimfb opened this issue Jul 10, 2015 · 4 comments

Comments

@jimfb
Copy link

jimfb commented Jul 10, 2015

var foo = [1];
expect(foo).toEqual([1, 2]);
foo.length = 0;

expect should print the expected value as [1] but prints as [].

@jimfb jimfb changed the title Expect holds reference to value, generates error string at the end of test Expect holds reference to value, generates error string at the end of test (after mutation) Jul 10, 2015
@zpao
Copy link
Contributor

zpao commented Jul 13, 2015

This might be a problem in Jasmine itself. Perhaps when/if we upgrade to a newer version of that this will just work.

@jimfb
Copy link
Author

jimfb commented Mar 3, 2016

@cpojer, In #434 as a reason for closing the issue, you said:

Cloning here would be too expensive for performance. I understand this is an issue but the side-effect in your function is probably not something you'd want in your application anyway, I'll assume? While I do not know what your application is doing, shallow copying the object in your function Object.assign({}, obj) is probably reasonable.

I don't think you need to clone, just generate the string representation at expect time, for use in the error message when it gets printed. It shouldn't be that expensive (said without knowing the internals). Having code that mutates (especially adding to a list) is pretty damn common. And when you're debugging, this error is extraordinarily frustrating because you're banging your head against the wall going "how could that be?" only to realize that the test framework lied to you.

@cpojer
Copy link
Member

cpojer commented Apr 16, 2016

Fixed in the next release.

@cpojer cpojer closed this as completed Apr 16, 2016
ghost pushed a commit that referenced this issue Apr 16, 2016
Summary:related to #429

It can be an expensive operation, but it only occurs when a test fails.

the following test:
```js
  it('keeps the reference', function() {
    var a = [1];
    expect(a).toEqual([1, 2]);
    a.push(2);
  });
```

before:
![screen shot 2016-04-12 at 5 55 40 pm](https://cloud.githubusercontent.com/assets/940133/14479858/ded42e0e-00d7-11e6-8841-0d817ca02d45.png)

after:
![screen shot 2016-04-12 at 5 56 03 pm](https://cloud.githubusercontent.com/assets/940133/14479860/e3d1710a-00d7-11e6-837f-e2f2bdfb27b7.png)
Closes #889

Differential Revision: D3188934

fb-gh-sync-id: 02e8b2e3466d7994a1d3e5e428b80ddd0f455fe5
fbshipit-source-id: 02e8b2e3466d7994a1d3e5e428b80ddd0f455fe5
@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 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants