-
Notifications
You must be signed in to change notification settings - Fork 14
Appium e2e testing setup #11
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
base: main
Are you sure you want to change the base?
Conversation
- Introduced E2E testing framework in the `e2e` directory, including configuration files and test scripts. - Added `HomeOnboardingScreen` page object for managing onboarding screen interactions. - Implemented initial test for wallet app onboarding, verifying UI elements and welcome messages. - Updated `.gitignore` to exclude E2E test artifacts and configuration files. - Modified `app.json` to disable development client for production builds. - Added `package-lock.json` and `package.json` for E2E testing dependencies.
ignaciolarranaga
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My main comments @tobi-legan (but want to hear from @jonathunne) are:
- I think you should not generate another project (i.e. another package.json to be specific) but to enhance the current one. E2E is part of the lifecycle of the app,
- I think you need to connect with Qase already, because we automate that suite
Later inside some opinionated comments.
I'm not sure why are you shipping this empty file also?: e2e/data/wallet-details-for-test.ts
| From project root: | ||
|
|
||
| ```bash | ||
| npm run e2e:build-apk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not they way it should happen, if there is a build command it should be npm run build in the main project or similar (you do not build for the e2es, you build because you need to build as part of the CI/CD, e2es are just part of the life cycle)
| Then run tests: | ||
|
|
||
| ```bash | ||
| npm test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm test:e2e I would suggest, a project might have multiple types of tests
| ├── tsconfig.json # TypeScript config | ||
| ├── package.json # Dependencies | ||
| ├── test/ | ||
| │ └── specs/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems too many levels isn't it?, but mainly related to my previous comment of I don't think you really need a project inside the other project. It seems more like e2e/*.test.ts or e2e/*.spec.ts & e2e/utils/*.ts for the utilities or similar.
@jonathunne might have a different vision though
| @@ -0,0 +1,24 @@ | |||
| { | |||
| "name": "wdk-starter-react-native-e2e", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to extend the main project rather than creating a new one. @jonathunne WDYT?
I would expect this workflow at the root:
- npm install
- npm run build
- npm test:e2e
Unless there is something I might be missing?
| timeout: 60000, | ||
| }, | ||
|
|
||
| // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove all the commented code unless you are really defining something
| paymasterToken: { | ||
| address: '0xdAC17F958D2ee523a2206206994597C13D831ec7', | ||
| }, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was unintended right?
| @@ -0,0 +1,80 @@ | |||
| # E2E Tests | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll probably recommend to add the relevant sections to the main README.md
|
|
||
| const homeOnboardingScreen = new HomeOnboardingScreen(); | ||
|
|
||
| describe('Wallet App E2E Tests', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
describe('Onboarding', () => { isn't it?
| const homeOnboardingScreen = new HomeOnboardingScreen(); | ||
|
|
||
| describe('Wallet App E2E Tests', () => { | ||
| it('should launch the standalone app and verify the create wallet button', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You must link to an exiting test case in the suite (or may be multiple ones: https://app.qase.io/project/TW) as well as introduce the connection to Qase.
So Let me explain this , and why this our case is a bit tricky and this will even also affect the type of tests that i will write if for this , this is just a couple of things to discuss and consider Again Thanks for the feedback. The separate The Core Issue: Test Data & Build ConfigurationE2E tests require the app to be built with specific wallet/network details embedded. This creates several challenges:
The separate Proposed ApproachI'm happy to merge E2E scripts and dependencies into root
The empty Qase IntegrationI haven't integrated with Qase yet. I can connect with the team to understand the existing setup and integrate WebdriverIO with Qase for test reporting/management. Next Steps
Happy to adjust based on team preferences. |
|
|
||
| docs | ||
|
|
||
| #i am ignoring this file for now so that my changes to the config file don't get pushed, since they are config changes, will remove it after |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this?
| describe('Wallet App E2E Tests', () => { | ||
| it('should launch the standalone app and verify the create wallet button', async () => { | ||
| // Wait for app to load | ||
| await driver.pause(3000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer explicit waits for elements instead of pauses.
e.g. await homeOnboardingScreen.getCreateWalletButton().waitForDisplayed({ timeout: 10000 });
| await driver.pause(3000); | ||
|
|
||
| // Verify driver session exists | ||
| const sessionId = (driver as any).sessionId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does driver have a type or do we have to use any here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete this empty file?
Description
This PR adds E2E testing infrastructure using WebdriverIO and Appium for the React Native wallet app.
Key Changes:
e2e/pageObjects/)e2e/test/specs/onboarding.e2e.ts)e2e/package.jsonfor better organizationFiles Added:
e2e/package.json- E2E dependencies and scripts (build-apk, appium, check-devices, etc.)e2e/wdio.conf.ts- WebdriverIO configuratione2e/tsconfig.json- TypeScript config for E2E testse2e/pageObjects/home-onboarding-screen.ts- Page object for onboarding screene2e/test/specs/onboarding.e2e.ts- Initial E2E teste2e/README.md- E2E testing documentationScripts Added to e2e/package.json:
build-apk- Build standalone release APK for testingbuild-ios- Build iOS app for testingappium- Start Appium servercheck-devices- Check connected Android devicescheck-ios-devices- Check iOS simulatorstest- Run E2E testsShort video of the test added:
https://jam.dev/c/43c4b354-600d-4926-be8f-f83692219bbc
Motivation and Context
Problem:
Solution:
e2e/directoryBenefits:
Related Issue
N/A - This is a new feature addition (E2E testing infrastructure)
Type of change