Add integration tests for CRUD with pretender

[benkonrath]

No description provided.

[benkonrath]

Alright, finally some progress. :)

[dustinfarris]

What does -drf mean?

[benkonrath]

drf is for django rest framework. I'm not exactly sure if this is correct; I'm just following format from the docs and the active model adapter.

http://emberjs.com/guides/models/customizing-adapters/#toc_authoring-adapters
https://github.com/emberjs/data/blob/master/packages/activemodel-adapter/lib/system/active_model_adapter.js#L104

I guess '-django-rest' or '-django' could also work. Or does it need to be 'DS/djangoREST' for some reason?

[dustinfarris]

I don't think the preceding hyphen is necessary. This is how I did it in the current v0.4.0 version: https://github.com/dustinfarris/ember-django-adapter/blob/master/app/adapters/django.js#L4 which corresponds to the django serializer (identified by filename) here: https://github.com/dustinfarris/ember-django-adapter/tree/master/app/serializers

Right now in the version-1.0 branch we just have an "application" adapter and an "application" serializer which means this line isn't necessary because Ember will choose those by default anyway. This may not be the right way to go, though, and perhaps we should continue doing it the way I did it in master, which would be to move app/serializers/application.js to app/serializers/drf.js and then change this line to defaultSerializer: 'drf'

[dustinfarris]

It looks like the preceding hyphen is used because ember data specifically registered it with that name:

https://github.com/emberjs/data/blob/master/packages/activemodel-adapter/lib/setup-container.js#L12-L13

My guess is that it is intended to prevent name clashing.

[benkonrath]

Ok, so should I remove the defaultSerializer line or just revert this change back to the original value in the version-1.0 branch?

[dustinfarris]

Great start here! Pretender looks very promising.

[dustinfarris]

Just remove for now. We can address how we want to make the serializer available in a different PR.

[benkonrath]

@dustinfarris I'm still poking away at this but I'll let you know when I think it's ready for a review ... probably after your unittest PR is merged.

[benkonrath]

Maybe the new version of Ember is causing this failure. I'll investigate tomorrow.

[benkonrath]

tests/dummy/app/adapters/application.js is in the pull request. I think the issue is that your tests use the injected adapter from moduleFor('adapter:application', ... while my tests rely on the adapter being set in the dummy app. I'm not sure how to proceed but I can play around with it again in a couple of days.

[dustinfarris]

Sorry, I was reading the diff wrong. So you have created dummy/app/adapters/application.js then? The addon should make that unnecessary (e.g. users should never have to create adapters/application.js or serializers/application.js unless they want to override/extend something).

[dustinfarris]

@benkonrath any luck with this?

[benkonrath]

When I remove dummy/app/adapters/application.js the calls to pretender aren't made. I tried a bunch of things to get this going without the dummy adapter but nothing worked.

My latest commit adds the /test-api/ namespace to the adapter to fix one of the test failures. I know this isn't exactly correct but I'm not sure what else I can do. Unfortunately pretender won't work with the test-host set on the dummy adapter. If we want to use this integration test strategy, we'd have to remove the test for the unit test for the host setting.

[benkonrath]

I forgot to mention: The nice thing about this latest version is that I no longer add the adapter to the dummy app. So I think I'm on the right track now with the host issue with Pretender.

[dustinfarris]

Awesome! Is there anything else outstanding before this can be merged?

[benkonrath]

I just created an issue #33 so I don't forget about the issue with setting the empty host in the integration tests. I think it's good to go.