E2e appium token request flow 198#5432
Conversation
cortisiko
left a comment
There was a problem hiding this comment.
left a couple comments! Also, please run detox to ensure nothing broke.
| Feature: Request Token | ||
| This feature goes through the request token flow | ||
|
|
||
| Scenario: Import account and navigate to Request |
There was a problem hiding this comment.
We have already have step for this guy.
Given I have imported my wallet
use it instead. But this would mean you would need to update the Given step in the networks.feature file as well.
| import SendScreen from '../../features/screen-objects/SendScreen'; | ||
| import Gestures from '../../features/helpers/Gestures'; | ||
|
|
||
| Given(/^I import wallet using seed phrase "([^"]*)?"/, async (phrase) => { |
There was a problem hiding this comment.
instead of putting this in a common step. Can you put https://github.com/MetaMask/metamask-mobile/blob/main/wdio/features/step-definitions/start-exploring.js#L11 in there instead?
| Then(/^I tap on button with text "([^"]*)?"/, async (text) => { | ||
| const timeout = 1000; | ||
| await driver.pause(timeout); | ||
| await SendScreen.tapOnText(text); |
There was a problem hiding this comment.
hmmm might need a generic class for this action. I can see confusion when trying to use this step. For example, If I am on the privacy and security page and I tap on "Delete wallet". Having the class as SendScreen would cause confusion when debugging.
| await driver.pause(timeout); | ||
| await Gestures.tapTextByXpath(button); | ||
| // eslint-disable-next-line no-console | ||
| console.log('On screen ' + type + ' ' + screen); |
| await driver.pause(timeout); | ||
| await Gestures.tapTextByXpath(button); | ||
| // eslint-disable-next-line no-console | ||
| console.log('On screen ' + screen); |
| await Gestures.setValueWithoutTap(this.requestAmount, amount); | ||
| } | ||
|
|
||
| async isTextElementDisplayed(text) { |
There was a problem hiding this comment.
keep the assertions to the bottom of test actions.
| await expect(Selectors.getXpathElementByText(text)).toBeDisplayed(); | ||
| } | ||
|
|
||
| async isTextElementNotDisplayed(text) { |
There was a problem hiding this comment.
keep the assertions to the bottom of test actions.
| import MetaMetricsScreen from '../screen-objects/Onboarding/MetaMetricsScreen.js'; | ||
| import OnboardingScreen from '../screen-objects/Onboarding/OnboardingScreen.js'; | ||
| import WelcomeScreen from '../screen-objects/Onboarding/OnboardingCarousel.js'; | ||
| import OnboardingWizardModal from '../screen-objects/Modals/OnboardingWizardModal.js'; |
There was a problem hiding this comment.
small nitpicking. Can you put the imports together? For example, all imports that come from the Modals folder should be together, all imports that come from the screens folder should be together.
| import { isTestNet } from '../../../util/networks'; | ||
| import { isTokenDetectionSupportedForNetwork } from '@metamask/controllers/dist/util'; | ||
| import generateTestId from '../../../../wdio/utils/generateTestId'; | ||
| import { |
There was a problem hiding this comment.
since you adding constants for the testID strings, can you have detox import the testID's from the constants file as well? I am speaking about https://github.com/MetaMask/metamask-mobile/blob/main/e2e/pages/RequestPaymentView.js#L3
| @@ -0,0 +1,9 @@ | |||
| // eslint-disable-next-line import/prefer-default-export | |||
There was a problem hiding this comment.
In this file name, the I in testids should be uppercase.
Development & PR Process
release-xxlabel to identify the PR slated for a upcoming release (will be used in release discussion)needs-dev-reviewlabel when work is completedneeds-qalabel when dev review is completedQA Passedlabel when QA has signed offDescription
Appium test coverage.
Screenshots/Recordings
If applicable, add screenshots and/or recordings to visualize the before and after of your change
Issue
Progresses #???
Checklist