emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.47k stars 4.21k forks source link

Async testing helper selectors resolve before async actions resolve #10578

Closed danfinlay closed 8 years ago

danfinlay commented 9 years ago

I previously reported this in the ember-qunit repository, but was informed it actually belongs here.

To recap the issue:

Async testing helpers only wait for router-based run loop async actions to resolve, and they freely resolve before action-triggered async actions. I think this behavior is strange and incorrect, and I think async test helpers should wait for the entire run queue to empty before resolving.

An example, as I wrote in my original post:

If I have a button that reveals another button, let's say something like:

<button class="reveal" {{action 'revealOtherButton'}}>Reveal!</button>
{{#if isRevealed}}
  <button class="special" {{action 'doImportantThings'}}>Do it!</button>
{{/if}}

And in my tests I say

click('.reveal');
click('.special');

I get an error, that .special was not found.

However, if I wrap that second click in an andThen helper, it works. (EDIT: Seemingly more often works, I no longer consider this a solution.)

click('.reveal');
andThen(function(){
  click('.special');
});

This implies to me that the selectors are resolved immediately, while the actions are resolved asynchronously. This seems wrong to me. I would expect the selectors to resolve as part of the async action, so I can click a selector that isn't revealed yet, as long as it's after an async helper that would reveal it.

Am I reading this right? Is this deliberate behavior? If so, I would argue it's wrong.

lolmaus commented 9 years ago

Please post a JSBin demo.

danfinlay commented 9 years ago

I think I need ember-jsbin-cli, because I just can't quite get jsbin tests working, tried copying from the docs and from this gist, but they each are failing before they can demonstrate what I mean. I'm going to take a break from this now.

first attempt. "Cannot call compile without the template compiler loaded." second attempt. "Nothing is handling the action"

danfinlay commented 9 years ago

I think this issue is echoing the sentiments of this StackOverflow post.

danfinlay commented 9 years ago

The solution to that SO post provides a workaround, as well as a working JSBin for the workaround.

If you'd like to see the problem behavior in action, here's a version of his workaround with the workaround removed.

lukemelia commented 9 years ago

I'm not confident at the moment that the problem is as described by this issue, but I do believe there is a bug here. Best next step would be to create a failing test in the ember-testing suite.

danfinlay commented 9 years ago

Thanks for the guidance here, @lukemelia, and by the way, great talk at EmberConf! With a visual paradigm like that, why would we want to do anything else? :)

I'll try to get a failing test in the Ember testing guide for this.

danfinlay commented 9 years ago

Also, @lukemelia: Can you explain what your controller.get('promise') technique does? I can't find documentation of what a controller returns in response to this.

danfinlay commented 9 years ago

I especially wonder because not all controllers return a value to this request, so it's a buggy solution for me.

lukemelia commented 9 years ago

I'm glad you like the talk, @flyswatter. In the example workaround, the promise property is set within searchTextChanged. It's not a standard part of a controller.

danfinlay commented 9 years ago

Yeah my bad, I just was noticing that myself!

So to make that async helper work, you need to know the name of a promise on the controller. That's way more work than someone should have to do to test async actions on a controller!

danfinlay commented 9 years ago

Is there a guide to writing tests for Ember somewhere? I suspect I'd write this failing test in this file somewhere, but I'm not sure where the models and controllers that are referenced are being loaded from, so I don't know what controller I can add an async method to in order to test this.

danfinlay commented 9 years ago

@lukemelia was right, my revised JSBin was not testing what I meant it to.

This corrected JSBin simplifies the original example to demonstrate what I meant to, and yet the test isn't broken, so that's probably going to drive me insane now.

danfinlay commented 9 years ago

Maybe what I'm really missing is that Promises are not automatically added to the Ember run loop??

stefanpenner commented 9 years ago

i think we need a convention, where if you return a promise from an action handler, the framework realizes async has started, and the test framework can be informed correctly.

This is RFC or stackoverflow territory, as it is missing functionality and not really a bug.

stefanpenner commented 9 years ago

Maybe what I'm really missing is that Promises are not automatically added to the Ember run loop??

they are, but the run-loop has nothing todo with async test helpers waiting.

danfinlay commented 9 years ago

the run-loop has nothing todo with async test helpers waiting.

Doesn't it? How do the async helpers know when the run loop has completed if they aren't tapping into it?

i think we need a convention, where if you return a promise from an action handler, the framework realizes async has started, and the test framework can be informed correctly.

I totally agree with you, basically as Ember goes more and more down the async/Promises road, returning Promises should do "the right thing" by default.

This is also feeling deeper and deeper out of my territory. I haven't even figured out how the Ember test suite works, so submitting a decisive RFC feels way more core than I'm really informed to be.

stefanpenner commented 9 years ago

Doesn't it? How do the async helpers know when the run loop has completed if they aren't tapping into it?

run-loop is sync, their are waiters that are checked, as is the router. https://github.com/emberjs/ember.js/blob/master/packages/ember-testing/lib/helpers.js#L210-L237

If we arbitrary promises held the tests back, the tests themselves would cause the test to pause forever.

chnn commented 9 years ago

What's a way of working around this? More and more I find myself performing asynchronous behavior that is outside of a transition and is not an ajax request. Should I use registerWaiter in app code? Set promise properties on application objects and look them up in tests (like the above SO answer)?

The guides say that async helpers "are 'aware' of (and wait for) asynchronous behavior within your application, making it much easier to write deterministic tests". Yet they only wait for some async behavior to complete, which can only be found out by reading that source code. I would be happy to submit a PR to the docs that makes it clear that the async helpers only wait for some async behavior, but I hope there's a more ideal solution. I can't write deterministic acceptance tests without solving this.

gabrielgrant commented 9 years ago

Also curious about this. I published a generic waiter for non-AJAX-based models to fix acceptance testing for Emberfire, ember-pouch etc., and have submitted a PR to wrap emberfire's login process to be test-aware, but it seems like trying to plug these async holes one-by-one may be a losing game and shouldn't really have to be an application-level concern in most cases.

@stefanpenner do you still think that tracking promises returned from an action is reasonably unlikely to break anything and sufficient to cover many/most use cases? If so, I may be able to take a crack at writing up an RFC next week.

stefanpenner commented 9 years ago

do you still think that tracking promises returned from an action is reasonably unlikely to break anything and sufficient to cover many/most use cases?

yes, its possible. But potentially something we can support via add-on to start, and cut over in 3.0 (if it is breaking)

If so, I may be able to take a crack at writing up an RFC next week.

:+1:

davewasmer commented 8 years ago

I put together an RFC that I think might address this, although it's a bit different from @gabrielgrant's suggestion of tracking the return value of actions.

acorncom commented 8 years ago

Now that we've got an RFC where this is being discussed, I'll go ahead and close this issue.

gtsop commented 4 years ago

Can I ask what is the ember-octate status regarding this issue? It's hard to find information on how to wait for async actions on a controller while doing acceptance tests. The guides make it seem like it should Just Work with await statements, but apparently it doesn't.

pzuraq commented 4 years ago

In general whenever you perform an async action, like clicking an element, you should await it so it completes everything that it is doing. Do you have a specific example you’re having trouble with?