Skip to content
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

Unregister/register service not working in beforeEach #203

Closed
erichonkanen opened this issue Feb 6, 2018 · 22 comments

Comments

@erichonkanen
Copy link

commented Feb 6, 2018

Using the latest with new test style and attempting to stub out an existing service with a mock one for my acceptance/other tests doing the following:
ember-cli-qunit@4.2.1
ember-source@3.0.0-beta.3

const setupLocalAcceptanceTest = function(hooks) {
  hooks.beforeEach(function() {
    this.owner.unregister('service:stream');
    this.owner.register('service:stream', STREAM_STUB);
module('Acceptance | test/stream-mixin', (hooks) => {
  setupApplicationTest(hooks);
  setupLocalAcceptanceTest(hooks);

This does not work and in my test the real service is used. Am I missing something or should this work? Also tried injecting the mock service as this.owner.inject('route', 'stream', 'service:stub-stream') which didn't work either. Ideally I can replace it via the registration method.

@rwjblue

This comment has been minimized.

Copy link
Member

commented Feb 11, 2018

Can you provide a demo repo so we can try to help figure this out? Generally speaking, this type of thing is known to work...

@erichonkanen

This comment has been minimized.

Copy link
Author

commented Feb 12, 2018

will get to it this week, started playing with ember twiddle

@rwjblue

This comment has been minimized.

Copy link
Member

commented Feb 16, 2018

I suspect the issue is that something eagerly looks up the service:stream in an instance-initializer. If this is the case, there is no way to register a stub (because the instance-initializers are ran before any user-land hooks in the new testing system).

@erichonkanen

This comment has been minimized.

Copy link
Author

commented Feb 16, 2018

I just checked and I only have 1 instance initializer which is for a mirage workaround... I do have an initializer for the stream service:

export function initialize(application) {
  application.inject('component', 'stream', 'service:stream');
  application.inject('controller', 'stream', 'service:stream');
  application.inject('route', 'stream', 'service:stream');
}

export default {
  name: 'stream',
  initialize,
};

and also it is used right away in the app route:

  model() {
    return this.stream.connect();
  },
@rwjblue

This comment has been minimized.

Copy link
Member

commented Feb 16, 2018

Hmm, an initializer like this seems fine generally speaking. But anything that forces the service:stream to exist eagerly (the main place this can happen is via an instance-initializer) will mean that you cannot stub it. For example, if you have any instance-initializers that look up routes, controllers, or components eagerly you will be unable to stub service:stream.

One example that I ran into earlier this week is that ember-simple-auth adds an instance-initializer that looks up route:application, which would cause exactly the issues you are seeing.

@erichonkanen

This comment has been minimized.

Copy link
Author

commented Feb 16, 2018

interesting, we are using ember-simple-auth.. how could I check if that was the cause? I'll be honest I was able to get around it by doing this less ideal solution:

    // mock the stream service methods -- could not get straight up unregister/
    // register to replace the original for some reason so stubbed the methods
    // instead for now -- revisit later -- 2/10/2018.
    this.owner.lookup('service:stream').setProperties({
      connect() { return RSVP.resolve(); },
      disconnect() { return RSVP.resolve(); },
      getConnection() { return RSVP.resolve(); },
      reconnect() { return RSVP.resolve(); },
      register() {},
      subscribe() { return RSVP.resolve(); },
      unregister() {},
      unsubscribe() { return RSVP.resolve(); },
    });
@erichonkanen

This comment has been minimized.

Copy link
Author

commented Feb 21, 2018

just ran into same problem on same app when adding an integration test helper that mocks out services:

export function setupIntegrationTest(hooks) {
  hooks.beforeEach(function() {
    this.owner.register('service:logger', Service.extend(STUB_BASE.logger));
    this.owner.register('service:store', Service.extend(STUB_BASE.store));
    this.owner.register('service:stream', Service.extend(STUB_BASE.stream));
    this.owner.register('service:user', Service.extend(STUB_BASE.currentUser));
    this.logger = this.owner.lookup('service:logger');
    this.store = this.owner.lookup('service:store');
    this.stream = this.owner.lookup('service:stream');
    this.user = this.owner.lookup('service:user');
module('Integration | Component | to-elsewhere/modals/foo', (hooks) => {
  setupRenderingTest(hooks);
  setupIntegrationTest(hooks);
@tpetrone

This comment has been minimized.

Copy link

commented May 10, 2018

So that would make impossible to stub store service?

@rajneeshaggarwal

This comment has been minimized.

Copy link

commented May 30, 2018

Can you try the following?

test('my test', function(assert) {
    const owner = Ember.getOwner(this);
    // Your code with owner to get the service
    owner.lookup(‘service:stream’)
}); 

@GreatWizard

This comment has been minimized.

Copy link

commented Jun 11, 2018

I run into the same issue on 3.1: My project is an addon without any custom instance-initializer and I'm not able to mock the service:store into my integration tests for a component.

@rajneeshnagarro owner is undefined: Cannot read property 'lookup' of undefined

@chrisvdp

This comment has been minimized.

Copy link

commented Jun 14, 2018

I am having the same issue and I also get Cannot read property 'lookup' of undefined when running the example in @rajneeshnagarro example using Ember 3.0

@Bushjumper

This comment has been minimized.

Copy link

commented Aug 22, 2018

I also ran into this issue when attempting to mock service:store in a unit test.

Using unregister inside Ember.run seems to get around it for now:

  hooks.beforeEach(function() {
    run(() => {
      this.owner.unregister('service:store');
      this.owner.register('service:store', MockStore);

      this.mockstore = this.owner.lookup('service:store');
    });
  });

@tmquinn

This comment has been minimized.

Copy link

commented Sep 26, 2018

So I have run into this issue as well but it's only until @Bushjumper's comment. unregister does run loop stuff, is the intention that we use the runloop here?

Also, do I need to unregister something, in order to register it?

@pete-the-pete

This comment has been minimized.

Copy link

commented Nov 12, 2018

@rwjblue can you explain what you meant by "But anything that forces the service:stream to exist eagerly (the main place this can happen is via an instance-initializer) will mean that you cannot stub it. For example, if you have any instance-initializers that look up routes, controllers, or components eagerly you will be unable to stub service:stream.", especially the first sentence?

Why would forcing something to exist mean it can't be stubbed...or does that mean it needs to be unregistered?

@nicksteffens

This comment has been minimized.

Copy link

commented Apr 3, 2019

Still seeing this issue when mocking the service:router inside integrations tests.

@tschoartschi

This comment has been minimized.

Copy link

commented Apr 4, 2019

We also encountered this issue. Would be nice to use beforeEach because it would make our testing code smaller :-)

@rwjblue

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Still seeing this issue when mocking the service:router inside integrations tests.

Don't do that 😈. Seriously though, why would you want to mock the router?

@rwjblue

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

FWIW, I'm going to close this issue:

  • ember-cli-qunit is deprecated
  • the issue is fairly well known/understood

I'd totally be open to making an issue on emberjs/ember-test-helpers to warn/assert when someone attempts to register something that has already been instantiated. That would at least remove the WAT factor of "why isn't my fake service being used"...

@bartocc

This comment has been minimized.

Copy link

commented Jun 28, 2019

I'd totally be open to making an issue on emberjs/ember-test-helpers to warn/assert when someone attempts to register something that has already been instantiated. That would at least remove the WAT factor of "why isn't my fake service being used"...

@rwjblue I've created emberjs/ember-test-helpers#672 to implement a warn/assert as you suggested

Still seeing this issue when mocking the service:router inside integrations tests.

Don't do that 😈. Seriously though, why would you want to mock the router?

As for ☝️, my use case is quite simple: I want my component to be aware of the router's currentRouteName. I don't really see why this would be a bad practice, but maybe you can enlighten me 🤩

This comment has been minimized.

Copy link
Member

commented Jun 28, 2019 — with Octobox

Just to be clear my point wasn't: don't use the router service, it was: don't mock the router service. Mocking leads to extremely brittle tests and often times to tests that pass when "reality is broken". The router service is perfectly capable of being used in these types of test, it can be used directly.

@bartocc

This comment has been minimized.

Copy link

commented Jul 2, 2019

Since discussing the mocking of the router service is a bit off topic here, I've opened https://discuss.emberjs.com/t/to-stub-or-not-to-stub-the-router-service-in-integration-tests/16742 for the discussion to continue

@NullVoxPopuli

This comment has been minimized.

Copy link

commented Jul 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.