Skip to content

update acceptance test generator#51

Merged
kimroen merged 3 commits intokimroen:masterfrom
fivetanley:acceptance-test-generation-fix
Feb 4, 2015
Merged

update acceptance test generator#51
kimroen merged 3 commits intokimroen:masterfrom
fivetanley:acceptance-test-generation-fix

Conversation

@fivetanley
Copy link
Contributor

Newer versions of QUnit use the return value of setup and teardown to wait for promises if the return value has a .then function. Coffeescript has implicit return so it returns the Application instance by default. There are a few problems with this:

  1. Ember.Application.then is deprecated
  2. The promise never resolves if you are calling visit/etc in the test block. The code to make the promise resolved can't run before setup is done (test waits for setup).

This fixes ember generate acceptance-test hanging by default.

Newer versions of QUnit use the return value of `setup` and `teardown` to wait for promises if the return value has a `.then` function. There are a few problems with this:

1) Ember.Application.then is deprecated
2) The promise never resolves if you are calling `visit`/etc in the test block. The code to make the promise resolved can't run before setup is done (`test` waits for `setup`).

This fixes `ember generate acceptance-test` hanging by default.
@kimroen
Copy link
Owner

kimroen commented Feb 3, 2015

Thanks!

You can choose to not return anything by ending the function with just return - maybe that is better in this case? Example.

@taddeimania
Copy link
Contributor

Thanks for this - it was causing my integration tests to hang. Glad to see a fix so fast.

@kimroen
Copy link
Owner

kimroen commented Feb 4, 2015

Thanks again! 👍

kimroen added a commit that referenced this pull request Feb 4, 2015
@kimroen kimroen merged commit 910cee3 into kimroen:master Feb 4, 2015
kimroen added a commit that referenced this pull request Feb 4, 2015
@kimroen
Copy link
Owner

kimroen commented Feb 4, 2015

Published as version 0.8.1, with some other small bugfixes.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants