Tests instantiating models with relationships fail on ember-canary

[bendemboski]

Here is an addon demonstrating the issue. It's just a vanilla addon with ember-data installed and a couple of models generated, user which belongsTo organization.

If you run ember try:one ember-canary (as of the filing time of this issue), it will fail because it hits this newly-added assertion:

not ok 12 Chrome 66.0 - Unit | Model | user: it exists
    ---
        actual: >
            null
        stack: >
            Error: Assertion Failed: expected container not to be destroyed
                at new EmberError (http://localhost:7357/assets/vendor.js:18994:25)
                at assert (http://localhost:7357/assets/vendor.js:16297:15)
                at Container.factoryFor (http://localhost:7357/assets/vendor.js:37787:58)
                at Class.factoryFor (http://localhost:7357/assets/vendor.js:59270:33)
                at Class.modelFactoryFor (http://localhost:7357/assets/vendor.js:86413:20)
                at Class._modelFactoryFor (http://localhost:7357/assets/vendor.js:86376:22)
                at Class._modelFor (http://localhost:7357/assets/vendor.js:86367:29)
                at Class.modelFor (http://localhost:7357/assets/vendor.js:86360:17)
                at Function.typeForRelationship (http://localhost:7357/assets/vendor.js:77253:34)
                at Function._findInverseFor (http://localhost:7357/assets/vendor.js:77311:28)
        message: >
            Promise rejected after "it exists": Assertion Failed: expected container not to be destroyed
        Log: |
    ...

This is happening because when the store is unloading the organization record (resulting from tearing down the test context), it looks at its relationships, which ultimately results in a modelFor call to look up the user model at the other end of the relationship, which looks it up in the container, which is already destroyed. It looks like this has been happening for a long time, but is just now failing because of the newly-added assertion. I'm not sure if ember-data's behavior is incorrect, or if the assertion is overly aggressive.

[rwjblue]

I'm not sure if ember-data's behavior is incorrect, or if the assertion is overly aggressive.

Probably a bit of both. 😆

The goal of the assertion is to ensure that a destroyed container (which has released all of its instances and whatnot for GC) does not have additional lookup methods called on it (which would now leak memory because there is no way to run destroy again). Depending on how hard this is to fix in ember-data we could change the upstream assertion into a warning or a private API deprecation slated for 3.5 removal, but either way the underlying issue reported here (doing owner.lookup or owner.factoryFor after destruction) should be addressed.

Thank you for reporting!

[wagenet]

I'm finding this happens in routes where I call controllerFor in a deactivate hook.

[wagenet]

@rwjblue

[wagenet]

I'm also seeing this happening when records are destroyed during teardown and it attempts to access the inverse meta.

[wagenet]

Fixed in emberjs/ember.js#16754

[GreatWizard]

I just reproduce the problem on ember 3.3.1 with a generated model when I add a relationship.

The generated test fails with the same error Error: Assertion Failed: expected container not to be destroyed

EDIT: I rollback on ember 3.1 for now

[ryanto]

I found this issue from Google. I'm running into this in an addon I'm writing tests for.

In my Ember try scenarios I have no container leak when using with Ember 3.1 and Ember Data 3.1.

However, In Ember 3.4 with Ember Data 3.4 this assertion pops up and there is a Container leak and the assertion causes my tests to fail.

The container leak/assertion failure only happens when I try to access Ember Data. For example, the failure happens when testing a route that calls store.findRecord, but the failure won't happen on tests that don't touch code that relies on data.

Right now my travis builds are failing for 3.4 (ember-release + data release) and I'm a little suck on how to get them working.

I'm unsure how to proceed or what I should be doing in this situation.

[runspired]

In my Ember try scenarios I have no container leak when using with Ember 3.1 and Ember Data 3.1.

@ryanto I think the issues you want are #5554 and emberjs/ember.js#16860

[ryanto]

Cool! I'll give those a read.

Also, I was getting hardcore trolled by npm dependencies earlier today. I think what I originally posted was incorrect and the result of bad dependency resolution. My final build was ending up with Ember 3.4 + ED 2.18... which causes that assertion error.

So if anyone comes across this error, double check that you're not using a data 2.x with Ember 3.4!