-
-
Notifications
You must be signed in to change notification settings - Fork 257
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
Support component integration tests #38
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import Ember from 'ember'; | ||
import isolatedContainer from './isolated-container'; | ||
import { isolatedRegistry } from './isolated-container'; | ||
import { getContext, setContext } from './test-context'; | ||
import { Klass } from 'klassy'; | ||
import { getResolver } from './test-resolver'; | ||
|
@@ -133,6 +133,7 @@ export default Klass.extend({ | |
|
||
setContext({ | ||
container: this.container, | ||
registry: this.registry, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exposing the registry to the test context lets people move to the non-deprecated registration APIs. This starts giving us a path away from the hacks that are currently used to avoid deprecations. |
||
factory: factory, | ||
dispatcher: null | ||
}); | ||
|
@@ -213,7 +214,9 @@ export default Klass.extend({ | |
|
||
|
||
_setupIsolatedContainer: function() { | ||
this.container = isolatedContainer(this.needs); | ||
var isolated = isolatedRegistry(this.needs); | ||
this.container = isolated.container; | ||
this.registry = isolated.registry; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes the isolated case consistent with the integrated case, which already provides |
||
}, | ||
|
||
_setupIntegratedContainer: function() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wary of changing the return value of
isolatedContainer
here, especially given the likelihood of the container/registry reform work proceeding very soon (emberjs/rfcs#46). In other words, I don't want developers who are usingisolatedContainer
to need to refactor twice in a short time. Maybe the question should be whetherisolatedContainer
is really used outside of ember-mocha and ember-qunit?@ef4 @rwjblue thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assumed that only ember-mocha and ember-qunit would be affected, but I could be wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can just implement
isolatedRegistry
instead.isolatedContainer
can keep the same public API, but get reimplemented in terms ofisolatedRegistry
.(While also maintaining compatibility for old Embers that don't have a registry.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! Thanks in advance @ef4 👍