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

Fix asynctree from getting cleared in between test case. #4325

Merged
merged 6 commits into from
Dec 30, 2024

Conversation

garg3133
Copy link
Member

@garg3133 garg3133 commented Dec 8, 2024

Problem

Right now, if a test case is written using the async/await syntax, the asynctree we use inside our queueing system gets reset after every awaited Nightwatch command/assertion, after which the done() method of the queue itself is called.

But, in between the tree getting reset and the queue's done() method being called, the control goes back inside the test case itself (because the awaited command/assertion is resolved now) and it reads the following NW commands and adds them into a new queue asynctree. So, when the done() method of the queue is called after that, because of this condition which checks if the tree is empty or not, because the tree is non-empty now, nothing happens and the test case continues its normal execution.

But, when the pointer goes back to the test case between the tree getting reset and the queue done() call, instead of executing a NW command (which results in the queue tree getting filled again), if we execute a normal JS code and await it (like the setTimeout code below):

await browser.waitForElementVisible('body');

await new Promise((resolve) => {
  setTimeout(function() {
    resolve(null);
  }, 50);
});

await browser
  .assert.visible('input[name=qi]')
  .sendKeys('input[name=q', 'nightwatch');

the tree will stay empty after the execution of this code and the control will pass to the done() method of the queue.

The above situation will cause the current queue to be resolved (through the call to this.deferred.resolve()), which in turn will cause the current runnable's this.deferred.resolveFn(result) to be called. But because it is called with the result passed as argument, which represents the promise returned by the currently running test case, the current runnable does not actually get resolved but remains stuck waiting for the test case to complete and the result to be resolved.

This means that because the resolution of the current runnable is now tied to the resolution of the promise returned by the test case, the current runnable will never be resolved until the test case itself is completed and resolved.


Now, when the awaited JS code inside the test case (due to which all of the above happened) finally gets resolved and returns its result, the following NW commands/assertions will be executed and added to the same queue (but with a different this.deferred no longer tied to the current runnable) and everything will continue running normally.

So, if nothing eventful happens and the rest of the test case executes successfully, the result to which the resolution of the current runnable is tied to will resolve normally, leading to the resolution of the current runnable and everything works fine.

BUT, if there is some error or an assertion failure someplace in the remaining test case, the tree will get cleared here [, followed by the entry inside the test case which is covered below and not important here], followed by a call to the done() method of the queue and hence the call to queue's this.deferred.resolve(). But since the this.deferred of the queue is no longer tied to the current runnable, this path of the resolution/rejection of current runnable is closed now.

Now, the resolution/rejection of the current runnable depends entirely on the resolution/rejection of the test case itself.

So, if this error or failure in the currently running NW command/assertion leads to:

  • rejection of the test case (will happen if await is directly used with the command and the command promise gets rejected)
    await browser
      .click()
      .failingCommand();  // `await` is directly tied to the promise returned by the `failingCommand`.
    --> the result to which the resolution of the current runnable is tied will get rejected and the current runnable will resolve with an error.
    --> TEST WILL CONTINUE NORMALLY.
  • continuation of the test case (will happen if await is directly used with the command and the command promise gets resolved)
    await browser
      .click()
      .failingCommand();  // `await` is directly tied to the promise returned by the `failingCommand`.
    
    await browser.sendKeys();
    --> the test case will continue after the failingCommand and run the .sendKeys command.
    --> TEST WILL CONTINUE BUT ABNORMALLY.
  • the test case getting stuck (will happen if the command is chained with some other NW commands)
    await browser
      .click()
      .failingCommand()  // `await` is not tied to the promise returned by the `failingCommand`.
      .sendKeys();
    --> the test case will get stuck after the failingCommand becasue the tree is emptied after the failure and so the last sendKeys command (to which the await is tied) will never run or get resolved.
    --> TEST WILL GET STUCK OR THE PROCESS WILL EXIT ABRUPTLY.

SOLUTION

The root cause of this issue is that if a test case is written with the async/await syntax, the tree is reset after every awaited NW command/assertion, which is then followed by a call to the queue done() method. Instead of this, the tree should also be reset at the end of the current test case or hook (or in case there is an error or a failure in some command, which is covered here). On doing this, although the queue done() method will still be called after every awaited NW command/assertion, it will never be executed completely because the tree will be non-empty at that point.

This also sovles an issue when using the "Step over and pause again" feature of the .pause() command where the feature wasn't working for test cases written with async/await syntax.

Copy link

github-actions bot commented Dec 8, 2024

Status

  • ❌ No modified files found in the types directory.
    Please make sure to include types for any changes you have made. Thank you!.

@garg3133 garg3133 changed the title Abstain tree from resolving in between test case. Fix asynctree from getting cleared in between test case. Dec 28, 2024
@garg3133 garg3133 merged commit 6463376 into nightwatchjs:main Dec 30, 2024
16 of 17 checks passed
@garg3133 garg3133 deleted the fix-tree-resolution2 branch January 1, 2025 10:56
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

Successfully merging this pull request may close these issues.

1 participant