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

Acceptance tests halt due to run.later scheduling #32

Closed
jgwhite opened this issue Mar 23, 2015 · 6 comments
Closed

Acceptance tests halt due to run.later scheduling #32

jgwhite opened this issue Mar 23, 2015 · 6 comments

Comments

@jgwhite
Copy link

jgwhite commented Mar 23, 2015

Given the following app:

// app/routes/application.js

export default Ember.Route.extend({
  actions: {
    sayHello() {
      let flashes = this.get('flashes');
      flashes.info('Hiya');
    }
  }
});
<!-- app/templates/application.hbs -->

<button {{action "sayHello"}}>Say hello</button>

{{#each flashes.queue as |flash|}}
  {{flash-message flash=flash}}
{{/each}}

An acceptance test has the following behaviour:

// tests/acceptance/hello-test.js

test('say hello', function(assert) {
  visit('/');
  click('button:contains("Say hello")');
  // Our test waits here until the run.later scheduled in _destroyLater is flushed
  andThen(() => {
    // By the time we get here the flash message has gone already
    assert.ok(find('Hiya').length);
  });
});

This is tricky both because andThen behaves unintuitively and because the test will pause for the full timeout duration.

I’ve worked around this in my tests with a total hack:

// tests/test-helper.js

import FlashObject from 'ember-cli-flash/flash/object';

FlashObject.reopen({ _destroyLater: null });

This is obviously not ideal but I’m struggling to come up with a reasonable API for this scenario. Any thoughts?

P.S. Love this addon so much ❤️

@leojpod
Copy link

leojpod commented Mar 23, 2015

I've run into the same issue when trying to test the mouseover-ed feature.
I think what could be done is to overload the _destroyLater method instead of removing it.
Then we could for instance do something like this:

if the test should lead to _destroyLater being call:

FlashObject.reopen({_destroyLater: function () { ok(true, 'yes it was called!'); });

or ok(false) if the method shouldn't be called.

that being said... I haven't had time to try coding it since I thought about it and maybe it is a rubbish idea 😄

@jgwhite
Copy link
Author

jgwhite commented Mar 23, 2015

A declarative public API for testing would be lovely.

This isn’t a serious suggestion, but food for thought:

test('say hello', function(assert) {
  visit('/');
  click('button:contains("Say hello"));
  pauseFlashes();
  andThen(() => { assert.equal(find(':contains("Hiya")').length, 1); });
  flushFlashes();
  andThen(() => { assert.equal(find(':contains("Hiya")').length, 0); });
});

@poteto
Copy link
Collaborator

poteto commented Mar 23, 2015

Thanks for reporting this @jgwhite and @leojpod! Yeah, it is definitely a problem I want to fix. I like the idea of being able to pause / flush flashes in testing, but I think I'd prefer for the flashes to just work in testing (without additional helpers) in a way that you'd expect. I'll investigate over the next few days to figure out the best approach.

@JarrodCTaylor
Copy link

Here is another idea. I have ENV.stickyFlash = false set in environment.js (or some subset of flashMessageDefaults as explained in the README) then add messages with something like:

Ember.get(this, 'flashMessages').add({
  message: 'Custom message',
  sticky: ENV.stickyFlash
});

This just explicitly sets sticky to the already default false. Now in my test I have ENV.stickyFlash = true so the message is still present inside any andThen

@cowboyd
Copy link
Contributor

cowboyd commented Apr 7, 2015

Using Mocha, the run.later for non-sticky flashes causes the _destroyLater hook to run immediately, which causes the tests asserting the existence of a flash to fail, but it's basically the same issue.

From my perspective, the most intuitive approach is to treat flashes as always sticky in test. That's because I almost always want to assert that they were shown, but I have complete faith that if I told ember-cli-flash that they should go away after five seconds, then they will. In the rare instances where you need to make assertions against a clean slate within the same test case, you could provide a clearFlashMessages helper.

Based on that, I think @jgwhite's workaround is the simplest, least surprising option, and I'd consider making it automatic. That way you spend basically zero mental overhead thinking about how your flashes might work differently in test than in development and production.

@poteto
Copy link
Collaborator

poteto commented Apr 7, 2015

Yeah, that's a good point @cowboyd, I like that approach and I'm gonna work on implementing that and the helper.

@JarrodCTaylor you can also set sticky: true inside of your dummy app's config/environment

This was referenced Apr 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants