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

Wait for pending AJAX in acceptance tests. #264

Merged
merged 3 commits into from
Dec 14, 2017

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Dec 13, 2017

Ember internally tracks AJAX requests in the same way that we do here for legacy style "acceptance" tests using the ember-testing.js asset provided by emberjs/ember.js itself. When @ember/test-helpers's settled utility is used in a legacy acceptance test context any pending AJAX requests are not properly considered during the isSettled check below.

This utilizes a local utility method present in Ember since around 2.8.0 to properly consider pending AJAX requests done within legacy acceptance tests.

Ember internally tracks AJAX requests in the same way that we do here
for legacy style "acceptance" tests using the `ember-testing.js` asset
provided by emberjs/ember.js itself. When `@ember/test-helpers`'s
`settled` utility is used in a legacy acceptance test context any
pending AJAX requests are not properly considered during the `isSettled`
check below.

This utilizes a local utility method present in Ember since around 2.8.0
to properly consider pending AJAX requests done within legacy acceptance
tests.
@Turbo87
Copy link
Member

Turbo87 commented Dec 13, 2017

@rwjblue do you think it's possible to write tests for this?

@rwjblue
Copy link
Member Author

rwjblue commented Dec 13, 2017

yes, I think so, but I am not 100% certain. I am working on the failing test (the failure is legit) then I'll take a stab at adding a reasonable facsimile of a regression test...

@rwjblue rwjblue added the bug label Dec 13, 2017
@rwjblue
Copy link
Member Author

rwjblue commented Dec 13, 2017

Just pushed up the fix for the prior failure (leaky state in the legacy acceptance testing system itself), and a regression test. The test was somewhat gnarly to add but I confirmed that the test failed prior to the changes in addon-test-support/ and passes afterwards.

@rwjblue rwjblue force-pushed the handle-acceptance-test-ajax branch 2 times, most recently from 245c02e to c4894e4 Compare December 14, 2017 02:16
@rwjblue
Copy link
Member Author

rwjblue commented Dec 14, 2017

FYI - I added a guard to prevent the new regression test from running on Ember versions older than 2.8. It is possible that I could have made it work, but I'm not sure and this issue has sucked enough time already. 😡 We can always try to address if someone provides a compelling case (and hopefully PR 😝 ), but since Ember 2.4 is already sunsetted I think its ok if some features don't work properly...

@Turbo87
Copy link
Member

Turbo87 commented Dec 14, 2017

awesome work @rwjblue ❤️

@Turbo87 Turbo87 merged commit df72e53 into emberjs:master Dec 14, 2017
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.

2 participants