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.79k stars 1.31k forks source link

`waitUntil` not working with async/await #3511

Closed garg3133 closed 6 months ago

garg3133 commented 1 year ago

Description of the bug/issue

When using waitUntil command with async/await, Nightwatch does not wait for the callback passed to waitUntil to return true. The callback is executed only once (no matter what is the return value of callback) and then the commands following the waitUntil command start to execute, while the callback to waitUntil keeps on running in the background until true is returned.

image

The command works fine when used without async/await.

Additional discussion: https://browserstack.slack.com/archives/C027A82KQC9/p1669788479985619

Steps to reproduce

Write any test using waitUntil command with async/await (the callback passed to waitUntil should return false) and add a few other commands after waitUntil and then run it. The callback will run over the other commands.

Sample test:

'sample test': async function (browser) {
  await browser.url('https://nightwatchjs.org');

  const result = await browser.waitUntil(function() {
    browser.title();
    // console.log('hello');

    return false;
  }, 200000);

  console.log('Result returned by waitUntil:', result);

  const elements = await browser.findElement('input');
  const title = await browser.title();
  await browser.assert.titleContains('Nightwatch');
};

Sample test

The following code fails:

'Search for BrowserStack': async function (client) {
    await client
      .click('id', 'org.wikipedia:id/search_container')
      .sendKeys('id', 'org.wikipedia:id/search_src_text', 'browserstack')
      .click({selector: 'org.wikipedia:id/page_list_item_title', locateStrategy: 'id', index: 0});

    await client.waitUntil(async function() {
      // wait for webview context to be available
      const contexts = await this.contexts();

      return contexts.length > 1;
    }, 20000);

    const contexts = await client.contexts();
    await client.setContext(contexts[1]);

    await client.assert.textEquals('.pcs-edit-section-title', 'BrowserStack');  // command run in webview context

    client.end();
  }

But the following code works:

'Search for BrowserStack': async function (client) {
    await client
      .click('id', 'org.wikipedia:id/search_container')
      .sendKeys('id', 'org.wikipedia:id/search_src_text', 'browserstack')
      .click({selector: 'org.wikipedia:id/page_list_item_title', locateStrategy: 'id', index: 0})
      .waitUntil(async function() {
        // wait for webview context to be available
        const contexts = await this.contexts();
        console.log(contexts);  // <-- returns result value

        return contexts.length > 1;
      })
      .perform(async function() {
        // switch to webview context
        const contexts = await this.contexts();
        console.log(contexts);  // <-- returns result value

        await this.setContext(contexts[1]);
      })
      .assert.textEquals('.pcs-edit-section-title', 'BrowserStack');  // command run in webview context

    client.end();
  }

Command to run

No response

Verbose Output

No response

Nightwatch Configuration

mobile: {
      selenium: {
        host: 'localhost',
        port: 4723
      },
      disable_error_log: true,
      webdriver: {
        timeout_options: {
          timeout: 150000,
          retry_attempts: 3
        },
        keep_alive: false,
        start_process: false,
        // default_path_prefix: ''
      }
    },

    'native.android': {
      extends: 'mobile',
      'desiredCapabilities': {
        'appium:automationName': 'UiAutomator2',
        browserName: null,
    'appium:app': 'samples/sample.apk',
        'appium:appPackage': 'org.wikipedia',
        'appium:appActivity': 'org.wikipedia.main.MainActivity',
        'appium:appWaitActivity': 'org.wikipedia.onboarding.InitialOnboardingActivity',
        platformName: 'Android',
        'appium:platformVersion': '11',
        'appium:newCommandTimeout': 20000,
        'appium:chromedriverExecutable': 'chromedriver-mobile/chromedriver',
        'appium:avd': 'nightwatch-android-11'
      }
    },

Nightwatch.js Version

2.4.1

Node Version

No response

Browser

No response

Operating System

No response

Additional Information

No response

garg3133 commented 1 year ago

Another issue I've found with waitUntil is that if we pass an async callback to waitUntil command, Nightwatch commands inside the callback still just return NightwatchAPI and no promise which can be awaited to get the result. While in perform command, it works fine and Nightwatch commands return a promise inside async callback passed to perfom command.

All this is when we do not use async in the test case function.

Edit: This problem goes away if we set always_async_commands top-level setting to true in the Nightwatch config file.

Previous conversation regarding this:

Priyansh: I am not able to await Nightwatch commands and get the result inside waitUntil command if the test case is not an async function. If I make the test case async, I get the result value on awaiting the Nightwatch commands inside waitUntil, while on removing async from the test case, I only get the NightwatchAPI instance. This is not the case with perform command, there I always get the result value no matter the test case is an async function of not.

image

Andrei: maybe this will help https://github.com/nightwatchjs/vite-plugin-nightwatch/blob/main/nightwatch/commands/mountReactComponent.js

Priyansh: But here the command function is async, so maybe that's why it works. Can we not make Nightwatch commands inside waitUntil return Promise if an async callback is passed to waitUntil no matter the main test case is async or not? Just like we do with perform command?

Priyansh: Even putting waitUntil inside perform isn't working, idk why (considering Nightwatch commands inside perform returns promises when async callback is passed to perform, the callback passed to waitUntil should also work the same since it's running in same async environment?)

image

stimothy commented 1 year ago

I also was running into the same issue as mentioned above. I was seeing another side effect, that I don't know if it is directly related to this issue or not. But when commenting out the waitUntil check the issue resolves itself:

waitForServicesToLoad: async function () {
        const page = this;
        await this.waitForElementVisible(
            'xpath',
            '@servicesContainer'
        );
        await this.api.waitUntil(async function () {
            const val = await page.getText('xpath', '@servicesContainer');
            console.log('Test 1 ' + val);
            return val !== 'Loading...';
        });
        console.log('Test 2 ' + await this.getText('xpath', '@servicesContainer'));
    },
getServiceTypePhase: async function (serviceTypeId) {
        await this.toggleServiceTypeRow(serviceTypeId, false);
        await this.api.waitForElementVisible(
            'xpath',
            this._getServiceTypePhase(serviceTypeId)
        );
        let phase = await this.api.getText('xpath', this._getServiceTypePhase(serviceTypeId));
        phase = await this.api.getText('xpath', this._getServiceTypePhase(serviceTypeId));
        return phase;
    },
toggleServiceTypeRow: async function (serviceTypeId, toggleOpen = true) {
        await this.waitForServicesToLoad();
        await this.api.waitForElementPresent('xpath', this._getServiceTypeRowCollapsed(serviceTypeId));
        const collapsed = await this.api.isVisible('xpath', this._getServiceTypeRowCollapsed(serviceTypeId));
        if (toggleOpen && collapsed) {
            await this.api.click('xpath', this._getServiceTypeRowCollapsed(serviceTypeId));
            await this.api.waitForElementVisible(
                'xpath',
                this._getServiceTypeRowExpandedBanner(serviceTypeId)
            );
        } else if (!toggleOpen && !collapsed) {
            await this.api.waitForElementVisible(
                'xpath',
                this._getServiceTypeRowExpandedBanner(serviceTypeId)
            );
            await this.api.click('xpath', this._getServiceTypeRowExpandedBanner(serviceTypeId));
            await this.api.waitForElementVisible(
                'xpath',
                this._getServiceTypeRowCollapsed(serviceTypeId)
            );
        }
    },

In the first function we wait for the container to finish loading the ajax request for services. The second function first has to ensure that the service in question is in a collapsed state before retrieving the status. The third function toggles the service in question to be in an expanded or collapsed state. Note that in the second function I try and retrieve the phase twice. This is due to the issue mentioned. If I use the waitUntil in the first function, then the first getText call in the second function returns true instead of a string. Then if I immediately call the getText function again as shown in the second function it returns the correct string. Commenting out the waitUntil in the first function causes the second functions first getText call to return with the string as opposed to true. I'm not sure if this is related to waitUntil, but it reproduces this result every time as long as the waitUntil mentioned above is present.

pujagani commented 7 months ago

I was able to reproduce this with a simpler example without requiring mobile config.

describe('waituntil', function() {
  this.tags = ['until'];

  it('demo test using waituntil', async function(client) {
    await client.navigateTo('https://www.wikipedia.org/');
    await client.sendKeys('#searchInput', 'BrowserStack');
    await client.waitUntil(async function() {
// callback continues running the background
      await client.click('#typeahead-suggestions > div > a:nth-child(1)');

      const src = await client.source();

      return false;
    }, 20000);

    await client.click('.url > a:nth-child(1)');

    const source = await client.source();

// test passes here
    await client.assert.strictEqual(source.includes('Selenium'), true);

    client.end();
  });

  it('demo test using waituntil chained', async function(client) {
    let source;
    await client
      .navigateTo('https://www.wikipedia.org/')
      .sendKeys('#searchInput', 'BrowserStack')
      .waitUntil(async function() {
// callback fails and test does not run ahead
        await client.click('#typeahead-suggestions > div > a:nth-child(1)');

        const src = await client.source();

        return false;
      }, 20000)
      .click('.url > a:nth-child(1)')
      .source(src => {
        source = src;
      });

    client.assert.strictEqual(source.value.includes('Selenium'), true);

    client.end();
  });
});
AutomatedTester commented 7 months ago

@pujagani has done a quick test and it looks like the issue is with how we wrap the waitUntil from Selenium

The test that works at the moment is

it('doubleClick(element)', async function () {
      await driver.get(fileServer.whereIs('/data/actions/click.html'))

      let box = await driver.findElement(By.id('box'))
      assert.strictEqual(await box.getAttribute('class'), '')

      await driver.actions().doubleClick(box).perform()
      await driver.wait(async function() {
        return await box.getAttribute('class') === 'blue'
      }, 10000)
      assert.strictEqual(await box.getAttribute('class'), 'blue')
    })
AutomatedTester commented 7 months ago

We need to compare other commands with where waitUntil is used e.g. https://github.com/search?q=repo%3Anightwatchjs%2Fnightwatch%20waitUntil&type=code

garg3133 commented 7 months ago

As far as I remember, the issue here is with how Nightwatch creates and handles the queue. When Nightwatch sees waitUntil command, it pushes the command to the queue (which is a linear list at this point). But when it goes to executing the callback present in the waitUntil command, because Nightwatch can't append the new commands present in the callback to the end of the main queue (because the main queue can contain other commands that are to be executed after the callback has completed its execution), it instead creates child elements to the waitUntil node in the main queue (to form kind of a tree structure; if the callback has 3 Nightwatch commands, waitUntil node will have 3 child nodes).

Now the problem here is that, if the waitUntil command is used alone (with await and without chaining), the node for waitUntil in the main queue is resolved on the first iteration of the callback itself (as soon as all the 3 child elements of waitUntil are completed, it gets resolved) and does not wait for the subsequent iterations of the callback to complete (if the callback evaluates to false). Due to this, Nightwatch starts executing subsequent commands, while the callback keeps running in the background until it evaluates to true (or timeout).

But if waitUntil is chained, it waits until all the iterations of the callback are complete before moving to the next command.

The issue with the debug() command is also similar to this issue: https://github.com/nightwatchjs/nightwatch/issues/3642

dikwickley commented 7 months ago

Here is something i noticed, this bug happens only when any command inside the waitUntil callback fails (for example a .click command)

So this test waits all the way till the waitUntil times out

it('demo test using waituntil 1', async function (client) {
    let counter = 0;

    await client.navigateTo('https://www.wikipedia.org/');
    await client.sendKeys('#searchInput', 'BrowserStack');
    await client.waitUntil(async function () {
      counter += 1;
      console.log({ counter });
      return false;
    }, 20000);

    // test doesn't reach here

    await client.click('.suggestions-dropdown > a:nth-child(1)');
    const title = await client.getTitle();

    await client.assert.strictEqual(title, 'BrowserStack - Wikipedia');

    client.end();
  });

and this test has the above mentioned behaviour of not waiting for the callback and continue execution of test

  it('demo test using waituntil', async function (client) {
    let counter = 0;

    await client.navigateTo('https://www.wikipedia.org/');
    await client.sendKeys('#searchInput', 'BrowserStack');
    await client.waitUntil(async function () {
      counter += 1;
      console.log({ counter });
      await client.click('._wrong_selector'); //there is no such selector and this fails

      return false;
    }, 20000);

    await client.click('.suggestions-dropdown > a:nth-child(1)');
    const title = await client.getTitle();
    // test passes here
    await client.assert.strictEqual(title, 'BrowserStack - Wikipedia');

    client.end();
  });
garg3133 commented 7 months ago

@dikwickley It's not about having a failing command inside waitUntil callback, it's about having any Nightwatch command inside the callback. If there is any Nightwatch command or assertion inside the waitUntil callback and the callback evaluates to false, this bug would occur. But happy to be corrected.

chikara1608 commented 6 months ago

As far as my understanding goes with this, @garg3133 is right. The way queuing works in nightwatch is that if all the child nodes of a parent node are resolved, it calls the method to resolve the parent node too. In this case also, whenever child nodes (the commands inside conditional function) of waitUntil command node are resolved the method is called for resolution of waitUntil command thus prematurely resolving even though the condition might not have been true as of now. For reference see : https://github.com/nightwatchjs/nightwatch/blob/b4e273a5fc86a3f8b47eee28e6488ca8a942f1e5/lib/core/asynctree.js#L192C1-L195C8

chikara1608 commented 6 months ago

On further inspection about why when waitUntil throws an error, the execution stops in case we are chaining commands but it move forwards in case of individual commands, we can refer to runChildNode snippet where errors are handled : https://github.com/nightwatchjs/nightwatch/blob/b4e273a5fc86a3f8b47eee28e6488ca8a942f1e5/lib/core/asynctree.js#L137-L163

Error is thrown in case of both chained and non-chained test cases and thus waitUntil is resolved and queue clears in both cases. The difference lies henceforth, i.e. in case of chained commands, the other commands beside waitUntil wait to get resolved but since the queue is empty now, they don't get resolved and execution waits there and eventually throws error. On the other hand in case of individual commands, waitUntil when resolved it simply moves to next command, add them in queue and continue execution.

One possible fix for this can be to reject waitUntil's promise in case it throws an error, that can be done by altering the condition inside shouldNodeRejectPromise : https://github.com/nightwatchjs/nightwatch/blob/b4e273a5fc86a3f8b47eee28e6488ca8a942f1e5/lib/core/asynctree.js#L95

garg3133 commented 6 months ago

@stimothy Just to make sure that your issue is also solved with this issue, does your test work fine if you chain the waitUntil command with some other command, like below?

waitForServicesToLoad: async function () {
        const page = this;
        await this.waitForElementVisible(
            'xpath',
            '@servicesContainer'
        );
        await this.api.waitUntil(async function () {
            const val = await page.getText('xpath', '@servicesContainer');
            console.log('Test 1 ' + val);
            return val !== 'Loading...';
        }).getTitle();  // <-- change here (chained a getTitle() command which shouldn't have any effect on the test)
        console.log('Test 2 ' + await this.getText('xpath', '@servicesContainer'));
    },

If not, would you mind sharing the verbose logs for the test run (after chaining the getTitle() command)?