mockFindAll fails in route test on Ember 2.11

[mblayman]

Hey, first, thanks for this project! Coming from a Django world, I much prefer using factories over dealing with fixtures.

I upgraded my app to Ember 2.11 and I hit a snag with one of my tests using mockFindAll (running [email protected]). The test is a little weird because I'm trying to test a model hook in a route so I'm using mockFindAll in a non-acceptance test location. The approach I took was to use mockSetup and mockTeardown. Here's the snippet of test code:

import Ember from 'ember';
import { mockFindAll, mockSetup, mockTeardown } from 'ember-data-factory-guy';
import { moduleFor, test } from 'ember-qunit';

moduleFor('route:dashboard/students/new', 'Unit | Route | dashboard/students/new', {
  beforeEach() {
    mockSetup();
  },

  afterEach() {
    mockTeardown();
  },

  needs: [
    'model:school',
    'model:semester',
    'model:student',
    'validator:number',
    'validator:presence'
  ]
});

test('it has a student for its model', function(assert) {
  mockFindAll('semester', 2);

  let route = this.subject();
  Ember.run(function() {
    let promise = route.model();
    promise.then(function(model) {
      assert.equal(model.student.get('constructor.modelName'), 'student');
    });
  });
});

This test used to work. Now it seems to blow up at the mockFindAll line at the start of the test. Here's the traceback I got:

