nightwatchjs / nightwatch

Integrated end-to-end testing framework written in Node.js and using W3C Webdriver API. Developed at @browserstack
https://nightwatchjs.org
MIT License
11.85k stars 1.35k forks source link

(feature) Allow returning promise objects in executeAsync #973

Open fabiosantoscode opened 8 years ago

fabiosantoscode commented 8 years ago

I'm currently doing some acceptance tests using nightwatch, parts of which run on the clientside, and I'd like to use Promises, as they can roll up errors and report them back quite easily.

However my problem is that I have to wrap all my client functions in the following:

(mostly pseudocode)

.executeAsync(function iAmExecutedOnTheClient(done) {
  Promise.resolve()  // to propagate any sync error in doSomeSetUp()
  .then(() => {
    return window.doSomeSetUp()
  }).then(() => {
    // custom stuff goes here. Exceptions may occur.
    // Testing that a third party is being called correctly using a sinon spy, if you're curious.
  }).then(() => { done({ noIssuesOccurred: true }) }).catch((e) => { done({error:e}) })
}, [], function checkForClientErrors (value) {
  if (value.noIssuesOccurred) // Ok, cool
  else if (value.error) // something was thrown/rejected, fail the test
  else // something else happened, like hitting ctrl+C during the test
})

This is not very DRY. I've factored out checkForClientErrors, but still the part where the promise is being made (just to catch stuff like accidental ReferenceErrors), and then the error being recreated and sent back to nightwatch, is quite problematic.

What I would like to write would be:

.executeAsync(() => {
  return Promise.resolve()
  .then(() => {
    return window.doSomeSetUp()
  }).then(() => {
    // my custom stuff goes here. If any errors are thrown by doSomeSetUp,
    // they are sent back to the test
  })
})

Which is free of custom error handling, similar to the mocha interface (returning a promise means the test ends when it resolves, and fails when it rejects), and catches all errors thrown synchronously (meaning, if doSomeSetUp were undefined, the test would fail and report that back to me).

I quite like my current workaround (macros OMG!), but since it involves Function#toString and eval() I'd like to avoid it altogether:

function createBrowserWrapper(fn) {
  return eval('(' + String(function wrapper(done__) {
    return Promise.resolve()
    .then(() => {
      return (MAIN_PART)()
    })
    .then(() => { done__({ noIssuesOccurred: true }) }).catch((e) => { done__({error:e}) })
  }).replace('MAIN_PART', String(fn)) + ')')
}

...

browser
.executeAsync(createBrowserWrapper(() => {
  return doSomeSetUp()
  .then(() => {
    // my test goes here currently
  })
}), [], verifyClientOutput)
beatfactor commented 8 years ago

Can you maybe summarize your request? It's not very clear what needs to be done here.

fabiosantoscode commented 8 years ago

TL; DR: in the context of executeAsync, allow returning a promise for a value instead of calling done() with the value. The idea being that promises automatically pass errors on, making client code more robust.

On 11:37, Mon, 16 May 2016 Andrei Rusu, notifications@github.com wrote:

Can you maybe summarize your request? It's not very clear what needs to be done here.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/nightwatchjs/nightwatch/issues/973#issuecomment-219396749

marr commented 8 years ago

Any update on this? The current solution for running async commands outlined here seems somewhat convoluted: http://stackoverflow.com/questions/37044543/nightwatch-is-there-a-way-to-find-out-if-executeasync-has-executed-the-javascri#37046211

senocular commented 8 years ago

@marr That SO response is convoluted. The purpose of the last callback function in executeAsync is to have a function that is executed in the nightwatch test after the execute code (first function argument) has completed and called done. That example is setting something in a synchronous execute using a timeout then later calling an executeAsync to wait for that result when it could have used a single executeAsync from the start:

    client.executeAsync(function(data, done) {
        window._asyncResult = undefined;
        setTimeout(function(){
          window._asyncResult = "abcde";
          done(window._asyncResult);
        }, 2000);
      }, ["1234"],
      function(result) {
        // evaluate the result
        client.assert.equal(result.value, "abcde");
      });

I guess you could use 2 if you felt it important to perform the assignment and validation in separate call stacks, but then you'd still go with the first async and then follow it up with a sync, avoiding any of that polling in the second bit.

As for this issue, I'm fairly sure its not getting in any time soon if at all. Use of promises have been proposed before (#638), and its not anywhere on the roadmap. This is also cramming promises into an existing api at the content level which involves additional string parsing to handle the promise wrapping (the implementation already shared above).

This would work better as a custom command, something like executeAsyncP which would use the createBrowserWrapper to do all the stuff needed to make promises work as expected in the execute code.

marr commented 8 years ago

Thanks @senocular! I remember your Flash tutorials from the AS3/Flex days.