Integration tests breaks for components that inject notifications

[Edralo]

Hello !

After upgrading a project from ember 2.11 to 2.13, I noticed that all the integration tests of the components that injects ember-cli-notifications are breaking : Assertion Failed: Attempting to inject an unknown injection: 'service:notification-messages'.

In these components, I inject your addon "individually" via :
notifications: Ember.inject.service('notification-messages'),.
I also tried to inject via an initializer afterward, and tests are ok with that method. The problem is I don't need to inject it everywhere, so I shouldn't use an initializer in my project..

I made a small repo that replicate this problem : https://github.com/Edralo/notification-test-fail

I haven't looked much to find where the problem is, but maybe it's linked to https://github.com/emberjs/rfcs/blob/master/text/0150-factory-for.md since the problem appeared with ember 2.12.

That's pretty much all i know about the issue for now, sorry

[Matt-Jensen]

@Edralo This issue may not relate to this project but an issue with qunit: ember-cli/ember-cli#6945. Did you have this issue after migrating ember-cli?

[Matt-Jensen]

My last comment may not be helpful to you, I was able to resolve this issue partially by correctly requiring the notification-messages by its' correct resolver name in my unit tests like so:

moduleFor('route:account', 'Unit | Route | account', {
  needs: ['service:notification-messages-service']
});

Note that the services actual name in the container is: notification-messages-service as that is its' original filename. I'd be curious to know if these errors in our integration tests are some kind of side effect? I'll update once I get my tests passing.

[Matt-Jensen]

I think I was on the right track. I'm not sure how we are able to inject notification-messages in our development environment but our tests are wanting to register: notification-messages-service which yields the following error:

TypeError: Attempting to register an unknown factory: 'service:notification-messages'

Updating your needs to my above code snippet will then generate the following error:

Assertion Failed: Attempting to inject an unknown injection: 'service:notification-messages'

You can correct this error, as a quick fix, by registering 'service:notification-messages-service' as service:notification-messages like so:

moduleFor('route:tmp-fix', 'Unit | Route | tmp-fix', {
  needs: ['service:notification-messages-service'],
  beforeEach() {
    const notifications = this.container.lookupFactory('service:notification-messages-service');
    this.register('service:notification-messages', notifications);
  }
});

However I was able to remove all these errors by renaming app/services/notification-messages-service.js to app/services/notification-messages.js which wont work for you outside your dev environment.

[Matt-Jensen]

Fresh [email protected] app example documenting issue: https://github.com/Matt-Jensen/ember-cli-notifications-test. Pull and run tests to reproduce above errors.

[Edralo]

Thanks for the unit tests !

Unfortunately, I'm still stuck about the integrations tests. Maybe I should investigate about ember-qunit and ember-cli-qunit (since we're using the latter)

[Matt-Jensen]

I was able to clear up the integration test issues with the same approach:

moduleForComponent('my-component', 'Integration | Component | my component', {
  integration: true,
  beforeEach() {
    const notifications = this.container.lookupFactory('service:notification-messages-service');
    this.register('service:notification-messages', notifications);
  }
});

[Edralo]

Sorry for the late comment, we finally fixed it.

We still had injections issue with your solution but it was easily fixable. We also needed to import the service via its installed directory inside the project and used it directly inside this.register :

import NotificationsService from 'ember-cli-notifications/services/notification-messages-service';

moduleForComponent('my-component', 'Integration | Component | my component', {
  integration: true,
  beforeEach() {
    this.register('service:notification-messages', NotificationsService);
    this.inject.service('notification-messages', { as: 'notifications' });
  }
});

Thanks a lot for the help @Matt-Jensen

[Matt-Jensen]

@Edralo I don't believe you should close this issue. Our solutions are just hacks to get the test suite to pass. We should close this once GH-171 is merged.

[Edralo]

Oops, sure

[ynnoj]

I'll merge #171 and roll it in to the 5.0.0 release as its a breaking change 🛠

[cah-brian-gantzler]

While this will work, its only a half measure and breaking convention over configuration. You will note on the documentation page you still have to import notification-messages-service to extend which is not what would be expected. The service really should be renamed everywhere for a complete fix.

In the meantime in the consuming application just creating a new service under whatever name you wish (notification-messages in this case), importing and only extending notification-messages-service will work as well without this pull request.

[ynnoj]

@cah-briangantzler I'll be renaming the service hence the breaking change and a required major release with 5.0.0.

[mansona]

This has changed with the release of #251 and shouldn't be an issue anymore 👍 let me know if you have any questions or are still having problems