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.

Sign up for GitHub

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

Jump to bottom

Fix flaky test Sign Typed Data Signature Request can initiate and confirm a Signature Request of Sign Typed Data #19823

Closed
DDDDDanica opened this issue Jun 29, 2023 · 2 comments · Fixed by #27532
Closed

Fix flaky test Sign Typed Data Signature Request can initiate and confirm a Signature Request of Sign Typed Data #19823

DDDDDanica opened this issue Jun 29, 2023 · 2 comments · Fixed by #27532
Assignees
@pnarayanaswamy
Labels
flaky tests release-12.8.0 Issue or pull request that will be included in release 12.8.0 team-confirmations Push issues to confirmations team

Comments

@DDDDDanica
Copy link
Contributor

DDDDDanica commented Jun 29, 2023 •
edited by chloeYue
Loading

Error message:
https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/101352/workflows/fb633881-146a-4682-8fe3-d865fc6bc5f0/jobs/3773393
@DDDDDanica DDDDDanica changed the title [Flaky e2e test]: Sign Typed Data Signature Request [Flaky e2e test]: MV3 only: Sign Typed Data Signature Request Jun 29, 2023
@chloeYue chloeYue changed the title [Flaky e2e test]: MV3 only: Sign Typed Data Signature Request [Flaky e2e test]: Fix flaky test Sign Typed Data Signature Request can initiate and confirm a Signature Request of Sign Typed Data Sep 26, 2024
@chloeYue chloeYue changed the title [Flaky e2e test]: Fix flaky test Sign Typed Data Signature Request can initiate and confirm a Signature Request of Sign Typed Data Fix flaky test Sign Typed Data Signature Request can initiate and confirm a Signature Request of Sign Typed Data Sep 26, 2024
@chloeYue chloeYue mentioned this issue Sep 27, 2024
Closed
9 tasks
@pnarayanaswamy
Copy link
Contributor

pnarayanaswamy commented Oct 1, 2024 •
edited
Loading

This one flaked not due to a test issue but an issue on the CI, subsequent re-run of the test passed and the test has not flaked for over 13 days.

Error: Unable to obtain browser driver.
        For more information on how to install drivers see
        https://www.selenium.dev/documentation/webdriver/troubleshooting/errors/driver_location/. Error: Error executing command for /home/circleci/project/node_modules/selenium-webdriver/bin/linux/selenium-manager with --browser,firefox,--output,json,--browser-path,/opt/firefox/firefox: request or response body error: error reading a body from connection: stream error received: unexpected internal error encountered
  (Ran on CircleCI Node 3 of 24, Job test-e2e-firefox)
    at getPath (node_modules/selenium-webdriver/common/driverFinder.js:35:11)
    at thenableWebDriverProxy.createSession (node_modules/selenium-webdriver/firefox.js:594:45)
    at createDriver (node_modules/selenium-webdriver/index.js:152:33)
    at Builder.build (node_modules/selenium-webdriver/index.js:688:16)
    at FirefoxDriver.build (test/e2e/webdriver/firefox.js:97:28)
    at buildBrowserWebDriver (test/e2e/webdriver/index.js:41:34)
    at buildWebDriver (test/e2e/webdriver/index.js:20:13)
    at withFixtures (test/e2e/helpers.js:187:21)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Context.<anonymous> (test/e2e/tests/signature/signature-request.spec.js:68:7)

@pnarayanaswamy pnarayanaswamy mentioned this issue Oct 1, 2024
Merged
7 tasks
@pnarayanaswamy
Copy link
Contributor

The tests however needed a small improvement that resulted in better execution times and are addressed in the PR #27532

github-merge-queue bot pushed a commit that referenced this issue Nov 14, 2024
@pnarayanaswamy
test: Improve test for signatures (#27532)
8441041 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

The current mock server setup injects all mocks at the beginning of the
test, which can lead to delays. Specifically, when we assert the account
details opened metric, only the relevant endpoint for that metric gets
triggered, while other mocks, like "signature approved," may not be
invoked yet. Although this doesn’t cause the test to fail, it results in
unnecessarily long wait times during the test, as the framework waits
for mocks that haven’t been triggered.

This PR addresses the issue by moving the metric assertions to the end
of the test. By doing so, we eliminate the idle waiting period,
streamlining the test flow and reducing overall test execution time.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27532?quickstart=1)

## **Related issues**

Fixes:
[#19823](#19823)

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
@metamaskbot metamaskbot added the release-12.8.0 Issue or pull request that will be included in release 12.8.0 label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pnarayanaswamy pnarayanaswamy

Labels
flaky tests release-12.8.0 Issue or pull request that will be included in release 12.8.0 team-confirmations Push issues to confirmations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: Improve test for signatures MetaMask/metamask-extension
6 participants
@gauthierpetetin @DDDDDanica @matthewwalsh0 @pnarayanaswamy @metamaskbot @chloeYue