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

Objects/arrays defined on classes are a vector for state leakage in tests #13558

Closed
aquamme opened this issue May 25, 2016 · 4 comments
Closed

Comments

@aquamme
Copy link

aquamme commented May 25, 2016

Twiddle

I was seeing some intermittent test failures and tracked it down to this. Fortunately for me, none of the objects or arrays defined on any of our classes actually needed to be shared across instances. Users who need this behavior are left with these options:

  • Ignore the intermittent test failures that may be caused by cross contamination between tests
  • Manually reset class state between tests
  • Switch to a different mechanism for sharing state between instances of a class
  • Some other option I'm not thinking of offhand

The guides advertise the behavior of objects/arrays defined on classes, but nowhere do I see it documented that this can cause state leakage between tests.

While this may not strictly speaking be a bug, it can definitely be a painful gotcha. At the least, I think some documentation should be added, but perhaps some other action is warranted.

@Serabe
Copy link
Member

Serabe commented May 25, 2016

Thank you for your report. In this case, this is not a bug and is documented in the guides.

I am closing this. Reopen it if I am mistaken.

Thank you so much for your report!

@Serabe Serabe closed this as completed May 25, 2016
@aquamme
Copy link
Author

aquamme commented May 25, 2016

Yes, I did actually link to that guide. What I want to point out though is that this feature can cause unpredictable behavior with tests, because the class state doesn't get reset between tests. That caveat isn't documented, but even if it were, it would be tedious and error prone to deal with. The test writer would have to be aware that the state isn't automatically reset between tests (unlike other app state), and write code to reset it manually between every test. You can take a peek at the twiddle for an example of test cross contamination.

@Serabe -- I don't have permission to reopen this issue.

@Serabe
Copy link
Member

Serabe commented May 25, 2016

This is a expected behaviour, not only in Ember but in Javascript. You can check in this jsbin.

This is clearly not expected coming from other language semantics.

This will be solved once the Ember Object model has been migrated to ES6 classes, but it depends on a couple of proposals for TC39 (I am thinking about Class Fields and Static Properties and Decorators).

This was originally discussed in #10341. There is no RFC open for that yet but you can see more discussion about ES5 getters/setters in this one

@aquamme
Copy link
Author

aquamme commented Jun 1, 2016

I understand it's a feature/quirk of the language, however, it seems asymmetric to me that the guides would point out the feature/quirk but not also warn of the potential complications in a test environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants