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

test(e2e): set up tests on mainnet #6229

Open
wants to merge 52 commits into
base: main
Choose a base branch
from
Open

test(e2e): set up tests on mainnet #6229

wants to merge 52 commits into from

Conversation

MuckT
Copy link
Collaborator

@MuckT MuckT commented Nov 14, 2024

Description

  • Moves E2E testing to mainnet.
  • Removes Verify.spec & FiatConnectTransferOut.spec.js.
  • Moves secret management from GH to GCP.

Test plan

To test locally run populate the e2e/example.env and rename for e2e/.env.

  • Tested locally on iOS & Android
  • Tested CI on iOS and on Android

Related issues

Backwards compatibility

Yes

Network scalability

Yes

Copy link

codecov bot commented Nov 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.92%. Comparing base (45363d2) to head (b410f43).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6229      +/-   ##
==========================================
- Coverage   88.93%   88.92%   -0.01%     
==========================================
  Files         737      737              
  Lines       31432    31432              
  Branches     5531     5839     +308     
==========================================
- Hits        27955    27952       -3     
+ Misses       3432     3280     -152     
- Partials       45      200     +155     
Files with missing lines Coverage Δ
src/analytics/AppAnalytics.ts 67.92% <100.00%> (ø)

... and 66 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 45363d2...b410f43. Read the comment docs.

---- 🚨 Try these New Features:

e2e/src/Verify.spec.js Outdated Show resolved Hide resolved
@MuckT MuckT marked this pull request as ready for review November 16, 2024 00:09
e2e/scripts/utils.ts Outdated Show resolved Hide resolved
.github/workflows/e2e-android.yml Show resolved Hide resolved
.github/workflows/e2e-android.yml Show resolved Hide resolved
e2e/scripts/fund-e2e-accounts.ts Outdated Show resolved Hide resolved
e2e/scripts/check-e2e-wallet-balance.ts Show resolved Hide resolved
// verification step comes after restoring wallet, skip this step
await waitForElementId('PhoneVerificationSkipHeader')
await element(by.id('PhoneVerificationSkipHeader')).tap()
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we expect wallets to have no funds, or for PN verification to not be required? I think we'll have to skip PN verification every time, no? Should that be within a try/catch, or should we continue to fail if the attempt to skip it errors? Also when would we expect the "no funds" screen to appear? Shouldn't these wallets always have something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The wallet associated with WALLET_ADDRESS is verified, so it's expected there will not be a phone verification request. The wallet associated with WALLET_12_WORDS_ADDRESS does not have any funds, as no tests require this wallet to be funded. Given that the main focus of these tests is that we restore with 12- and 24-word mnemonics, these conditional skips seem fine to me.

.github/workflows/e2e-android.yml Outdated Show resolved Hide resolved
e2e/src/usecases/Assets.js Outdated Show resolved Hide resolved
e2e/example.env Outdated Show resolved Hide resolved
e2e/README.md Show resolved Hide resolved
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

Successfully merging this pull request may close these issues.

3 participants