extend models in acceptance tests

[Leooo]

• Our apps use a common repo, where models are defined together with model mixins that can be plugged in inside each app as suited. For example we would have inside the commons repo:

  • premise.js model
  • premise-services-mixin.js mixin, to be optionally added to premise.js in an app if needed
  • premise-energy-mixin.js mixin, to be optionally added to premise.js in an app if needed

• in the commons repo, we want to run an acceptance test for premise.js model extended with premiseEnergyMixin

• current code below breaks (sorry old test API) when we don't add the delete premiseModel.superclass.modelName; line, when run after any previous acceptance test module where premise model is used:

import { test } from 'qunit';
import moduleForAcceptance from 'dummy/tests/helpers/module-for-acceptance';
import premiseEnergyMixin from 'ember-commons/models/premise-energy-mixin';

moduleForAcceptance('Acceptance | intellinav links', {
  beforeEach() {
    let owner = this.application.__container__.owner,
      premiseFactory = owner.factoryFor('model:premise');
      let extendedModel = premiseFactory.class.extend(premiseEnergyMixin);

    owner.unregister('model:premise');
    owner.register('model:premise', extendedModel);
    let store = this.application.__container__.lookup('service:store'),
      premiseModel = store.modelFor('premise');

    //when any other acceptance test module runs, the `modelName` is set for premise model
    //this screws up the logic inside ED findPossibleInverses as it will look for inverses based on the modelName that both
    //premiseModel and premiseModel.superclass share, so will send an exception "multiple possible inverse relationships of type"
    //as finding possible inverses. We would need to do this extend before the app is started, but dont know how to do with acceptance tests
   
    delete premiseModel.superclass.modelName;

This breaks because of this line:

data/addon/-private/system/model/model.js

Line 49 in 87be9b7

if (type.superclass) {

I understand, as per #5000 (comment), that we should normally try to "ensure that classes are correct prior to application instantiation".

But how can we do that for acceptance tests? I wouldn't know how to extend the model above without access to application.__container__, that comes from startApp ?

This doesn't seem to be an easy problem to solve (though correspond to real world cases for modularized Ember applications).

Would this be an accepted fix to add a PR to ensure that in

data/addon/-private/system/model/model.js

Line 49 in 87be9b7

if (type.superclass) {

we don't double-count identical relationships?

Thanks

@runspired @rwjblue

[runspired]

This is actually more related to #4886 and #4966 with some relation to a bugfix that has already landed in #5406 which should only affect canary and beta Ember users (unsure what version you are using).

[runspired]

@Leooo circling back on this:

1. why are you extending in the beforeEach of an acceptance test? That goes against the concept of acceptance tests...

2. if this is an attempt at an integration test for the mixin, then using a simpler setup like the ones we have in our own tests works quite well :) Example

[runspired]

@Leooo To explain the underlying issue, we add modelName as a static property of the underlying class. What you've observed here is #5406 wherein recent versions of Ember which have true inheritance cause the modelName to be inherited. When we see a modelName is already present, we don't overwrite it (part of our support for polymorphism); however, we needed to upgrade our check to use hasOwnProperty due to the change in inheritance, which we will backport (tracking in #5577).

[Leooo]

Thanks for giving some background it may help

…

On Tue, 21 Aug 2018, 7:34 pm Chris Thoburn, ***@***.***> wrote: @Leooo <https://github.com/Leooo> To explain the underlying issue, we add modelName as a static property of the underlying class. What you've observed here is #5406 <#5406> wherein recent versions of Ember which have true inheritance cause the modelName to be inherited. When we see a modelName is already present, we don't overwrite it (part of our support for polymorphism); however, we needed to upgrade our check to use hasOwnProperty due to the change in inheritance, which we will backport (tracking in #5577 <#5577>). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5451 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACT4mO0LdZsb2AIYZ0Qj3ouZKKG3hBVyks5uTFKogaJpZM4TjHR3> .