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

handleCreate seems to require an Em.run #62

Closed
digitalcora opened this issue Feb 10, 2015 · 9 comments
Closed

handleCreate seems to require an Em.run #62

digitalcora opened this issue Feb 10, 2015 · 9 comments

Comments

@digitalcora
Copy link

When using testHelper.handleCreate in an acceptance test, I get an error that requires me to wrap it in an Em.run call. It looks like this is because handleCreate uses createRecord, which can have asynchronous side effects.

@danielspaniel
Copy link
Collaborator

@Grantovich .. yes .. I have the same issue, and I wrap all tests in a run loop that use handleCreate and other methods as you say that have async side effects .. do you think it is better for me to wrap that call in handleCreate with run loop, or for you to wrap your tests?

@digitalcora
Copy link
Author

I'm not sure of the solution, but as someone using an Ember library I usually expect that all the library's functions will work in a default Ember environment, unless documented otherwise – and by default, tests are not wrapped in a run loop.

Possible solutions, in increasing order of complexity:

  1. Document the fact that handleCreate must always be wrapped in a run loop by the user.
  2. Wrap handleCreate in a run loop internally. This removes the need for users to do it, but seems like kind of a band-aid (might have performance implications?).
  3. Find a way to avoid calling createRecord within handleCreate. This feels like the "right" solution since handleCreate shouldn't need to create any records itself – the purpose of the function is just to set up an HTTP mock. I don't know how much work it would take to accomplish this, though.

@danielspaniel
Copy link
Collaborator

Good analysis, and I vote for #3 .. it is annoying that I am creating a record to be able to use the serializer .. but I could not find a way around it .. all entry to serializer uses a record .. ( that I could find ) .. could you take a look at what I am doing and see if you have another way to accomplish that .. ??
In order to compare the matching parameters to the ones that the record is actually making ( and it gets to use the serializer ) .. I think I also have to use the serializer to be able to make a true match.
Any ideas?

@digitalcora
Copy link
Author

I'll take a look when I have some free time. It might not be necessary to do the serialization if handleCreate were treated as a straight HTTP mock function (for instance, you'd have to give it the IDs of associated models rather than the models themselves), but then that wouldn't line up with the way handleUpdate does it.

@danielspaniel
Copy link
Collaborator

Well, the problem or lack of problem is that handleUpdate does not do any matching... it just mocks an update .. and does not care about what exact attributes are being updated. handleCreate on the other hand does try and make that match .. which I added because people have asked me to make handleCreate with ability to match attributes. It used to be just like handleUpdate ( did not bother with matching ) ..
not sure if any one cares to have handleUpdate try and match attributes ( no one has mentioned it )
anyway .. it's good to ponder this one and I will look again at how I can remove the createRecord in there .. I don't love it at all

@danielspaniel
Copy link
Collaborator

I have not found another way to avoid creating the new record. I am going to go with idea #2 then, and wrap the call to createRecord in Em.run call .. and will commit that code and make a new version ( next few days )

@danielspaniel
Copy link
Collaborator

I have setup my tests so they fail now without that run loop as they should have before ( I had forgotten to setup my tests the way a normal Ember application would by calling the setupForTesting method.
So then, when I tried my fix inside of handleCreate, it did not work.
The only thing that worked was a run loop around the whole test.
So I have documented that in the Readme file, so people will know what to expect.
I just pushed that code and new docs.
If you have any other questions/ comments /objections let me know .. else I will close this issue.

@digitalcora
Copy link
Author

So then, when I tried my fix inside of handleCreate, it did not work.
The only thing that worked was a run loop around the whole test.

That's odd... in my tests, wrapping just the handleCreate in a run loop was enough for it to work. Give me some time to poke at it, I'd like to know if this is a quirk of my test setup or something else.

@danielspaniel
Copy link
Collaborator

Ok .. let me clear up what I meant.
I was putting a run loop around the createRecord / destroyRecord in the MockCreateRequest function.
That did not work.
I think I tried also around just handleCreate .. but don't remember now if that worked or not. My main point was .. there was no way I could find to wrap stuff internally to avoid users having to wrap something themselves.
Thanks for following up though and helping to investigate.

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