Deprecate and remove the WebDriverJS promise manager

Question

Deprecate and remove the WebDriverJS promise manager

jleyba opened this issue 8 years ago · 48 comments

This is a tracking bug for deprecating and removing the promise manager used by WebDriverJS.

Background

WebDriverJS is an asynchronous API, where every command returns a promise. This can make even the simplest WebDriver scripts very verbose. Consider the canonical "Google search" test:

let driver = new Builder().forBrowser('firefox').build();
driver.get('http://www.google.com/ncr')
    .then(_ => driver.findElement(By.name('q')))
    .then(q => q.sendKeys('webdriver'))
    .then(_ => driver.findElement(By.name('btnG')))
    .then(btnG => btnG.click())
    .then(_ => driver.wait(until.titleIs('webdriver - Google Search'), 1000))
    .then(_ => driver.quit(), e => {
      console.error(e);
      driver.quit();
    });

WebDriverJS uses a promise manager that tracks command execution, allowing tests to be written as if they were using a synchronous API:

let driver = new Builder().forBrowser('firefox').build();
driver.get('http://www.google.com/ncr');
driver.findElement(By.name('q')).sendKeys('webdriver');
driver.findElement(By.name('btnG')).click();
driver.wait(until.titleIs('webdriver - Google Search'), 1000);
driver.quit();

The original goal for the promise manager was to make it easier to read and reason about a test's intent. Unfortunately, any benefits on this front come at the cost of increased implementation complexity and reduced debuggability—in the example above, inserting a break point before the first findElement call will break before the findElement command is scheduled. To break before the command actually executes, you would have to modify the test to break in a callback to the driver.get() command:

let driver = new Builder().forBrowser('firefox').build();
driver.get('http://www.google.com/ncr').then(_ => debugger);
driver.findElement(By.name('q')).sendKeys('webdriver');

To help with debugging, tests can be written with generators and WebDriver's promise.consume function (or equivalent, from libraries like task.js). These libraries are "promise aware": if a generator yields a promise, the library will defer calling next() until the promise has resolved (if the promise is rejected, the generator's throw() function will be called with the rejection reason). While users will have to make liberal use of the yield keyword, they can use this strategy to write "synchronous" tests with predictable breakpoints.

const {Builder, By, promise, until} = require('selenium-webdriver');

let result = promise.consume(function* doGoogleSearch() {
  let driver = new Builder().forBrowser('firefox').build();
  yield driver.get('http://www.google.com/ncr');
  yield driver.findElement(By.name('q')).sendKeys('webdriver');
  yield driver.findElement(By.name('btnG')).click();
  yield driver.wait(until.titleIs('webdriver - Google Search'), 1000);
  yield driver.quit();
});

result.then(_ => console.log('SUCCESS!'), e => console.error('FAILURE: ' + e));

The need to use promise.consume/task.js will be eliminated in TC39 with the introduction of async functions:

async function doGoogleSearch() {
  let driver = new Builder().forBrowser('firefox').build();
  await driver.get('http://www.google.com/ncr');
  await driver.findElement(By.name('q')).sendKeys('webdriver');
  await driver.findElement(By.name('btnG')).click();
  await driver.wait(until.titleIs('webdriver - Google Search'), 1000);
  await driver.quit();
}

doGoogleSearch()
    .then(_ => console.log('SUCCESS!'), e => console.error('FAILURE: ' + e));

Given that the JavaScript language continues to evolve with more support for asynchronous programming (and promises in particular), the benefits to WebDriver's promise manager no longer outweigh the costs. Therefore, we will be deprecating the promise manager and eventually removing it in favor of native language constructs.

Deprecation Plan

Phase 1: allow users to opt-out of the promise manager

Estimated date: now (selenium-webdriver@3.0)

