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

[Question] context.waitForEvent('page') not working on Github Actions #16459

Closed
canopy-js-user opened this issue Aug 11, 2022 · 17 comments
Closed

Comments

@canopy-js-user
Copy link

canopy-js-user commented Aug 11, 2022

I have Playwright tests that perform an action that opens a new tab, and then assert on the properties of the new tab.

The test waits for context.waitForEvent('page'), and is getting a timeout on Github Actions.

The code is the head of this repo: https://github.com/canopy-js/canopy-js

The tests that are failing are eg https://github.com/canopy-js/canopy-js/blob/main/playwright/navigation.spec.js#L62

Here is an example of a failing build: https://github.com/canopy-js/canopy-js/actions/runs/2835914632

However, when I run these same tests locally (npx playwright test), there are no errors.

Any ideas what might be happening?

Thanks

@rwoll
Copy link
Member

rwoll commented Aug 11, 2022

Thanks for the links and pointers.

I ran in a GitHub codespace, and it looks like it fails in headless, but passes in headful for 2/3 of the browsers:

 $ xvfb-run npx playwright test --headed
  1 failed
    [firefox] › navigation.spec.js:57:8 › Navigation › Meta-enter on global link opens new tab to redirected path 
  2 passed (32s)

Do you have any special key handlers in your app, or are you testing browser's built-in behavior?

@rwoll rwoll added the triaging label Aug 11, 2022
@canopy-js-user
Copy link
Author

canopy-js-user commented Aug 11, 2022

Hi there @rwoll thanks for the quick response.

I am testing special key handlers in the app. Given I would assume the Playwright browsers to be the same locally and on Github Actions, do you know of a reason that would make a difference?

Also, I see many more tests failing on Github actions (having tried both headed and headless) so not sure if that is working the same as your environment.

Thanks so much!

@rwoll
Copy link
Member

rwoll commented Aug 11, 2022

I'm not sure off the top of my head, and further investigation will be needed. Can you link us to the code where you are handling the keyboard events?

@canopy-js-user
Copy link
Author

canopy-js-user commented Aug 11, 2022

Sure, thanks @rwoll

A metafunction for defining key handlers is defined here: https://github.com/canopy-js/canopy-js/blob/main/client/keys/register_key_listeners.js

The actual handler functions are defined here: https://github.com/canopy-js/canopy-js/blob/main/client/keys/key_handlers.js

Additionally it seems like some of the failing tests might be click handlers defined here: https://github.com/canopy-js/canopy-js/blob/main/client/render/click_handlers.js

It seems the common denominator for all the failing tests is that the user has pressed a key / clicked an element which triggered a handler function which is opening a new tab with code like this:

https://github.com/canopy-js/canopy-js/blob/main/client/render/click_handlers.js#L16

The tests are triggering these clicks / key presses, and then waiting for the new tab to be created, eg:

https://github.com/canopy-js/canopy-js/blob/main/playwright/navigation.spec.js#L61

const [newPage] = await Promise.all([
      context.waitForEvent('page'),
      page.locator('body').press('Meta+Enter')
    ]);
await newPage.waitForLoadState();

which works locally but not on Github actions it seems.

Thanks for the help

@rwoll
Copy link
Member

rwoll commented Aug 11, 2022

  1. Please see the end of this message for a patch that should help you get your CI pipelines green while the underlying issue is investigated.
  2. It's not clear to me if your meta key handlers should work on Linux (which is what GHA is currently set to in your test.yml). As a debugging step, I recommend setting your GHA runner to match the OS you are running locally (at least mac vs. windows vs. linux) to see if that impacts the results. I'd also perform the test manually in each of the stock browsers to see what their behavior is and if it diverges from PW.

Please try (2) and let us know what you find. If you're able to provide a minimal repro, i.e. start with a blank directory, do npm init playwright and add the minimal amount of code that reproduces the issue, we'll likely be able to investigate more efficiently.

diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index f6b5df6..e52724d 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -24,7 +24,7 @@ jobs:
     - name: Run regular tests
       run: npm run jest
     - name: Run Playwright tests
-      run: npx playwright test
+      run: xvfv-run -- npx playwright test
     - uses: actions/upload-artifact@v2
       if: always()
       with:
diff --git a/playwright/navigation.spec.js b/playwright/navigation.spec.js
index c31026e..5e4628c 100644
--- a/playwright/navigation.spec.js
+++ b/playwright/navigation.spec.js
@@ -1,5 +1,7 @@
 const { test, expect } = require('@playwright/test');
 
+test.use({ headless: false });
+
 test.beforeEach(async ({ page }) => {
   page.on("console", (message) => {
     if (message.type() === "error") {
@@ -54,7 +56,9 @@ test.describe('Navigation', () => {
     await expect(page.locator('h1')).toHaveText('New York');
   });
 
-  test('Meta-enter on global link opens new tab to redirected path', async ({ page, context }) => {
+  test('Meta-enter on global link opens new tab to redirected path', async ({ page, context, browserName }) => {
+    test.fixme(browserName === 'firefox', 'https://github.com/microsoft/playwright/issues/16459');
+    
     await page.goto('/United_States/New_York');
     await expect(page.locator('.canopy-selected-link')).toHaveText('New York');
 
@@ -200,7 +204,8 @@ test.describe('Navigation', () => {
     await expect(page).toHaveURL('New_York#Southern_border');
   });
 
-  test('Pressing Meta+enter on local redirects to new tab with same path', async ({ page, context }) => {
+  test('Pressing Meta+enter on local redirects to new tab with same path', async ({ page, context, browserName }) => {
+    test.fixme(browserName === 'firefox', 'https://github.com/microsoft/playwright/issues/16459');
     await page.goto('/United_States/New_York#Southern_border');
     await expect(page.locator('.canopy-selected-link')).toHaveText('southern border');
 
@@ -216,7 +221,8 @@ test.describe('Navigation', () => {
     await expect(newPage).toHaveURL('United_States/New_York#Southern_border');
   });
 
-  test('Pressing alt-meta-enter on local zooms to lowest path segment', async ({ page, context }) => {
+  test('Pressing alt-meta-enter on local zooms to lowest path segment', async ({ page, context, browserName }) => {
+    test.fixme(browserName === 'firefox', 'https://github.com/microsoft/playwright/issues/16459');
     await page.goto('/United_States/New_York#Southern_border');
     await expect(page.locator('.canopy-selected-link')).toHaveText('southern border');
 
@@ -417,7 +423,8 @@ test.describe('Navigation', () => {
     await expect(newPage).toHaveURL('United_States/New_York#Southern_border/New_Jersey#Northern_border');
   });
 
-  test('Meta-alt-clicking on an import opens to the import path', async ({ page, context }) => {
+  test('Meta-alt-clicking on an import opens to the import path', async ({ page, context, browserName }) => {
+    test.fixme(browserName === 'firefox', 'https://github.com/microsoft/playwright/issues/16459');
     await page.goto('/United_States/New_York#Southern_border');
     await expect(page.locator('.canopy-selected-link')).toHaveText('southern border');
 
@@ -434,7 +441,8 @@ test.describe('Navigation', () => {
     await expect(newPage).toHaveURL('New_Jersey#Northern_border');
   });
 
-  test('Meta-enter on an import opens to the import path', async ({ page, context }) => {
+  test('Meta-enter on an import opens to the import path', async ({ page, context, browserName }) => {
+    test.fixme(browserName === 'firefox', 'https://github.com/microsoft/playwright/issues/16459');
     await page.goto('/United_States/New_York#Southern_border');
     await expect(page.locator('.canopy-selected-link')).toHaveText('southern border');
     await page.locator('body').press('ArrowDown');

@canopy-js-user
Copy link
Author

Ok great, thanks for the help @rwoll

Can you think of a reason why the meta key in a browser would work differently on linux? How do they open new tabs?

Hopefully I can take a look at this tomorrow morning and try to reproduce.

Thanks again

@rwoll
Copy link
Member

rwoll commented Aug 11, 2022

Off the top of my head, nope—and maybe there's no difference. I use Linux, but mostly only for non-GUI things!

@aslushnikov Can you skim through this and see if anything jumps out to you?

@canopy-js-user
Copy link
Author

@rwoll Maybe this is related? I'll look into it tomorrow #12168

@rwoll
Copy link
Member

rwoll commented Aug 11, 2022

@canopy-js-user Good find—yes, that looks very related. Let me know what you find! If needed you can use the browserName and os.platform to branch your logic in the test and issue different keys depending on OS. If you only care about working on certain platforms, you can use GHA Runner that matches the target platform instead of the Linux runner.

@canopy-js-user
Copy link
Author

@rwoll Ok great, I'll go ahead with this approach and let you know how it works. Thanks again!

@canopy-js-user
Copy link
Author

canopy-js-user commented Aug 14, 2022

Hi @rwoll

So the plot thickens.

I put logging in the key handlers and it seems like cross-platform differences in the meta key wasn't the issue.

I have this line in a spec:

    const [newPage] = await Promise.all([
      context.waitForEvent('page'),
      page.locator('body').press(`Meta+Enter`),
    ]);

And I put in the handler function logging to log what key strokes are registered. (The code converts the key codes into a string like "meta-enter")

The tests works on chromium and webkit, but fails in Firefox.

In chromium and webkit I see the following logs:

meta-undefined
meta-enter

ie, the meta key is being pressed by itself, and then the enter key is pressed, triggering the combined shortcut "meta-enter"

However, for Firefox, I see the following logs:

meta-undefined
enter

Ie, the meta key is being pressed, then released, and then the enter key is being pressed, but never do we register both being pressed at the same time.

Do you have any idea why specifically in Linux + Firefox the key press combination code might work differently? Is there any way I can force a simultaneous key press?

I've tried using keyboard.down instead of keyboard.press but it doesn't make a difference.

Thanks

@rwoll
Copy link
Member

rwoll commented Aug 14, 2022

I'm not sure, and we're about to switchover triaging duties, so I suggest creating a minimal repro and re-filing so it can be reviewed with just the relevant code. (i.e. in a fresh project, run npm init @playwright/test, and then add the absolute minimal amount of code that illustrates the issue you are facing.)

It may end up still being same/dupe of #16459 (comment), but without a minimal repro we'll be unable to follow the full context to help properly.

Thanks!

/cc @aslushnikov

@rwoll rwoll closed this as completed Aug 14, 2022
@canopy-js-user
Copy link
Author

@rwoll ok thanks for your help!

@nico-olivares
Copy link

Hey guys. I stumbled upon this thread trying to solve the same issue. The thread didn't solve it for me, but it pointed me in the right direction.
Here is my solution to the problem. Now my tests run fine in mac and CI (linux).

let macOS = process.platform === 'darwin' //darwin is macOS
let [newTab] = await Promise.all([
	context.waitForEvent('page'),
	macOS
		? link.click({ modifiers: ['Meta'] })
		: link.click({ modifiers: ['Control'] }),
])

@canopy-js-user
Copy link
Author

canopy-js-user commented Sep 9, 2022

Very nice, I did something similar:

// before the tests

const os = require('os');
let platform = os.platform();
let systemNewTabKey;
if (platform === 'darwin') {
  systemNewTabKey = 'Meta';
} else {
  systemNewTabKey = 'Control';
}

// in the test

page.keyboard.press(`${systemNewTabKey}+Enter`)

@nico-olivares

This comment was marked as resolved.

@canopy-js-user
Copy link
Author

@nico-olivares Good point, will edit for future readers

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

No branches or pull requests

4 participants