Conversation
…/appium-improvements
__device-tests__/helpers/utils.js
Outdated
| .toString() | ||
| .replace( /^\s+|\s+$/g, '' ); | ||
| if ( ! isNaN( androidVersion ) ) { | ||
| desiredCaps.platformVersion = androidVersion; |
There was a problem hiding this comment.
What's the result of this if there are multiple running Android devices with different versions? Do you think it's a good idea to accept maybe an ENV variable for this? I was also looking into the use of the emulator cli for firing up the Android emulator if it's not running? WDYT?
There was a problem hiding this comment.
Also another thing to note is that the specification of the Android platform version is more a requirement for the CI environment, locally if this isn't specified it'll just use the first running one it picks up
There was a problem hiding this comment.
So you mean we could just clear it?
desiredCaps.platformVersion = undefined;
There was a problem hiding this comment.
Well, I think that'll still have the key in there and I'm not actually sure how it'd respond to that. delete desiredCaps.platformVersion would be better if we want it to just pick up any running emulator.
| let initialData = this.props.initialData; | ||
| let initialTitle = this.props.initialTitle; | ||
| if ( initialData === undefined ) { | ||
| if ( initialData === undefined && __DEV__ ) { |
There was a problem hiding this comment.
Yep, so just to be clear, doing so we don't want metro running during our e2e tests and instead we bundle a "production" version of the app (without minification to understand the stacktrace) that we install on our device when running local test (using yarn test:e2e:android:local for instance) or remotely on saucelabs using the circle ci script and running yarn test:e2e:android.
Generated by 🚫 dangerJS |
|
Should be good to go now, @JavonDavis can you give it a go :)? |
|
@Tug can you push an update to the README as well? |
|
@Tug running locally with |
There was a problem hiding this comment.
Thanks for the improvements here @Tug especially replacing the file switching hack with the __DEV__ environment variable. I push 2 commits that update the local app path to point to the new build location and also to bundle the ios app when building so that a new main.jsbundle is generated.
This looks good to me 👍🏽 I guess my only concern would be the need to stop the package manager before running the tests locally seems like a chore but it's better than copying over files 😅
Can you just update the README and then after that I think we can go ahead and merge
| // Local App Paths | ||
| const defaultAndroidAppPath = './android/app/build/outputs/apk/debug/app-debug.apk'; | ||
| const defaultIOSAppPath = './ios/build/gutenberg/Build/Products/Debug-iphonesimulator/gutenberg.app'; | ||
| const defaultIOSAppPath = './ios/build/gutenberg/Build/Products/Release-iphonesimulator/gutenberg.app'; |
There was a problem hiding this comment.
Changed to the Release folder since that's where the app is being built now for local runs as well
|
@Tug I'm just noticing the zip command in the build:ios step doesn't produce the zip you'd expect and as a result I think it's leading to some error on SauceLabs... When you unzip the file you'll see the Gutenberg.app as expected but when that's unpackaged the contents are incorrect. |
This PR brings some improvements to our Appium/e2e tests.
It bundles a test version of the app that it uses for testing instead of relying on a metro server running.
Doing so we are closer to a production build and we don't need a hack on the initial html part. We can just rely on the
__DEV__global variable that RN has.It also does a minor refactoring on how the test suite is run and remove one more dependency with an external script,
appium, by installing it as a dev dependency and running it as an attached child process.