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

Tests failing after upgrading to 1.0.0-beta16 #2924

Closed
pmdarrow opened this issue Mar 23, 2015 · 17 comments
Closed

Tests failing after upgrading to 1.0.0-beta16 #2924

pmdarrow opened this issue Mar 23, 2015 · 17 comments

Comments

@pmdarrow
Copy link

After upgrading to 1.0.0-beta16, all of my models tests are failing on this line: 31583db#diff-1636bea36bbe333a834ea11fbfbde48dR1876. It appears this._containerCache is undefined and therefore this._containerCache[key] throws an exception. Each model test seems to pass fine when run in isolation, but when I test an entire module or run the whole suite, I get exception errors for each test.

@bmac
Copy link
Member

bmac commented Mar 23, 2015

hmm. I wonder if this line is causing the issue https://github.com/emberjs/data/blob/master/packages/ember-data/lib/system/store.js#L1929. It should only get run when the store is being destroyed. Could your tests be holding on to and old store?

@pmdarrow
Copy link
Author

After playing around in the debugger, it seems like you might be right—despite this._containerCache = Ember.create(null); running during the execution of qunit.module.setup() and delete this._containerCache; running during qunit.module.teardown(), this._containerCache is still undefined in retrieveManagedInstance.

Is the problem with ember-test-helpers? (I should have noted I'm using Ember-CLI 0.2.0 for running my tests).

@bmac
Copy link
Member

bmac commented Mar 23, 2015

It could be related ember-test-helpers. I know the subject() helper was know to reuse an old container in the past. I'm not sure if its still an issue. https://github.com/rwjblue/ember-qunit/issues/81

@jrjohnson
Copy link
Contributor

I'm seeing a different error

 Cannot read property 'adapter:user' of undefined

But it seems to also be related to the subject() test helper. I worked around it by creating the model using createRecord when I needed it instead of calling this.subject()

@igorT
Copy link
Member

igorT commented Mar 23, 2015

ping @rwjblue

@pmdarrow
Copy link
Author

That's the same issue I'm experiencing @jrjohnson, I just tracked down the exact line throwing the exception.

@bmac
Copy link
Member

bmac commented Mar 24, 2015

@pmdarrow Do you mind sharing what line was throwing the exception?

@rwjblue
Copy link
Member

rwjblue commented Mar 24, 2015

Should be fixed once emberjs/ember-test-helpers#28 is merged and released in a new ember-qunit version.

@pmdarrow
Copy link
Author

@bmac the line I linked in my first post is where the exception is being thrown. Looks like it's been fixed elsewhere so you can probably close this.

@bmac bmac added this to the 1.0.0-beta.16.1 milestone Mar 24, 2015
@rwjblue
Copy link
Member

rwjblue commented Mar 24, 2015

ember-qunit@0.3.0 is release fixing this...

@rwjblue rwjblue closed this as completed Mar 24, 2015
subhashb pushed a commit to subhashb/ember-cli-qunit that referenced this issue Mar 29, 2015
`ember-qunit@0.3.0` contains fix for
emberjs/data#2924, which resolves issues with `this.subject()`
@novaugust
Copy link

I'm seeing this same error trying to upgrade ghost to 1.11 (TryGhost/Ghost#5094), but we're on mocha rather than qunit. Can anyone point me towards a solution for that?

@bmac
Copy link
Member

bmac commented Apr 3, 2015

Sounds like the same issue. @dgeb which version of ember-mocha uses v0.4.1 of ember-test-helpers?

@rwjblue
Copy link
Member

rwjblue commented Apr 3, 2015

@novaugust - Update to 0.6.0 of ember-mocha in your bower.json (no changes needed in ember-cli-mocha other than the blueprint so you can just get started with ember-mocha@0.6.0).

@novaugust
Copy link

Thanks gents, fixed now

@cibernox
Copy link
Contributor

I'm still facing this bug using the latest and greatest versions of everything:

ember-data: beta.18
ember: 1.12
ember-qunit: 0.4.0
ember-cli-qunit: 0.3.14

On the Ember.run(application, 'destroy'); of the teardown of some acceptance tests the _containerCache is undefined.

@cibernox
Copy link
Contributor

I've found the change in my code that triggered this:

I changed the cleaning of the state of a /new route from the willTransition action to the deactivate hook:

This works:

  actions: {
    willTransition() {
      const spellingTest = this.modelFor(this.routeName);
      spellingTest.get('trays').filterBy('isNew').forEach(function (tray) {
        if (tray.get('isNew')) {
          tray.get('tasks').filterBy('isNew').forEach(t => t.destroyRecord());
          tray.destroyRecord();
        }
      });
      return true;
    },
    // ...
  }

This fails:

  deactivate() {
    const spellingTest = this.modelFor(this.routeName);
    spellingTest.get('trays').filterBy('isNew').forEach(function (tray) {
      if (tray.get('isNew')) {
        tray.get('tasks').filterBy('isNew').forEach(t => t.destroyRecord());
        tray.destroyRecord();
      }
    });
  }

Then I realized that I don't need to call destroyRecord because the record never was saved, so a deleteRecord is enough. I changed it and now it works:

  deactivate() {
    const spellingTest = this.modelFor(this.routeName);
    spellingTest.get('trays').filterBy('isNew').forEach(function (tray) {
      if (tray.get('isNew')) {
        tray.get('tasks').filterBy('isNew').forEach(t => t.deleteRecord());
        tray.deleteRecord();
      }
    });
  }

I'd say that for some reason the destroyRecord schedules some kind of call to the adapter to perform a destroy action, and when Ember.run consumes drains the queues, the teardown of the container happens before that scheduled action, and when its turn comes the cache it's not there anymore.

@olivierlesnicki
Copy link

Seeing same issues with ember-qunit 0.3.9

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

8 participants