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

feat: add api spec test infrastructure #9356

Merged
merged 75 commits into from
Jun 26, 2024
Merged

Conversation

tmashuang
Copy link
Contributor

@tmashuang tmashuang commented Apr 22, 2024

Description

This PR adds tests using api-specs via the @open-rpc/test-coverage tool.

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2238

Manual testing steps

  1. yarn setup
  2. yarn test:e2e:ios:debug:build
  3. yarn test:api-specs

Screenshots/Recordings

Screenshot 2024-06-18 at 3 25 26 PM
image

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • 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 format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). 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.

Copy link
Contributor

github-actions bot commented Apr 22, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Apr 22, 2024
@tmashuang
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@tmashuang tmashuang force-pushed the e2e-json-rpc-coverage-tool branch from 25b23ca to 5f0d013 Compare May 15, 2024 17:15
Copy link

socket-security bot commented May 17, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: npm/@mantine/hooks@7.8.0, npm/@open-rpc/server-js@1.9.3, npm/@open-rpc/test-coverage@2.2.2, npm/json-schema-ref-parser@6.1.0

View full report↗︎

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

@shanejonas shanejonas force-pushed the e2e-json-rpc-coverage-tool branch 2 times, most recently from 3a3fb51 to 96a9a31 Compare June 3, 2024 21:06
index.js Outdated Show resolved Hide resolved
@shanejonas shanejonas marked this pull request as ready for review June 5, 2024 20:40
@shanejonas shanejonas requested a review from a team as a code owner June 5, 2024 20:40
@cortisiko
Copy link
Member

@shanejonas @adonesky1 I take it that we intend on including this as part of our test pipeline? If so, we should add the tests to the smoke pipeline. So that you know, our CI platform (bitrise) operates slightly differently than circleCI.

Here is a visual representation of what our smoke pipeline looks like on bitrise:
Screen Shot 2024-06-05 at 4 35 45 PM

Now here are the steps we can do for now just as a proof of concept and iterate in the future:

Feel free to reach out if anything.

Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good stuff @shanejonas I left a few comments. Sharing https://github.com/MetaMask/contributor-docs/blob/main/docs/e2e/mobile-e2e-guidelines.md#step-2-create-the-networks-page-object in case you want a quick run down on how we create page objects within our tests!

e2e/api-specs/json-rpc-coverage.js Outdated Show resolved Hide resolved
e2e/api-specs/json-rpc-coverage.js Outdated Show resolved Hide resolved
e2e/api-specs/json-rpc-coverage.js Outdated Show resolved Hide resolved
e2e/api-specs/json-rpc-coverage.js Outdated Show resolved Hide resolved
e2e/api-specs/json-rpc-coverage.js Outdated Show resolved Hide resolved
e2e/api-specs/json-rpc-coverage.js Outdated Show resolved Hide resolved
@shanejonas shanejonas requested a review from a team as a code owner June 6, 2024 16:42
@shanejonas shanejonas force-pushed the e2e-json-rpc-coverage-tool branch from 49954d7 to 9f23ce6 Compare June 6, 2024 18:07
@@ -56,6 +56,7 @@
"test": "yarn test:unit && yarn test:e2e",
"test:unit": "jest ./app/ ./locales/",
"test:unit:update": "time jest -u ./app/",
"test:api-specs": "detox test -c ios.sim.apiSpecs",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getting this error when running things for the first time using just detox test -c ios.sim.apiSpecs

Failed to find the app binary at:
/Users/jiexi/Projects/metamask-mobile/ios/build/Build/Products/Debug-iphonesimulator/MetaMask.app

@Cal-L
Copy link
Contributor

Cal-L commented Jun 18, 2024

@shanejonas nice work on this! Just curious, were you ever able to get the mock server working?

@shanejonas shanejonas requested a review from a team as a code owner June 18, 2024 18:24
@shanejonas
Copy link
Contributor

@Cal-L mock server stuff is working now, but an unrelated android e2e suite is failing.

@cortisiko cortisiko requested a review from a team as a code owner June 21, 2024 17:54
bitrise.yml Outdated Show resolved Hide resolved
@shanejonas shanejonas changed the title E2e json rpc coverage tool feat: add api spec test infrastructure Jun 24, 2024
@shanejonas
Copy link
Contributor

@SocketSecurity ignore npm/json-schema-ref-parser@6.1.0
@SocketSecurity ignore npm/@mantine/hooks@7.8.0
@SocketSecurity ignore npm/@open-rpc/server-js@1.9.3
@SocketSecurity ignore npm/@open-rpc/test-coverage@2.2.2

@cortisiko cortisiko self-requested a review June 24, 2024 20:08
@cortisiko cortisiko added the Run Smoke E2E Triggers smoke e2e on Bitrise label Jun 24, 2024
Copy link
Contributor

github-actions bot commented Jun 24, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 576cb10
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/ee977b50-594a-488d-8dfd-52eddb5421be

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

cortisiko
cortisiko previously approved these changes Jun 24, 2024
Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌮 🌮 🌮 Looks good!

tommasini
tommasini previously approved these changes Jun 25, 2024
Copy link
Contributor

@tommasini tommasini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shanejonas shanejonas dismissed stale reviews from tommasini and cortisiko via 9d9c2d9 June 26, 2024 14:16
Copy link

Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@shanejonas shanejonas merged commit 94d146e into main Jun 26, 2024
31 checks passed
@shanejonas shanejonas deleted the e2e-json-rpc-coverage-tool branch June 26, 2024 16:18
@github-actions github-actions bot locked and limited conversation to collaborators Jun 26, 2024
@metamaskbot metamaskbot added the release-7.27.0 Issue or pull request that will be included in release 7.27.0 label Jun 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
external-contributor release-7.27.0 Issue or pull request that will be included in release 7.27.0 Run Smoke E2E Triggers smoke e2e on Bitrise
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants