Added tests for Ember Data Model Fragment factory support in unit tests

[patocallaghan]

As discussed in #184 I've added unit tests to test for support of https://github.com/lytics/ember-data-model-fragments. It's not comprehensive but it's a start. Acceptance tests can follow.

I've added an employee model which from a model-fragment point of view looks like:

Employee
* name (fragment)
* titles (array)
* departmentEmployments (fragmentArray)

  DepartmentEmployment
  * department (fragment)

    Department
    * addresses (fragmentArray)

As model-fragment supports polymorphic fragmentArrays I added billing-address and mailing-address as well but was unsure how to construct the list using a factory to test it.

@danielspaniel anything else you think we should test for?

[danielspaniel]

This is a great start, so thanks for helping me understand how the model-fragments work.
The test that fails brings up an interesting issue because if you do:
name: {} in a factory, it usually will fill in that default for that relationship as you say, but the model fragment is a fragment because it's not a relationship, so there will have to be special case for that one,
just as I was thinking that there would be for the serializer things.
So for that attributed ( name ) .. if you look at the attribute type, it is a => type: "-mf-fragment$name"
so I will try and do special case logic for those attributes that are "-mf-fragment" types if you know what I mean

[danielspaniel]

hooray .. that worked, so that test now passes

[danielspaniel]

Boy, if you could now do some acceptance tests I will be your best friend for a whole day. I can't wait to get this thing finished ( for some reason ) .. not sure why though.

[danielspaniel]

I just pushed that change to model-definition so you can see that

[patocallaghan]

Great stuff @danielspaniel, I'll add the acceptance tests. Will I keep them here or add them to a separate PR? I probably won't get to it for a day or two though.

[danielspaniel]

Putting them here is better I reckon @patocallaghan ... so we can wrap it up in one go.

[patocallaghan]

@danielspaniel I've added a single acceptance so far (see 2nd commit) and it's showing a failure due to this being undefined in the model fragment when using mockFind. I don't think I've set it up incorrect as it's somewhat similar to the unit tests.

Also did you push your model-definition fix here or upstream to master? I may have accidentally overwritten over it if it was to this branch.

[patocallaghan]

@danielspaniel I've just added this other failing unit test I've been getting. I'm not sure if it's related to model-fragments or not but if not I'll pull it out to a separate issue if you like.

Basically when doing a mockUpdate and then doing a save() the attributes are still dirty (is that expected?). Then test fails with the following error:

Promise rejected after update the employee: Assertion Failed: You can only unload a record which is not inFlight

which I'm presuming happens because the test is trying to clean up but the record is dirty.

[danielspaniel]

Ok @patocallaghan .. I "fixed" up that test so it will not get the inflight errors .. but I moved that mockUpdate tests to the shared-factory-guy-test-helper-tests.js file
so it would be run when the application serializer was either JSONAPI, ActiveModel or REST, and sure enough, it failed on the JSONAPI serializer test.

The test in that file is here => test('with model that has model fragment as the updated field' ... ( line 1450 )

The error I am getting is:
: The JSONAPISerializer is not suitable for model fragments, please use JSONSerializer

I read the docs in model fragments ( there was issue for this ) .. and they suggest setting a serializer for you fragments? I think. Not sure I quite get it. Any ideas?

I got tests to pass with this change in fixture-converter.js ( line 31 )

   getTransformValueFunction(type) {
    if (!this.transformKeys || type.match('-mf')) {
      return this.noTransformFn;
    }

but do people ever want to custom serialize their values? I assumed they would want to value to stay as JSON and not really mess with it, but that might be naive.

Anyway .. this is the last sticking point I think before this can be pushed out..

Any ideas?

[patocallaghan]

@danielspaniel that's great you figured it out. Just have some questions if you don't mind :)

• I don't see of any of the changes you speak of? Have you pushed those changes here or are they on another branch?
• How did you resolve the inflight error? I get that myself in the app I work on and thankfully was able to reproduce it here.
• When using any of the mockCreate, mockUpdate or mockDelete helpers do you always need to have the application serializer for them?

Re: model fragments + JSON API, unfortunately I don't know much about it but from the docs it seems like it definitely is not allowed. It seems by default model fragments bypasses the application serializer (as it could be JSONAPI) and always registers its own based on the JSONSerializer. These tests outline the behavior https://github.com/lytics/ember-data-model-fragments/pull/148/files#diff-893e7c88185e7eb321a8bb882b55258fR44. It looks by default fragments should have a serializer of JSONSerializer.

[danielspaniel]

Looks like I have been pushing to my own branch or your code in factory guy repo .. oops.
Can you check out the branch called : patocallaghan-patoc/add_tests in my repo.
That is where you'll find the changes.
You will see answer to inflight when you see the test on line 1450 of that file ( pretty simple )

I will try and fix that JSONAPI thing .. for the tests .. it's a royal pain .. but that was helpful to see the tests you pointed too ..

[danielspaniel]

Actually .. hold on.. I see the probable issue in the JSONAPI thing.
I have to redo how I am testing because I am forcing JSONAPI serializer on everyone in the tests ( when I test JSONAPI adapter )
Ooops, I knew that would bite me one day.. and that day has come ..

[danielspaniel]

I fixed it .. so I will merge that ..and then maybe add another test if you like ?

What did you mean with this question:
When using any of the mockCreate, mockUpdate or mockDelete helpers do you always need to have the application serializer for them?

me not understand.

[danielspaniel]

I release a new version 2.5 that has all our work.. so thanks a ton for helping on this.
I figured .. what the heck, just release it and then we can patch if people find issues.
If you can think of other fun tests to add, please do, cause I was being a bit preemptive in releasing that version.

[patocallaghan]

Thanks @danielspaniel this is great!

re: my question:

When using any of the mockCreate, mockUpdate or mockDelete helpers do you always need to have the application serializer for them?

You mention in #188 (comment) above that you moved my original test so that it would have an application serializer. I was just wondering was that something I needed to specifically include in my unit tests when using mockUpdate etc or should it just work out of the box.

[danielspaniel]

Oh .. right ..good question.
For that mockUpdate test .. I like to have that run through all the adapter/serializers ( JSON API, REST, ActiveModel ) and by putting it in shared adapter tests that test will be run 3 times ( one for each ) .. see what I mean?
I do same for all mock this or that's so I am sure it works with all the adapters/serializers ember data supports.
And once I fixed up my test setup ( to not force the application serializer on ALL models .. dumb ) everything worked as expected with the fragments.
I reckon we could add tests for mockCreate .. but we can wait till someone complains that it does not work.