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

Tests failing with chrome #53

Closed
rabi-siddique opened this issue Nov 14, 2024 · 3 comments · Fixed by #54
Closed

Tests failing with chrome #53

rabi-siddique opened this issue Nov 14, 2024 · 3 comments · Fixed by #54
Assignees
Labels
bug Something isn't working

Comments

@rabi-siddique
Copy link
Collaborator

rabi-siddique commented Nov 14, 2024

Recently E2E tests have been failing within the wallet app:
https://github.com/Agoric/wallet-app/actions/runs/11828028790
https://github.com/Agoric/wallet-app/actions/runs/11807920901

The very first test case is failing which involves setting up the Keplr wallet. The first test case is very important as all the other tests depend on it. Due to this majority of tests will fail in the CI run. This was shocking because we have other dapps such as dapp-psm, dapp-inter etc and none of those dapps failed the test case related to setting up the Keplr wallet.

@agoric/synpress is failing on this line. And the Cypress test runner shows the following stack trace:

TypeError: Cannot read properties of undefined (reading 'bringToFront')
    at Object.switchToKeplrWindow (/Users/rabisiddique/Desktop/Work/wallet-app/node_modules/@agoric/synpress/commands/playwright-keplr.js:51:23)
    at getWalletAddress (/Users/rabisiddique/Desktop/Work/wallet-app/node_modules/@agoric/synpress/commands/keplr.js:354:16)
    at invoke (/Users/rabisiddique/Library/Caches/Cypress/12.17.3/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:246:16)
    at /Users/rabisiddique/Library/Caches/Cypress/12.17.3/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/util.js:59:14
    at tryCatcher (/Users/rabisiddique/Library/Caches/Cypress/12.17.3/Cypress.app/Contents/Resources/app/node_modules/bluebird/js/release/util.js:16:23)
    at Promise.attempt.Promise.try (/Users/rabisiddique/Library/Caches/Cypress/12.17.3/Cypress.app/Contents/Resources/app/node_modules/bluebird/js/release/method.js:39:29)
    at Object.wrapChildPromise (/Users/rabisiddique/Library/Caches/Cypress/12.17.3/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/util.js:58:23)
    at RunPlugins.taskExecute (/Users/rabisiddique/Library/Caches/Cypress/12.17.3/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:252:10)
    at RunPlugins.execute (/Users/rabisiddique/Library/Caches/Cypress/12.17.3/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:166:21)
    at EventEmitter.<anonymous> (/Users/rabisiddique/Library/Caches/Cypress/12.17.3/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:56:12)
    at EventEmitter.emit (node:events:517:28)
    at process.<anonymous> (/Users/rabisiddique/Library/Caches/Cypress/12.17.3/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/util.js:33:22)
    at process.emit (node:events:517:28)
    at emit (node:internal/child_process:944:14)
    at process.processTicksAndRejections (node:internal/process/task_queues:83:21)

Quick Fix

We can switch to using the Chromium browser in CI like other dapps. This will address the issue for us in CI. Such a solution is implemented in Agoric/wallet-app#199

Impact of the Issue

For CI, we can use the Chromium browser, similar to other dapps. But, for local testing, we have two options:

  1. Resolve this issue to ensure compatibility with both Chrome and Chromium.
  2. Decide not to address the issue and update our READMEs to specify that Chromium is required for testing, which must be installed prior to running tests.
@rabi-siddique
Copy link
Collaborator Author

Problematic section of code is the assignWindows function:
https://github.com/agoric-labs/synpress/blob/bc88761cf4655ac3c7fb3731f37178cae4fd72fa/commands/playwright-keplr.js#L141C1-L158C5

When setupWallet is called, it internally invokes assignWindows to configure browser windows. Essentially, it reads the currently open browser tabs, examines their URLs, and, based on specific criteria, assigns those tabs to designated variables. One of these variables is keplrWindow.

This issue arises when keplrWindow is not properly assigned or initialized. Because of this the TypeError: Cannot read properties of undefined (reading 'bringToFront') error occurs because keplrWindow is undefined when the code attempts to call its bringToFront method.

@rabi-siddique
Copy link
Collaborator Author

rabi-siddique commented Nov 14, 2024

Narrowing down further this check has started to fail:
https://github.com/agoric-labs/synpress/blob/bc88761cf4655ac3c7fb3731f37178cae4fd72fa/commands/playwright-keplr.js#L149C1-L156C1

I noticed two issues:

  • Keplr registration URL has a # at the end.
    image

  • The registration URL checked in the code is deformed. Has extra spaces and line breaks around it:

"chrome-extension://
          dbjbkadblogdlnkiablkonbbpiaempmj
        /register.html"

The issue here is that the URL is encoding extra spaces as %20, leading to an invalid match.

@rabi-siddique
Copy link
Collaborator Author

Keplr registration URL has a # at the end.

This isn't a major issue. And is not the reason for recent failures.

@rabi-siddique rabi-siddique added the bug Something isn't working label Nov 15, 2024
@rabi-siddique rabi-siddique linked a pull request Nov 15, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant