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

Fix async test helpers #10463

Merged
merged 2 commits into from
Mar 15, 2015
Merged

Fix async test helpers #10463

merged 2 commits into from
Mar 15, 2015

Conversation

teddyzeenny
Copy link
Contributor

Refactored async helpers to be more robust and fixed some issues with the previous implementation. Before now you couldn't block an async helper by returning any promise.

This didn't work (but now does):

Ember.Test.registerAsyncHelper('sleep', function(app, duration) {
  return new Ember.RSVP.Promise(function(resolve) {
    setTimeout(resolve, duration);
  });
});

The only two ways that worked were returning wait or using Ember.Test.promise which shouldn't be public api.

Ember.Test.registerAsyncHelper('sleep', function(app, duration) {
  let promise = new Ember.RSVP.Promise(function(resolve) {
    setTimeout(resolve, duration);
  });
  return wait(promise);
});

Ember.Test.registerAsyncHelper('sleep', function(app, duration) {
  return Ember.Test.promise(function(resolve) {
    setTimeout(resolve, duration);
  });
});

Some async helpers were also accidentally triggered synchronously if they were the first helper in a test. People should not rely on this behavior, so now all async helpers are triggered asynchronously. (The checkbox helpers were relying on this unwanted behavior so I fixed them).

@rwjblue
Copy link
Member

rwjblue commented Mar 15, 2015

LGTM

rwjblue added a commit that referenced this pull request Mar 15, 2015
@rwjblue rwjblue merged commit d6f42c2 into emberjs:master Mar 15, 2015
@seanpdoyle
Copy link
Contributor

@teddyzeenny 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants