Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Consider allowing e2e test to ignore certain timeouts #14118

Open
juliemr opened this issue Feb 23, 2016 · 13 comments
Open

Consider allowing e2e test to ignore certain timeouts #14118

juliemr opened this issue Feb 23, 2016 · 13 comments

Comments

@juliemr
Copy link
Member

juliemr commented Feb 23, 2016

Based on a discussion we had about angular/protractor#2950. Since this feature would require changes to Angular.js, moving discussion here.

The basic problem is that sometimes, apps have a long-running or repeating $timeout call for some reason. Protractor waits for all timeouts to finish before reporting stable, so this causes it fail for any tests on that page.

An example use case would be a page using a widget that they don't necessarily control the code for, which has a long running $timeout. Ideally, they'd be able to ignore this timeout without changing the code for the widget.

Mitigating, currently available solutions:

  • you can always turn off all waiting for Protractor using browser.ignoreSynchronization = true or by directly using browser.driver.<method>
  • If you control the offending app code, use $interval instead of $timeout

Potential solutions that would require changes to angular:

  • Instead of waiting for there to be 0 timeouts, the test writer could pass in a number X, and we could wait for there to be <= X.
  • We could add yet another parameter to $timeout telling angular to ignore it for reporting stability (note that this requires some changes to the app code)
  • We could add some ability to name your timeouts, and let protractor pass in a list of timeouts it should ignore. Concerns here: name conflicts, names getting minified. (note that this requires some changes to the app code)
@petebacondarwin
Copy link
Contributor

1: Instead of waiting for there to be 0 timeouts, the test writer could pass in a number X, and we could wait for there to be <= X.

Although this would be easy to implement, I think this would not work in enough cases that it is not worth investing time in as a complete solution. Moreover it is fragile in the case that someone adds a new unexpected timeout that basically breaks unrelated tests.

2: We could add yet another parameter to $timeout telling angular to ignore it for reporting stability

While this is a nice option in itself, we already have a heavily overloaded $timeout function signature - in fact we already allow unlimited arguments on this function.

3: We could add some ability to name your timeouts, and let protractor pass in a list of timeouts it should ignore. Concerns here: name conflicts, names getting minified.

If we could work around the minification issue then this would be neat, since it is good practice to name your functions from a debugging point of view.

@Narretz
Copy link
Contributor

Narretz commented Feb 24, 2016

You could exclude the names from minification in a built-step. Would be a bit of a hassle, but not too problematic.

@petebacondarwin
Copy link
Contributor

@Narretz : By "exclude" did you mean "extract"?

@juliemr
Copy link
Member Author

juliemr commented Feb 24, 2016

I don't think that introducing new build steps is a reasonable default recommendation - there are lots of different build systems and we can't have a preset solution for all of them. The only way to really do this generally would be to have the names a string constants.

@gkalpak
Copy link
Member

gkalpak commented Apr 13, 2016

So, I list a few cases that we would want to support ideally (feel free to add more):

  1. Ignore a specific long-running timeout.
  2. (a) Ignore all timeouts (but wait for other things, such as HTTP requests).
    (b) As an extra requirement, it would be nice to be able to toggle ignoring timeouts on/off.
  3. Ignore everything (e.g. timeouts, HTTP requests etc).

@gkalpak
Copy link
Member

gkalpak commented Apr 14, 2016

After looking into it a little more, I believe that (3) is covered by browser.ignoreSynchronization.

Also, (for most cases) (2a) and (2b) can also be achieved with timely use of browser.ignoreSynchronization. There is also the protractor-xhr-only-plugin (which BTW is not mentioned on the Plugins docs), which does exactly that.

I believe that many issues people are running into, stem from the improper use of browser.ignoreSynchronization, due to overlooking the asynchronicity that Protractor introduces to tests. (I'm specuating here.)

The obvious thing to do is to improve the documentation of browser.ignoreSynchronization, so that people use it correctly. BTW, it isn't mentioned at all on the API docs. Is this intentional, @juliemr ?

Another option to make this easier for people to take advantage of, would be to expose a method on protractor to allow people to run blocks of code with ignoreSynchronization enabled/disabled. E.g.:

/* Code run here waits for sync */

protractor.ignoreSynchronization(function () {
  /* Code run here does not wait for sync */
});

/* Code run here waits for sync */

And we still don't have a good story for (1).


FWIW, I think we could add support in Angular's testability to wait for specific asynchronous event types. This would allow more flexibility for (2a)/(2b) and not require extra plugins.
E.g. augment $$testability.whenStable() like this:

// Wait for everything. The same as what we have now (so it's backwards compatible)
$$testability.whenStable(callback);

// Wait for $http requests only. Ignore $timeouts or other stuff.
$$testability.whenStable('http', callback);

// Wait for $timeouts only. Ignore $http requests or other stuff.
$$testability.whenStable('timeout', callback);

// Wait for X and Y event types only. Ignore other stuff.
$$testability.whenStable(['X', 'Y'], callback);

Then Protractor should expose an API for the users to specify what they care to wait for at a given point.

@petebacondarwin
Copy link
Contributor

$$testability.whenStable(['X', 'Y'], callback);
What are X and Y here?

@gkalpak
Copy link
Member

gkalpak commented Apr 15, 2016

Types of async events. Right now I think we only track $http and $timeout, but in the future we might decide to track other stuff or let users register their async operations under a custom namespace.

@petebacondarwin
Copy link
Contributor

I see. I wasn't sure if you meant named timeouts or https requests to particular urls

@petebacondarwin
Copy link
Contributor

I agree that step 1 should be improving the current docs.

@gkalpak
Copy link
Member

gkalpak commented Apr 15, 2016

FWIW, Protractor aside, I believe that augmenting the API to allow users to specify "types" of deferred tasks is quite powerful, as it allows them to use non-Angular APIs, but still integrate with Angular's sense of "stability". (This would also provide a way to solve issues such as #13862 or #14116.)

E.g. one could do (imaginary APIs):

$browser.$incOutstandingRequestCount('http');
mySocketConnection.onmessage = function (event) {
  $browser.$completeOutstandingRequest('http', function() {
    // Do some stuff with `event` first
  });
};
mySocketConnection.send('You there, server?')

@petebacondarwin
Copy link
Contributor

@juliemr what do you think?

@juliemr
Copy link
Member Author

juliemr commented Apr 26, 2016

@gkalpak there's a problem that's not covered by browser.ignoreSynchronization - if a page has a setTimeout that is always pending, then you'll have to ALWAYS ignore synchronization for that page, which reduces the usefulless of Protractor.

We could potentially do the thing where we add more arguments to whenStable to determine what type of events to wait for. I don't see any downsides there other than small complexity. If it SG to Pete I'd be willing to take a stab at it.

(you're totally right, we should improve docs for ignoreSynchronization. We want to re-do it so we've been putting that off).

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