Model unit tests silently cache this.subject()

[aprescott]

I see in #154 that it's intentional for this.subject() to be cached, at least in the context of component tests, but I've been struggling with some fairly basic model unit tests against a straight-forward computed property and I consider the memoization behavior to be buggy. Perhaps I'm missing something obvious though.

If the caching behavior is intended even in this case, then at the very least a loud warning/error should probably be given.

• ember-cli 2.13.3.
• ember-qunit 2.1.3

---

To give a somewhat contrived example, suppose you have a simple model with a DS.attr() and a computed property:

// app/models/user.js

export default DS.Model.extend({
  role: DS.attr(),

  isAdmin: Ember.computed.equals("role", "admin")
});

Then we'd like to test when isAdmin comes back true, for a few possible values of role (which could be any string). A concise way to do that is like so:

import { moduleForModel, test } from 'ember-qunit';

moduleForModel('user', 'Unit | Model | user', {
  needs: []
});

test("isAdmin depends on the role attribute", function(assert) {
  assert.expect(3);

  [
    ["admin", true],
    ["Admin", false],
    ["superadmin", false]
  ].forEach(([role, expectedValue]) => {
    const model = this.subject({role: role});
    assert.equal(model.get("isAdmin"), expectedValue);
  });
});

This doesn't work, because this.subject({role: role}) is cached with the first set of attributes given, so the first test will succeed, but the second test (with "Admin") will fail because isAdmin for that instance is still true.

This seems like a bug to me. If this truly is the desired behavior, then I think a warning or error is needed to alert that subject() is being called twice.

I would definitely prefer to be able to call this.subject() multiple times, for at least 2 reasons.

1. Breaking out multiple test()s for varying just a single value is quite redundant.
2. Because the role property uses DS.attr(), calling model.set("role", role) will generate Ember run-loop errors, requiring you to wrap set() calls in Ember.run(() => {}), which gets verbose.

[trentmwillis]

I'm unsure if the caching behavior should be kept or not (and I don't have context on the original decision), but I agree that at the very least we need to warn users that the result is memoized.

[rwjblue]

Caching behavior would be completely deprecated (and not added to the new system) once the outstanding RFC's are implemented.

[rwjblue]

New API landed in #286, closing this for now (as we are unlikely to change the moduleFor* behavior on this since it would be too breaking for folks relying on the feature). The new API (from emberjs/rfcs#232) does not include this.subject at all...