grafana / xk6-browser

k6 module that adds support for browser automation and end-to-end web testing via the Chrome Devtools Protocol
https://grafana.com/docs/k6/latest/javascript-api/k6-experimental/browser/
GNU Affero General Public License v3.0
341 stars 42 forks source link

`waitFor` on `Locator` errors when waiting on `detached` #736

Closed ankur22 closed 1 year ago

ankur22 commented 1 year ago

Brief summary

We have a scenario where we're trying to wait for a given selector to be attached then detached in succession; the application is a SPA.

Has there been any progress into https://github.com/grafana/xk6-browser/issues/472? Waiting for the locator to be attached appears to be working just fine, but waiting for detached always throws some sort of error.

Tested against: dbede12

The waitFor method on locator only seems to work with the visible state.

In the below code it works against localhost:8080, which is a test server that can be found here with instructions on how to run it locally: https://github.com/ankur22/testserver.

import { sleep } from 'k6';
import launcher from 'k6/x/browser';

export default function () {
  const browser = launcher.launch('chromium', {
    headless: false,
  });
  const context = browser.newContext();
  const page = context.newPage();
  const res = page.goto('http://localhost:8080/other');
  const l = page.locator('#input-text-hidden');
  l.waitFor({
    state: 'hidden',
  });
  const l2 = page.locator('#input-text-test');
  l2.fill('wait complete');

  sleep(5);
  browser.close();
}

In the test above against the test page, the expected output is that wait complete is filled in one of the input text boxes, but this doesn't happen and instead times out.

xk6-browser version

https://github.com/grafana/xk6-browser/commit/dbede120c63df43995813a847a25b0e66e289592

OS

Unknown

Chrome version

Unknown

Docker version and image (if applicable)

NA

Steps to reproduce the problem

See summary

Expected behaviour

Waiting for the locator to be detached works.

Actual behaviour

Waiting for the locator to be detached returns an error.

inancgumus commented 1 year ago

@ankur22, the test script is for an old version:

  const browser = launcher.launch('chromium', {
    headless: false,
  });

Shouldn't it be the following?

import { chromium } from 'k6/x/browser';
...
const browser = chromium.launch({
  headless: false,
});

With the script fixed, I tested this using the testserver and received the following error:

ERRO[0030] GoError: waiting for "#input-text-hidden": websocket: close 1006 (abnormal closure): unexpected EOF
        at reflect.methodValueCall (native)
        at file:///Users/inanc/grafana/k6browser/tests/issue736.js:12:12(32)

Is this the error you mentioned in the issue?


Then I used page instead of locator:

  ...
  const res = page.goto('http://localhost/other'); // server runs on port 80 - i checked it
  page.waitForSelector('#input-text-hidden', {
    state: 'hidden',
  });
  page.fill('#input-text-test', 'wait complete');
  ...

And received a similar error:

ERRO[0030] GoError: waiting for selector "#input-text-hidden": waiting for selector "#input-text-hidden" did not result in any nodes
        at github.com/grafana/xk6-browser/api.Page.WaitForSelector-fm (native)
        at file:///Users/inanc/grafana/k6browser/tests/issue736.js:11:23(28)

WDYT? It seems like it's not a locator error (again) and rather an underlying error in page.


By the way, is this issue related to #472 somehow?

ka3de commented 1 year ago

Could we clarify what was the original problem in this issue? Seems like the code exposed through the /other handler in the test server has changed and now it's not clear to me what was the original problem. As of now it seems to me this is a duplicate of #472 , as in my initial analysis, the test times out when waiting for the locator on #input-text-hidden with state: hidden .

This is the test that I'm running, updated based on latest version syntax and changes done to the test server:

import { sleep } from 'k6';
import { chromium } from 'k6/x/browser';

export default function () {
    const browser = chromium.launch({
        headless: false,
    });

    const context = browser.newContext();
    const page = context.newPage();
    const res = page.goto('http://localhost:8080/other');

    const l = page.locator('#input-text-hidden');
    l.waitFor({ state: 'hidden' });
    const l2 = page.locator('#text1');
    l2.fill('wait complete');

    sleep(5);
    browser.close();
}

Based on this, the behavior experienced is test TO waiting for the first locator:

ERRO[0030] communicating with browser: websocket: close 1006 (abnormal closure): unexpected EOF  category=cdp elapsed="0 ms" goroutine=21
ERRO[0030] GoError: waiting for "#input-text-hidden": websocket: close 1006 (abnormal closure): unexpected EOF
running at github.com/grafana/xk6-browser/api.Locator.WaitFor-fm (native)
default at file:///<redacted>/script.js:14:14(32)  executor=per-vu-iterations scenario=default source=stacktrace

Which seems same behavior as in #472 .

ankur22 commented 1 year ago

I think the original description is confusing. The scope of this issue should focus on the ability to waitFor an element to be detached from the DOM. The reference to hidden/visible state is misleading, and the original description assumed that the issue could be related. My new findings are that when an element is detached, the test script will not progress when it should. In Playwright the test script does progress as expected.

  1. First retrieve the latest version of the testserver.
  2. Run the following playwright test to valid what should happen when an element is detached (NOTE: it requires a manual click on the detach button at the top of the page):

    // @ts-check
    const { test, chromium } = require('@playwright/test');
    
    test.describe('attach-detach', () => {
      test('detach', async () => {
        const browser = await chromium.launch({headless: false});
        const context = await browser.newContext();
        const page = await context.newPage();
    
        try {
          await page.goto("http://localhost/other")
          await page.locator("#attach-detach").waitFor({state: "detached"})
          console.log("detached")
        } finally {
          await page.close();
          await browser.close();
        }
      });
    });

    Once you have clicked on detach the test should print detached and end successfully.

  3. Now run the following k6-browser test (again, manually click on the detach button):

    import { chromium } from 'k6/x/browser'
    
    export default async function () {
      const browser = chromium.launch({ headless: false })
      const context = browser.newContext()
      const page = context.newPage()
    
      try {
        await page.goto('http://localhost/other', { waitUntil: 'networkidle' })
        page.locator("#attach-detach").waitFor({state: "detached"})
        console.log("detached")
      } finally {
        page.close()
        browser.close()
      }
    }

    This time the test will not exit once the detach button is pressed, and instead the test will eventually timeout.

ka3de commented 1 year ago

Thanks for the clarification and examples @ankur22 ! I was able to reproduce both tests.

ka3de commented 1 year ago

I have made some progress on this, so I will write my initial conclusions on the issue: I believe the problem is mainly in the injected_script.js, which includes the JS code to poll the DOM in order to wait for events etc.

This diff will probably explain the situation:

diff --git a/common/js/injected_script.js b/common/js/injected_script.js
index ea419f2..c85440a 100644
--- a/common/js/injected_script.js
+++ b/common/js/injected_script.js
@@ -706,7 +706,10 @@ class InjectedScript {
     let timedOut = false;
     let timeoutPoll = null;
     const predicate = () => {
-      return predicateFn(...args) || continuePolling;
+      // predicateFn will return 'undefined' when state is detected to change to 'detached'
+      // and 'hidden', making this always return 'continuePolling' if we keep the commented
+      // code (|| continuePolling)
+      return predicateFn(...args); //|| continuePolling;
     };
     if (timeout !== undefined || timeout !== null) {
       setTimeout(() => {
@@ -765,7 +768,11 @@ class InjectedScript {
           return;
         }
         const success = predicate();
-        if (success !== continuePolling) resolve(success);
+        if (success !== continuePolling) {
+          // With the previous commented code, now execution will resolve
+          // here when state is changed to 'detached'
+          resolve(success);
+        }
         else requestAnimationFrame(onRaf);
       }
     }
@@ -889,10 +896,12 @@ class InjectedScript {
         case "attached":
           return element ? element : continuePolling;
         case "detached":
+          // Here we return undefined when the element is detached
           return !element ? undefined : continuePolling;
         case "visible":
           return visible ? element : continuePolling;
         case "hidden":
+          // Here we return undefined when the element hidden
           return !visible ? undefined : continuePolling;
       }
     };

With this modification now the test is not hang on an "inifinite loop", but we still get an error like:

ERRO[0002] communicating with browser: read tcp 127.0.0.1:52808->127.0.0.1:39417: read: connection reset by peer  category=cdp elapsed="0 ms" goroutine=60
ERRO[0002] Uncaught (in promise) GoError: internal error while removing binding from page: read tcp 127.0.0.1:52808->127.0.0.1:39417: read: connection reset by peer
running at github.com/grafana/xk6-browser/api.Page.Close-fm (native)
default at file:///<redacted>/script.js:17:8(45)  executor=per-vu-iterations scenario=default
ERRO[0002] process with PID 28670 unexpectedly ended: signal: killed  category=browser elapsed="6 ms" goroutine=79

This happens exactly when the state changes, so I'm guessing this might be a mishandling of the return from the script evaluation maybe? I have to investigate this further.

With this, I believe fixing this issue will fix also #472 .