Add support for integration: 'legacy'

[hjdivad]

This semantics of the integration flag have changed in a
backwards-incompatible way since v0.4.3. This corresponds to an ember upgrade
(using ember-cli) between v1.12 and v1.13.

In order to ease this upgrade path for ember-cli users, integration: 'legacy'
will set up a component test with v0.4.3 semantics. Of course, newer tests
would still be encouraged to be written with integration: true.

[hjdivad]

@rwjblue here's a PR for that integration: 'legacy' flag we talked about. I'm still not 100% on the internals of ember-test-helpers so please let me know if there's stuff in this PR that looks wonky that I should examine more closely.

Thanks.

[hjdivad]

Now the tests even pass!

As an aside the test failures indicate that this feature will not work with ember 2.0, which seems fine to me.

thoughts?

[ef4]

What was the semantic change for the integration flag?

[hjdivad]

@ef4 There are two things that impacted tests I had written.

1. render now renders into an outer context rather than rendering the component directly. Among other things, this means that render requires a template. This isn't really that big of a deal as it's easy to make this change to tests.
2. subject is now disallowed. This means primarily that there is no sensible way of getting a reference to the component. That makes a lot of sense with the current approach, in which interaction with the component in a component test is done primarily via the DOM. But previously, where integration: true more or less just meant "you don't have to write needs entries for everything" it would often make sense to set the flag when writing component unit tests in which you would make assertions about component properties.

[ef4]

But those semantics are literally the original definition of integration:true. There was no earlier integration:true mode with different semantics.

Prior to the introduction of that flag, all tests were unit tests, and they used render and subject as you describe.

[hjdivad]

@ef4 whatever the intended semantics, on version 0.4.3 with ember 1.12, you could use subject and render as I describe with integration: true as a means of writing unit tests without needing to specify needs.

ie on version 0.4.3 you could write tests like

moduleForComponent('my-component', {
  integration: true, // no need to specify `needs` now
});

test("my-component has this flag set after you click a thing", function () {
  const component = this.subject();

  this.render();

  Ember.run(function () {
    component.$('button').click();
  });

  equal(component.get('flagB'), true, 'flagB is now set');
});

[rwjblue]

@ef4 - integration: true for moduleForComponent definitely changed meaning when the current (this.render with a template string) style was added. Prior to those changes, integration: true simply gave you a container with no resolver restrictions (so you didn't have to specify needs).

[rwjblue]

Specifically, the chain of events (as far as I can track down) is:

• Adding standalone integration test support #21 added TestModuleForIntegration, which was not exposed by ember-qunit (because we were bikeshedding about the exact final user-land API).
• Move integration container logic to TestModule #24 moves integration: true to the top level TestModule
• [email protected] and was released in [email protected]
• Support component integration tests #38 removed TestModuleForIntegration and moved that logic into TestModuleForComponent
• [email protected] was released in [email protected]

So from 0.3.6 through 0.4.3 you could use integration: true as a way to avoid specifying needs.

---

tldr; I really mucked things up 😞

[rwjblue]

How does this differ from https://github.com/switchfly/ember-test-helpers/blob/master/lib/ember-test-helpers/test-module-for-component.js#L72-L120? It seems that these are essentially duplicated...

[rwjblue]

https://github.com/switchfly/ember-test-helpers/blob/master/lib/ember-test-helpers/test-module.js#L129-L135 determines if the container that is being used is isolated or has the full resolver (note how we clobber the underlying resolver here after manually resolving all of the needs specified items).

[rwjblue]

Sorry for beating around the bush so much here. I think that legacy mode should only need to do the following:

• Change the conditional https://github.com/switchfly/ember-test-helpers/blob/master/lib/ember-test-helpers/test-module.js#L129-L135 to just use _setupIntegratedContainer for legacy mode.
• Use setupComponentUnitTest when in legacy mode.

The combination of those two changes, essentially sets us back to the behavior of ember-test-helpers 0.3.6 - 0.4.3.

[rwjblue]

@hjdivad -- Let me know if you have any questions about my last few comments here...

[rwjblue]

Closing for now, happy to reopen if you want to pick this back up.

[hjdivad]

@rwjblue hey sorry it took so long to find time to try this out again. I've redone the branch with the changes you mentioned and our test suite passes. The only other change I had to make was this one: https://github.com/hjdivad/ember-test-helpers/blob/integration-legacy/lib/ember-test-helpers/test-module-for-component.js#L202-L204 to prevent subject from getting indirectly stomped.

Should be good to reopen or I can open a new PR if you'd prefer.

[rwjblue]

Weird, apparently I can only reopen if there hasn't been a force push to the branch the closed PR was submitted from (which seems kinda dumb/weird).

[rwjblue]

Submitted #121 to replace

[hjdivad]

@rwjblue that is a strange restriction.

[rwjblue]

D'OH!! The comment above was about the GH restriction of reopening PR's. 🤦