remove initializer, get config from environment #53
Conversation
| @@ -9,7 +9,7 @@ moduleFor( | |||
| 'Unit | Mixin | component query manager', { | |||
| needs: ['service:apollo'], | |||
| beforeEach() { | |||
| // needed to set up config since initializers don't run here | |||
| // needed to set up config since environment won't be initialized in this type of test | |||
There was a problem hiding this comment.
If you add config:environment to the needs array for your unit tests, you should be able to get rid of these beforeEach blocks.
I believe the plan is for the isolated container that unit tests use to eventually go away completely, so at that point the needs will be unnecessary too.
bgentry
force-pushed
the
remove-initializer
branch
from
5b372d0
to
9e26cb1
Compare
|
@dfreeman d'oh, slow afternoon, thanks :) |
tests/unit/services/apollo-test.js
Outdated
| // Need this here because apollo requires a uri, but our initializer doesn't | ||
| // run in unit tests. | ||
| // Need this here because apollo requires a uri, but our config environment | ||
| // isn't set up in unit tests. | ||
| options: { | ||
| apiURL: 'https://this-should-be-set-from-environment.example', | ||
| }, |
There was a problem hiding this comment.
@dfreeman removing the beforeEach worked for the mixin unit tests, but I wasn't able to get rid of options overriding in this service test. If I remove it, these tests fail because getOwner inside the apollo service returns undefined. I must be missing something on this part...
There was a problem hiding this comment.
It looks like your OverriddenService instance doesn't have an owner because you're create()-ing it manually.
Instead, you could do something like:
let customDataIdFromObject = o => o.name;
this.register('service:overridden-apollo', ApolloService.extend({
clientOptions: // ...
}));
let service = this.container.lookup('service:overridden-apollo');
// ...There was a problem hiding this comment.
you mean just like any other service test? 🤦♂️
Thanks as always!
bgentry
force-pushed
the
remove-initializer
branch
from
9e26cb1
to
fdfa804
Compare
|
The downside to this is that all of my unit tests of things like routes now require a |
|
Maybe the service needs to have some sort of default options set that it has to ensure that it still works in tests when |
|
Opened #53 for the follow-up item. |
Attempts to resolve #52.
Maybe @viniciussbs and @dfreeman can advise as to whether I've done this correctly, and in particular whether I should change something to avoid having to inject the config into all unit tests.