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

Why should browser.waitUntil continue to wait if promise has been fulfilled or rejected? #2361

Closed
jochakovsky opened this issue Oct 21, 2017 · 9 comments

Comments

@jochakovsky
Copy link

The problem

I'm writing some tests that require making a call to an API and waiting for the call to finish before continuing with the test. browser.waitUntil(condition) seemed like a good utility to use, but I'm having some trouble understanding why it behaves the way it does when condition is a promise.

Environment

  • WebdriverIO version: 4.8.0
  • Node.js version: 8.7.0
  • Standalone mode or wdio testrunner: wdio testrunner
  • if wdio testrunner, running synchronous or asynchronous tests: synchronous
  • Additional wdio packages used (if applicable):

Details

Current behavior seems to be as follows:

  • If condition resolves to a truthy value, waitUntil stops waiting.
  • If condition resolves to a falsey value, or rejects with any value, waitUntil times out with WaitUntilTimeoutError.

API documentation describes this behavior correctly.

If condition is a plain function, re-checking it every interval until it is either truthy or times out makes perfect sense. However, promises, once resolved, cannot be resolved again and cannot change value. I'm having a lot of trouble understanding why someone would want the test to continue to wait until the timeout is reached when the outcome of the promise is known.

Proposal:

  • If condition resolves to a truthy value, waitUntil stops waiting.
  • If condition resolves to a falsey value, or rejects with any value, waitUntil throws an error (probably not WaitUntilTimeoutError anymore).

This still feels a little strange, since when working with promises, you're usually more concerned with resolved vs. rejected than truthy vs. falsey. An alternative that would be more intuitive, but might be too big of a change, would be:

  • If condition resolves, waitUntil stops waiting.
  • If condition rejects, waitUntil throws an error (probably not WaitUntilTimeoutError anymore).

Maybe browser.waitUntil is the wrong utility to use - I'm happy to consider alternative ways to synchronously wait for a promise to resolve before continuing with a test.

Link to Selenium/WebdriverIO logs

N/A

Code To Reproduce Issue [ Good To Have ]

First test passes, remaining tests fail:
https://gist.github.com/jochakovsky/65cea6612f210705d470224a4da2a718

@christian-bromann
Copy link
Member

I'm happy to consider alternative ways to synchronously wait for a promise to resolve before continuing with a test.

Have you tried to use the call command? It takes a callback which waits until a return promise is resolved.

@jochakovsky
Copy link
Author

Thanks @christian-bromann , this is doing exactly what I needed!

I'm still confused why browser.waitUntil's handling of promises ever makes sense. Should passing a promise to browser.waitUntil be deprecated, and the docs updated to direct users towards browser.call?

@christian-bromann
Copy link
Member

Should passing a promise to browser.waitUntil be deprecated, and the docs updated to direct users towards browser.call?

No. We use waitUntil internally for all the waitFor commands. And they make use of promises.

@jochakovsky
Copy link
Author

No concerns from me about internal use, but "is waitUntil used internally with promises" and "should waitUntil's public API documentation officially support promises" aren't quite the same question. As part of the public API, my opinion is that this behavior violates the principle of least astonishment and will likely lead to more people writing tests with unexpected behavior, or tests that take much longer to fail than expected.

@christian-bromann
Copy link
Member

should waitUntil's public API documentation officially support promises

waitUntil supports promises, it just continues waiting if a promise was rejected, right?

@jochakovsky
Copy link
Author

jochakovsky commented Nov 3, 2017

Yes - I just can't personally see how that would be the desired behavior for an external consumer. It also continues waiting if the promise was fulfilled with a falsey value.

@christian-bromann
Copy link
Member

So to be clear, you think that it would be better if waitUntil would error out if the returned promises is rejected. If so, I can definitely see use cases where a promise is rejected and someone wants to use waitUntil to retry until it gets fulfilled.

@jochakovsky
Copy link
Author

That use case makes sense for a function, which can potentially return a different value on any call. But promises, once settled, will never change value. It doesn't seem to make sense to continue waiting.

https://www.ecma-international.org/ecma-262/6.0/#sec-promise-objects

A promise is said to be settled if it is not pending, i.e. if it is either fulfilled or rejected.

A promise is resolved if it is settled or if it has been “locked in” to match the state of another promise. Attempting to resolve or reject a resolved promise has no effect.

@christian-bromann
Copy link
Member

Ahh I think now I understand. I don't think waitUntil should be used with just a promise as a condition. I can't recollect what my initial intention was when I put Function/Promise in the docs. This stuff can already be achieved using the call command (which is also documented here).

rostik404 pushed a commit to gemini-testing/webdriverio that referenced this issue Nov 21, 2017
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

No branches or pull requests

2 participants