TypeError: Cannot read property 'serializer' of null
    at ContainerInstanceCache.get (http://localhost:7357/assets/vendor.js:94844:37)
    at Class.serializerFor (http://localhost:7357/assets/vendor.js:94634:34)
    at _default.fixtureBuilder (http://localhost:7357/assets/vendor.js:79477:37)
    at FactoryGuy.fixtureBuilder (http://localhost:7357/assets/vendor.js:80807:43)
    at MockFindAllRequest._default (http://localhost:7357/assets/vendor.js:82417:71)
    at MockFindAllRequest.MockGetRequest (http://localhost:7357/assets/vendor.js:82156:82)
    at new MockFindAllRequest (http://localhost:7357/assets/vendor.js:82074:86)
    at mockFindAll (http://localhost:7357/assets/vendor.js:81656:16)
    at Object.<anonymous> (http://localhost:7357/assets/tests.js:1060:42)
    at runTest (http://localhost:7357/assets/test-support.js:3470:30)

I'm not super experienced at deep debugging of Ember tests (dealing with a giant vendor file makes me want to tear my hair out), but I'm happy to provide any additional information that might be helpful.

[danielspaniel]

can you try this:

 mockSetup(this.container);

[mblayman]

Thanks for the suggestion. Sadly, it still fails.

[danielspaniel]

Darn, ok .. that was first try. Does this fail with ember-data 2.10 ? or just 2.11 ? Actually ... can you tell me what version of both you have ember/ember-data
Tomorrow I will put your test into factory guy and try to see if I can get a failure.
But it would help me if you experimented with different versions to see if it is version specific ..

[danielspaniel]

But first try adding a few things so your test looks like:

test('it has a student for its model', function(assert) {
  let done = assert.async();
  mockFindAll('semester', 2);

  let route = this.subject();
  Ember.run(function() {
    let promise = route.model();
    promise.then(function(model) {
      // model should be an array of 2 semesters .. right? 
      // so what are you testing exactly?  me confused
      assert.equal(model.student.get('constructor.modelName'), 'student');
      done();
    });
  });
});

[danielspaniel]

what are you trying to test exactly?

what this doing?

model.student.get('constructor.modelName')

if the routes model is returning 2 semesters .. your model would be an array .. no?

[mblayman]

The model is an RSVP hash. The semesters just happen to be one of the things that the hash is returning. Here's the hook:

  model() {
    return Ember.RSVP.hash({
      student: this.store.createRecord('student'),
      semesters: this.store.findAll('semester').then((semesters) => semesters.sortBy('date'))
    });
  },

I reduced the test to this and it still fails. Hopefully, this gets some noise out of the equation.

test('it has a student for its model', function(assert) {
  mockFindAll('semester', 2);
});

Without the mockFindAll call, the test blows up. I guess that makes it incidental setup.

I'll try to test against a version that was previously working and let you know the versions of Ember and Ember Data.

[mblayman]

I tried for the last two hours to get a working test run from some previous commit on my repo. I think the combo of Ember wanting to install stuff globally in npm and npm's sad state of affairs for pinning dependencies in a sane way means that I'm not able to get an answer. Shame on me for not having a CI machine where I could quickly point you at a before and after log.

I guess that means that I can't say with confidence that the problem cropped up in the jump to 2.11 since I don't always run the test suite before I commit.

[danielspaniel]

Ok .. no worries. Here's what you can do

import Ember from 'ember';
import { mockFindAll, mockSetup, mockTeardown, manualSetup } from 'ember-data-factory-guy';
import { moduleFor, test } from 'ember-qunit';

moduleFor('route:profiles', 'Unit | Route | profiles', {
  integration: true, // do this instead of needs, saves alot of headache
 // needs: [
 //   'model:profile',
 //   'transform:blah'   => you forgot this  
 // ]

  beforeEach() {
    manualSetup(this.container); // you were doing this, but just making sure this is clear
    mockSetup();
  },

  afterEach() {
    mockTeardown();
  },
});

I have updated factory guy to now spit out a nice message when you make this same configuration mistake ( forgot to needs the transformer ) .. and it is VERY common.
In unit tests you have to needs everything that is used in the test and that includes transformers.

checkout factory guy v2.11.5 to get the friendly error message that will tell you exactly what transform you need to add or just use integration:true in the test and you are back in business 😄

[danielspaniel]

https://github.com/danielspaniel/ember-data-factory-guy/releases/tag/v2.11.5

[mblayman]

I switched my setup to exactly this:

moduleFor('route:dashboard/students/new', 'Unit | Route | dashboard/students/new', {
  beforeEach() {
    manualSetup(this.container);
    mockSetup();
  },

  afterEach() {
    mockTeardown();
  },

  integration: true
  // needs: [
  //   'model:school',
  //   'model:semester',
  //   'model:student',
  //   'validator:number',
  //   'validator:presence'
  // ]
});

and it is still failing. Now I'm getting a new error before the TypeError that I previously reported. For completeness, I'll put in both tracebacks as I see them from QUnit runner.

Assertion after the final `assert.async` was resolved
Source: 	
    at Assert.ok (http://localhost:7357/assets/test-support.js:1770:11)
    at Class.exception (http://localhost:7357/assets/test-support.js:4788:47)
    at adapterDispatch (http://localhost:7357/assets/vendor.js:49801:13)
    at Object.dispatchError (http://localhost:7357/assets/vendor.js:28168:7)
    at onerrorDefault (http://localhost:7357/assets/vendor.js:41736:19)
    at Object.trigger (http://localhost:7357/assets/vendor.js:68738:11)
    at http://localhost:7357/assets/vendor.js:69638:28
    at invoke (http://localhost:7357/assets/vendor.js:10889:14)
TypeError: Cannot read property 'serializer' of null
Expected: 	
true
Result: 	
false
Source: 	
    at Class.exception (http://localhost:7357/assets/test-support.js:4788:47)
    at adapterDispatch (http://localhost:7357/assets/vendor.js:49801:13)
    at Object.dispatchError (http://localhost:7357/assets/vendor.js:28168:7)
    at onerrorDefault (http://localhost:7357/assets/vendor.js:41736:19)
    at Object.trigger (http://localhost:7357/assets/vendor.js:68738:11)
    at http://localhost:7357/assets/vendor.js:69638:28

I'm guessing that the new error is the assertion that you added to the code. I'm not sure if my test is structured incorrectly so that I can't read the proper message or if there is something odd about adding an assert there.

Thanks for the super fast turnaround!

[danielspaniel]

Ok @mblayman .. I going to have to get my scuba gear and dive into that code of yours. I can screen share with you and see what is going on, or maybe as a start, show the ENTIRE test ( and not just the top part )
and can you show me your models too ( both )

[mblayman]

@danielspaniel thanks for the generous offer of your time. I found the mistake that I made. Earlier you suggested I add let done = assert.async();. I added that, but I mistakenly put the done() call after the promise instead of inside its then chain. Once I moved it, the test started working.

I don't think I understand why adding a done() call helps resolve mockFindAll, but I'm grateful for the help.

[danielspaniel]

No problem mate, I enjoy the scuba so I happy to give that code a look around.
And 💃 hooray .. your first use of async testing in a unit test.
What happens ( and your test was good idea .. I have done similar ) is that mockFindAll does not do anything except wait for store to call for data.
so the test calls for the route to get model data, then you wait and put the test in the then block.
Then after the test you call done()
If you don't use done() to tell the test to wait till YOU say it's done,
the test run and finish and NOT wait for the "then" to finish.

[mblayman]

Awesome. Thanks for the explanation.