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

Feature Request: Testability API changes #15585

Closed
heathkit opened this issue Jan 5, 2017 · 5 comments
Closed

Feature Request: Testability API changes #15585

heathkit opened this issue Jan 5, 2017 · 5 comments

Comments

@heathkit
Copy link

heathkit commented Jan 5, 2017

What is the current behavior?

Protractor synchronizes with Angular applications by using the whenStable() method of the Testability API to be notified when there are no more asynchronous tasks pending. This works for most cases, however there are situations in which whenStable() will not return in a reasonable amount of time, yet the application is stable enough to be tested. These include applications which:

  • have polling behavior in a $timeout callback (they should be using $interval, but this is still a * consistent problem)
  • make frequent $http calls for analytics or tracking that are unrelated to the UI.
  • have long-running $timeout calls, the result of which will not affect the UI

In these situations, users get a confusing timeout error from Protractor. We'd like to add some features to the testability API that will give users the chance to tune it for their application and get better synchronization behavior from Protractor.

Protractor also tries to provide debugging information about pending $timeout and $http tasks in the event of timeouts. The way we currently do this is complicated and not very reliable, and it would be great if we could just get this information from the Testability API directly.

These issues have previously been discussed in #14118 and the longstanding angular/protractor#169.

What is the expected behavior?

Ignore $timeout tasks with invokeApply=true

Since calling $timeout with invokeApply set won't cause a digest cycle, there's no need to wait for these tasks to finish as they won't mutate the page when they finish. This would be a breaking change for e2e tests, which were previously waiting on these timeout tasks.

ignoreHttpDomains

Add an option to the testability service (could just be a method to call) to whitelist certain domains (or url patterns). When making requests, $http would query the testability service and not increment the outstanding request count for whitelisted URLs.

Timeout Reporting

It would be helpful if we could pass a timeout and an error handler to whenStable(). If waiting for stability times out, the error handler would be called with a list of tasks that were pending (or at least a count).

Fine-grained deferred tracking

It would be nice to have an alternative to whenStable() that invokes a listener when the set of deferred tasks changes. That would give us total flexibility to decide which asynchronous tasks we need to wait for. It would be helpful to know how many tasks are pending and have some kind of detailed information about the task.

@Narretz
Copy link
Contributor

Narretz commented Jan 11, 2017

Hi, this is a great list of improvements. Some notes:

Ignore $timeout tasks with invokeApply=true

You can't guarantee that "they won't mutate the page when they finish". 3rd party modules could do all sorts of crazy stuff that doesn't rely on the digest cycle to mutate the page.

ignoreHttpDomains

Sounds like an interesting addition. This would be for requests such as logging, analytics etc that don't affect the actual app? Couldn't those be mocked, too?

Timeout Reporting

Sounds useful.

Fine-grained deferred tracking

Sounds useful on the surface, but I don't know how much sense it makes to introduce a completely new API.

As always, a PR with a PoC is very welcome.

Regarding timeouts, there's also more discussion here: #14118

@heathkit
Copy link
Author

You can't guarantee that "they won't mutate the page when they finish". 3rd party modules could do all sorts of crazy stuff that doesn't rely on the digest cycle to mutate the page.

That's a good point. Still, we think ignoring these timeout tasks would be more correct, even though conceivably people could do anything when the callback comes in. We have a long standing issue with people doing polling behavior or long-running $timeout tasks that block testing. We've talked about having people label the tasks they would like to ignore (#14118, as you found), or adding an ignoreForTesting flag, but having the Testability API ignore invokeApply tasks seems like a reasonable compromise that avoids complicating the $timeout with extra testability-related options.

We have ways for people to add custom client-side wait-logic, so even if there are cases where they'd need to wait for an invokeApply it's easier for clients to add waiting than it is to avoid waiting on things.

ignoreHttpDomains

Sounds like an interesting addition. This would be for requests such as logging, analytics etc that don't affect the actual app? Couldn't those be mocked, too?

They could, but in practice we've found some teams are more likely to mess with our synchronization logic than mock these http calls. This would be more of a convenience, quality of life thing.

@Narretz
Copy link
Contributor

Narretz commented Jun 18, 2018

I'm closing this as duplicate of #14118.
Case 1) is very similar to the main use case in this issue.
Case 2) can behandled by mocking out requests to analytics urls etc.
Case 3) can also be covered by ignoring timeouts completey. Ignoring invokeApply = false timeouts would be a BC which we can't do.

We might actually get better task tracking into 1.7: #16603 but for e2e tests, Protractor would need to implement the new APIs

@heathkit
Copy link
Author

Thanks for cleaning this up! FYI, we added task tracking to the Testability API for Angular in angular/angular#16863, though we still need to add support for the new API in Protractor.

@gkalpak
Copy link
Member

gkalpak commented Jun 19, 2018

Oh, interesting. It might be a good idea to match Angular's API (to a reasonable extent).
Thx for the info, @heathkit.

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

No branches or pull requests

3 participants