Within selenium-webdriver, replace all hard dependencies on the selenium-webdriver/promise.ControlFlow class with a new interface: selenium-webdriver/promise.Scheduler.
```
/** @interface */
class Scheduler {
 execute(fn) {}
 timeout(ms) {}
 wait(condition, opt_timeout) {}
}
```

Add a Scheduler implementation that executes everything immediately using native Promises.

/** @implements {Scheduler} */
class SimpleScheduler {
 /** @override */ execute(fn) {
   return new Promise((resolve, reject) => {
     try {
       resolve(fn());
     } catch (ex) {
       reject(ex);
     }
   });
 }

 /** @override */ timeout(ms) {
   return new Promise(resolve => setTimeout(_ => resolve(), ms));
 }

 /** @override */ wait(condition, opt_timeout) {
   // Implementation omitted for brevity
 }
}

Introduce the SELENIUM_PROMISE_MANAGER environment variable. When set to 1, selenium-webdriver will use the existing ControlFlow scheduler. When set to 0, the SimpleScheduler will be used.

When SELENIUM_PROMISE_MANAGER=0, any attempts to use the ControlFlow class will trigger an error. This will help users catch any unexpected direct dependencies. This will also impact the use of the promise.Deferred and promise.Promise classes, as well as the promise.fulfilled() and promise.rejected() functions, which all have a hard dependency on the control flow. Users should use the native equivalents.

At this point, SELENIUM_PROMISE_MANAGER will default to 1, preserving existing functionality.

Phase 2: opt-in to the promise manager

Estimated date: October 2017, Node 8.0

Following the release of a Node LTS that contains async functions, change the SELENIUM_PROMISE_MANAGER default value to 0. Users must explicitly set this environment variable to continue using the ControlFlow scheduler.

Phase 3: removing the promise manager

Estimated date: October 2018, Node 10.0

On the release of the second major Node LTS with async functions, the ControlFlow class and all related code will be removed. The SELENIUM_PROMISE_MANAGER environment variable will no longer have any effect.

Answer 1 · 2016-11-01T21:34:18.000Z

@jleyba I am happy to help out with documentation for this.
We just swapped out the promise manager to async/await entirely with babel and its working well for us!

For other people following up:

There is some sample code we put on github for reference using Mocha and Async / Await
https://github.com/airware/webdriver-mocha-async-await-example

The technical talk we gave at SF Selenium Meetup in Oct 2016 on the move to Async/Await can be found here: Fullstack End-to-end test automation with Node.js, one year later

Answer 2 · 2016-11-03T18:36:23.000Z

Phase 1 is complete: 3.0.0 has been published to npm.

Answer 3 · 2017-02-03T18:50:17.000Z

After the promise manager is removed, how will people be able to call WebDriver functions in NodeJS imperatively (one after another)?

Answer 4 · 2017-02-03T19:00:05.000Z

async/await is here for you!

Answer 5 · 2017-02-03T20:24:19.000Z

There's an example of using async/await in the background section of the initial write up for this issue.

Answer 6 · 2017-02-03T21:18:35.000Z

Ok. I tried this, but I still get an error:

driver = new webdriver.Builder().forBrowser('chrome').build();
await driver.get(url);

await driver.get(url);
^^^^^^
SyntaxError: Unexpected identifier

driver = await new webdriver.Builder().forBrowser('chrome').build(); has a similar exception

await driver.manage().window().maximize(); has a similar exception.

process.env.SELENIUM_PROMISE_MANAGER=0; did not stop the errors

Answer 7 · 2017-02-03T21:22:29.000Z

@wcdeich4

What node version are you using ? async await is supported in Node 7 and above with a feature flag. Else you can use babel to transpile to older versions of Node.

Answer 8 · 2017-02-03T21:24:43.000Z

Also for people jumping on board async await, this thread in the Nodejs project tracks the implementation roll out.

nodejs/promises#4

Answer 9 · 2017-02-03T21:42:07.000Z

Oh. I had Node v6.9.4.

