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

Calling save on models #28

Closed
zigomir opened this issue Mar 31, 2014 · 10 comments
Closed

Calling save on models #28

zigomir opened this issue Mar 31, 2014 · 10 comments

Comments

@zigomir
Copy link
Contributor

zigomir commented Mar 31, 2014

I'm happily using moduleForModel helper. But then to be able to call .save() on the record it returns you need to attach a "new" store to it like so

var flight = this.subject();
flight.set('store', App.__container__.lookup('store:main'));

Which is a bit ugly and flight record already contains its store. If I don't "re-attach" store like this I'll get the error when calling .save on the record

: store.serializerFor(...) is undefined
Source:
FixtureAdapter<.mockJSON@http://localhost:4001/vendor/ember-data/ember-data.js:722:9
FixtureAdapter<.createRecord@http://localhost:4001/vendor/ember-data/ember-data.js:833:1
_commit@http://localhost:4001/vendor/ember-data/ember-data.js:10423:1
Store<.flushPendingSave/<@http://localhost:4001/vendor/ember-data/ember-data.js:9659:11
utils.forEach@http://localhost:4001/vendor/ember/ember.js:2750:9
Store<.flushPendingSave@http://localhost:4001/vendor/ember-data/ember-data.js:9660:1
DeferredActionQueues.prototype.flush@http://localhost:4001/vendor/ember/ember.js:8175:17
Backburner.prototype.end@http://localhost:4001/vendor/ember/ember.js:8263:11
Backburner.prototype.run@http://localhost:4001/vendor/ember/ember.js:8302:13
apply@http://localhost:4001/vendor/ember/ember.js:7952:18
run@http://localhost:4001/vendor/ember/ember.js:6598:9
@http://localhost:4001/build/test/unit.js:37:7
wrapper@http://localhost:4001/vendor/ember-qunit/dist/globals/main.js:245:1
Test.prototype.run@http://localhost:4001/vendor/qunit/qunit/qunit.js:1303:4
run/<@http://localhost:4001/vendor/qunit/qunit/qunit.js:1463:5
process@http://localhost:4001/vendor/qunit/qunit/qunit.js:1016:4
QUnit.start/<@http://localhost:4001/vendor/qunit/qunit/qunit.js:186:5

I tried to fix the problem myself but I lack knowledge and couldn't figure it out just yet. Or am I doing something completely wrong for the kind of testing ember-qunit wants to provide?

@stefanpenner
Copy link
Member

so interestingly, moduleForModel doesn't currently setup enough for you to test saving. It make sense why you would want to though.

@zigomir
Copy link
Contributor Author

zigomir commented Mar 31, 2014

Yes. I've updated the stack trace. It's actually the store.serializerFor(...) is undefined that I get if I don't re-set the store. Although, if I compare stores (the one attached to record on moduleForModel and the one from App.__container__.lookup('store:main') I can see that the last one doesn't have any changed records in its recordArrayManager and typeMaps is empty here as well.

@zigomir
Copy link
Contributor Author

zigomir commented Apr 1, 2014

@stefanpenner @rpflorence can someone suggest me in which direction I can start digging, I'd love to help and find a solution for this

@stefanpenner
Copy link
Member

@zigomir
Copy link
Contributor Author

zigomir commented Apr 1, 2014

Thanks. So, if I change

return container.lookup('store:main').createRecord(name, options);

to

return App.__container__.lookup('store:main').createRecord(name, options);

it will work for my case. But with this change I break ember-qunit's tests because of course there is no global App in there.

@stefanpenner
Copy link
Member

@zigomir you do not want to do this, as it defeats the point of isolated tests. We must add all the proper ED serializers to the moduleForModels helper

@stas
Copy link

stas commented Apr 1, 2014

@zigomir I took a look at your tests and it seems more like an integration/acceptance test rather than a controller test. Mostly because calling save on your model depends a lot on the adapter you are using, the serializer and the server response. Also from what it seems, you have some actions being send in the tests (changeDepartureDate), which most likely are triggered by some UI.

I suggest you consider making this an acceptance test with appropriate mocked server response.
You can use either ember-testing-httpRespond or mockjax.

@zigomir
Copy link
Contributor Author

zigomir commented Apr 1, 2014

@stas Thanks for the response. You're right. This was my question as well. Currently I have a stub API for testing which is much more by definition of integration than unit testing. But I still somehow believe it would be nice if we could mock store and its serializer so you could test even controller actions that are calling save on models.

Currently I'm playing with moduleForModel and I added

container.register('serializer:application', DS.JSONSerializer);

Now I'm one small step further with this and trying to find a way to properly mock all the things. Now I get You can only unload a record which is not inFlight. error because Ember Data works like so.

But shouldn't FixturesAdapter prevent doing ajax requests or?

Anyhow, if you think this is a blind path for me to pursue, just tell me and will do my testing differently.

@stas
Copy link

stas commented Apr 1, 2014

I understand what you are trying to achieve, please let me suggest you try a different approach.

Right now you are using the FixturesAdapter for both: development and testing and you might be switching to AMS or RESTAdapter in production. Well, this is an issue.

Normally your tests should use as much as possible the same setup as you will be using in production, also because FixturesAdapter will not know how to handle responses from the API, since it will be searching for entries somewhere in App.Model.FIXTURES. So what you should really be doing is using AMS/RESTAdapter in your tests (of course with a relevant serializer).

Regarding the stubbed API, this is easily solved by using one of the already suggested ajax mocking libraries.

To resume:
Create an adapter based on your current environment. Example:

# app/adapters/application.coffee
Adapter = DS.ActiveModelAdapter.extend
  # API End-point namespace
  namespace: 'api/v1'

FixtureAdapter = DS.FixtureAdapter.extend()

if window.ENV.development
  Adapter = FixtureAdapter

`export default Adapter`

Use integration tests if you need to handle data submission by mocking API requests. Example:

# test/acceptance/users_index_test.coffee
App = undefined
module 'Users Page',
  setup: ->
    App = startApp()
    $.mockjax
      url: '/api/v1/users'
      # Could be 'POST', when you are creating a record
      type: 'GET'
      responseText: 
        users: [
          {id: 1, name: 'User #1'}
          {id: 2, name: 'User #2'}
        ]

  teardown: ->
    $.mockjaxClear();
    Ember.run App, 'destroy'

test 'index renders available users', ->
  visit('/users/index').then ->
    equal $('.users-list li').length, 2

Consider loading fixtures, only in development. An example implementation can be found in our broccoli boilerplate.

@zigomir
Copy link
Contributor Author

zigomir commented Apr 3, 2014

@stas thanks! I'll try mockjax for sure. For development I actually use ActiveModelAdapter. The main point I'll have from this is that unit tests should not exercise anything near request phase at all.

Testing (especially JS code) is still pretty new ground for me and I'm not well educated about good practices yet.

I'm gonna close this issue. Thanks for all your help!

@zigomir zigomir closed this as completed Apr 3, 2014
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

3 participants