Answer 10 · 2017-02-03T21:50:59.000Z

But I just installed 7.5.0 & still get the error :(

Answer 11 · 2017-02-04T05:20:58.000Z

You need to enable it on the command line when you start node:

// sample.js
async function x() {
  let x = await Promise.resolve(1);
  console.log(`x == ${x}`);
}
x();

$ node --version
v7.5.0

$ node --harmony_async_await sample.js 
x == 1

Answer 12 · 2017-03-04T05:11:24.000Z

Is there a way to guarantee that if the developer/tester writing a test uses plain Mocha (not Selenium's test.it), a plain test function (not a generator yielding the promises to co/promise-consume, nor async/await), and doesn't handle the promises with .then, that the test will fail at that test and not at some later asynchronous point? I was trying out the SELENIUM_PROMISE_MANAGER=0 flag a while back to see if we could detect tests that are not properly handling the promises, and in at least some cases we were getting tests passing without waiting for the promisey behavior, only for the error from the promise manager being turned off to occur later instead. I found that with a reference to the driver variable I could wait for a driver action's promise to resolve in an afterEach hook (e.g. driver.quit) and thus guarantee that failures are associated to the problematic test (albeit belatedly), but that's only doable if all the tests are using the same driver variable -- otherwise, "the developer has to remember to add this standard afterEach block and not some custom afterEach block that also might not wait on the promise" is no better than "the developer has to remember to handle the promises one way or another in the test function".

(If need be I can theoretically put together example code for this, verify it's an issue with the very latest Selenium, etc.; but I wanted to first check whether this is a known issue in the first place or if there's just something I'm missing or anything like that.)

Answer 13 · 2017-03-30T04:36:35.000Z

@ScottFreeCode this is a general problem for any promise-based API: how to promptly detect and handle unhandled rejected promises (the way the promise manager tries to do this is actually a source for a lot of its complexity).

Right now node will log a warning if it detects an unhandled native promise. I believe there's plans to handle these the same as uncaught exceptions in the future.

Answer 14 · 2017-04-18T04:25:06.000Z

Hi, do I understand correctly that removing promise manager and using async/await syntax doesn't allow me to use setControlFlow like in this example https://github.com/SeleniumHQ/selenium/blob/master/javascript/node/selenium-webdriver/example/parallel_flows.js?

Would it be possible to run paralell flows using async/await the at all?

Answer 15 · 2017-04-18T04:38:21.000Z

@artemyarulin I'm not sure about this but can't you just use await Promise.all(all, of, these)?

Answer 16 · 2017-04-18T04:56:31.000Z

Oh yeah, it works just fine. I was confused as first: converted original example to async/await syntax by just using await prefix for function and because of late binding all my browsers got same position so I though they share control flow

Here example converted to async/await which allows running selenium in parallel:

var webdriver = require('selenium-webdriver'),
    By = webdriver.By,
    until = webdriver.until;

webdriver.promise.USE_PROMISE_MANAGER = false;

[0,1,2].map(async (i) => {
  var driver = await new webdriver.Builder().forBrowser('chrome').build();
  await driver.manage().window().setSize(600, 400);
  await driver.manage().window().setPosition(300 * i, 400 * i);
  await driver.get('http://www.google.com');
  await driver.findElement(By.name('q')).sendKeys('webdriver');
  await driver.findElement(By.name('btnG')).click();
  await driver.wait(until.titleIs('webdriver - Google-haku'), 1000);
  await driver.quit();
})

Answer 17 · 2017-04-18T05:07:45.000Z

Hm, is there anything else that I need to set? Current example is rather unstable and now I got errrors like in 25% runs like ElementNotVisibleError: element not visible and Error: EPIPE write EPIPE while it was running just fine with original example.

Answer 18 · 2017-04-18T06:30:33.000Z

I'm not really familiar with selenium-webdriver, I use webdriverio instead. But from what I see I would say that your example is a bit not fair and that's the reason it's flaky and fails sometimes.

By "not fair" I mean that you're trying to run the same async code 3 times in the same browser. I.e. you're quit() of the first iteration can happen somewhere in the middle of the execution of the third iteration. Of course this is not true if selenium-webdriver launches separate chrome instances for every build(). But if it's not this is the reason.

Answer 19 · 2017-04-18T06:44:17.000Z

Yeah, it actually runs 3 separate browser apps and it works like expecteded most of the time.
Thanks, I'll have a look into webdriveio.

P.S. While searching found valuable discussion at gemini-testing/testplane#18, small world 😄

Answer 20 · 2017-05-03T02:00:21.000Z

@ScottFreeCode FYI, Angular uses Zone.js to track asynchronous tasks started in a test (docs here) and wait for them all to finish before moving on to the next test. I'd like to add something similar to Protractor, eventually.

Answer 21 · 2017-05-25T04:56:55.000Z

   return new Promise((resolve, reject) => {
     try {
       resolve(fn());
     } catch (ex) {
       reject(ex);
     }
   });

is Equal to

   return new Promise((resolve, reject) => {
       resolve(fn());
   });

In the PromiseA+ standard. So let's do not overcomplicate the code and keep the original call stack.

Answer 22 · 2017-06-12T20:41:08.000Z

@jleyba Now that node 8 is out and async/await is not behind a flag anymore, are you about to set "webdriver.promise.USE_PROMISE_MANAGER = false" as the default value now?

Answer 23 · 2017-07-07T18:09:37.000Z

@mo No. The default won't change until node 8 moves to LTS (which is scheduled for October)

Answer 24 · 2017-07-25T19:41:04.000Z

@artemyarulin: Did you ever figure out the EPIPE issue? I'm running into the same when trying to rewrite a test runner for USE_PROMISE_MANAGER = false.

Answer 25 · 2017-07-26T05:09:19.000Z

Nope, don't remember what I've did but it went away. Actually just checked and tests are using async/await, but I don't change USE_PROMISE_MANAGER.

Also started migration to https://github.com/gemini-testing/gemini for better screenshot support

Answer 26 · 2017-08-01T18:33:03.000Z

Hi folks, quick update: it looks the W3C WebDriver spec is nearing completion. When that's done, Selenium 4.0 won't be far behind. I don't want this deprecation to span beyond a major version bump, so it looks like the deprecation plan will be sped up considerably.

My current thinking is changing USE_PROMISE_MANAGER = false by default and deprecating all the promise manager stuff now (or what would be a 3.6.0 release to npm). The question then is whether to keep the deprecated promise manager around for a year or just delete it in 4.0

Curious to know what the community thinks.

Answer 27 · 2017-08-02T02:18:33.000Z

I really like this selenium-webdriver JS binding. And I am creating my own Web GUI testing engine on top of selenium-webdriver (https://www.npmjs.com/package/tia).
I always did not like the promise manager, due to problems at debugging (loosing calls stack), and almost did not use promise manager abilities in the engine.

So, as for me - I will not regret if this promise manager will be removed.

Also I think, any code requires some man-hours to support it, so the promise manager could be deleted to allow contributors to focus on really useful stuff.

Answer 28 · 2017-08-02T09:32:43.000Z

Hi @jleyba !
I think we can delete promise manager as keeping it's codebase can be painful and confusing for developers/contributors. People using it can suck to 3.6 until migration.
Thanks for all your work btw!

Answer 29 · 2017-08-02T09:40:19.000Z

We don't need the promise manager either 👍

Answer 30 · 2017-08-09T19:41:19.000Z

@jleyba
This is big breaking change and migration is really hard to huge projects.

We should add some deprecation warnings messages into code first. But not moving Promise Manager into 4.x looks like good idea.

Answer 31 · 2017-10-25T18:48:58.000Z

The promise manager has been removed. This change will be included in the 4.0 release, some time after the next Node LTS (so November at the earliest?)

Answer 32 · 2017-10-27T18:32:37.000Z

Thanks for all your work @jleyba!!

Answer 33 · 2017-11-22T20:32:05.000Z

Any chance we can add a checkboxes next to the phases, also perhaps with the PR number.

Answer 34 · 2017-12-22T19:57:32.000Z

So has the promise manager been removed?

Answer 35 · 2018-01-03T13:11:03.000Z

Would it be possible to publish a prerelease for 4.0.0-dev to npm? 😃 Would be nice for some easy tests and experiments.

Answer 36 · 2018-05-03T22:45:35.000Z

Since this is a breaking change that impacts almost every line of test code, what is the time line for a complete supported version 4.0 npm module publication?

In other words how much time do we have to prepare?

Answer 37 · 2018-05-04T21:25:48.000Z

npm already has 4.0.0-alpha.1 which includes the removal of the promise manager. It's tagged as "alpha" because the Selenium project as a whole wasn't ready to move to 4.0 and I couldn't release it as part of 3.X given the scope of changes. It's still considered a fully supported release, just like anything else published to npm

Answer 38 · 2018-05-04T23:23:41.000Z

@jleyba any plan to do another 3.x release?

Answer 39 · 2018-05-04T23:39:24.000Z

@jleyba Thanks! Sorry I was not clear. I need to know when selenium-webdriver will release 4.0.

We support multiple languages and we would really prefer to use the same version across all of them. Thus we can't switch to 4.0 alpha, and yet we need a long lead time to prepare for it.

I'm only looking for a ballpark figure, "in the next few weeks", "early fall 2018".

Answer 40 · 2018-05-05T01:59:10.000Z

@johnjbarton Simon Stewart (@shs96c) is on record that we’ll be starting the engines on a 4.0 release once the W3C WebDriver Specification goes to Recommendation, which is likely to happen in late May/early June. Expect some beta period, then a full 4.0 release. In other words, “before Christmas” (inside joke for long-time project followers).

Answer 41 · 2018-05-14T07:23:40.000Z

Will we able to use Jasmine after this update implementation ?

Answer 42 · 2018-06-29T21:04:27.000Z

Hello.
What is the planned dates to release of this feature?
Does it mean that all protractor tests need to have async/await in specs and page-objects when promise manager will be completely removed?

Thank you in advance.

Answer 43 · 2018-07-12T10:34:44.000Z

@jleyba How we can use async/await for cases when need to fill element (or block of elements) in web page using setter? Example:

mainPage.Information = "Address information..."

Where information is setter with sendKeys, like:

set Information(value) {
   this.information.sendKeys(value);
}

Typescript is not allow async/await for setters.
So, how we can use such approach without promise manager?

Thank you.

Answer 44 · 2018-07-24T16:34:35.000Z

Does anyone help me with comment above?
What is the destiny with promise manager?

It is a not good situation, when something couldn't work due to deprecation of functionality.
Using a setter to fill web-elements is very laconic style and it is bad to lose it.

Thanks!

Answer 45 · 2018-07-24T16:51:10.000Z

await mainPage.setInformation("Address information...");

Answer 46 · 2018-07-24T17:12:27.000Z

@johnjbarton, thank you, but it is not a solution. Its just a method, but the idea to use a setter.

Answer 47 · 2018-07-24T17:16:15.000Z

If TypeScript won't let you use async/await with a setter, don't use a setter.

FWIW, I don't think you should try to hide expensive RPCs in a setter anyway. Bit of an anti-pattern IMO.

Answer 48 · 2019-07-12T17:15:51.000Z

@StanislavKharchenko this isn't a TypeScript thing. This is a JavaScript thing.

https://es5.github.io/#x11.13.1

The value of a setter expression is always the RHS.

So there can be no promise to wait on. await page.value = 'abc' makes no sense.