[RNMobile] Cache the Web Driver Agent in iOS e2e tests workflow #29831
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fluiddot
added
the
Mobile App - i.e. Android or iOS
fluiddot
changed the title
7 tasks
|
Size Change: +1.52 kB (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
|
The first run worked fine, it built the WDA and added the required capabilities for using a prebuilt WDA 🎊 . I've triggered another run to see if it caches properly the WDA and the tests pass 🤞 . |
|
The second run with the cache worked too 🎊 . |
ceyhun
reviewed
Mar 15, 2021
| @@ -140,6 +142,13 @@ const setupDriver = async () => { | |||
| } | |||
|
|
|||
| desiredCaps.app = path.resolve( localIOSAppPath ); | |||
|
|
|||
| if ( isCI ) { | |||
There was a problem hiding this comment.
I think having another code path just for local CI could further complicate things. WDYT about adding test:e2e:build-wda in test:e2e:ios:local as an additional step and removing the check for CI so both local and CI runs stay the same?
There was a problem hiding this comment.
The difference between local and CI regarding the command test:e2e:build-wda is that for CI the WDA cache gets invalidated with the build cache key so if there's any changes in the native side, the WDA is rebuilt but this won't happen for local. This could produce an error if you change for example the Xcode version without deleting the derived data folder ios/build/WDA.
For local, my idea was to let Appium decide when the WDA should be rebuilt, that's why we have another code path but I'd be happy to figure out a different approach.
There was a problem hiding this comment.
When we put test:e2e:build-wda inside test:e2e:ios:local it should also always rebuild WDA when ran locally, right? We can add a rm -rf ios/build/WDA in test:e2e:build-wda to be safe, but it seems to me that it's the same case for building with test:e2e:build-app:ios locally and I don't think it produces an error, so maybe even the rm -rf isn't necessary.
There was a problem hiding this comment.
Just a note here if we go the rm -rf ios/build/WDA to utilize https://www.npmjs.com/package/rimraf for cross-platform compatibility. If it's not needed here then please do ignore 🙇🏾
There was a problem hiding this comment.
When we put test:e2e:build-wda inside test:e2e:ios:local it should also always rebuild WDA when ran locally, right? We can add a rm -rf ios/build/WDA in test:e2e:build-wda to be safe, but it seems to me that it's the same case for building with test:e2e:build-app:ios locally and I don't think it produces an error, so maybe even the rm -rf isn't necessary.
Yeah you're right, we can run the command test:e2e:build-wda when running test:e2e:ios:local. I thought that the workflow was running test:e2e:ios:local but it's not, sorry for the misleading. Since this command is only run locally, it makes sense to also build the WDA. This would also require to pass the WDA derived data folder as an environment variable when running it from Gutenberg-mobile, as we do with the app’s path but nothing else.
Regarding removing the derived data folder, I'm going to check it but most likely it won't be necessary.
Thanks for the feedback @ceyhun and @jd-alexander 🙇 !
There was a problem hiding this comment.
I applied the changes from these comments in this commit. I'm going to open a Gutenberg-mobile PR with the required changes to run the e2e tests there too.
There was a problem hiding this comment.
Here is the PR in Gutenberg-mobile related to these changes, @ceyhun and @jd-alexander I've assigned you as reviewers there too.
ceyhun
reviewed
Mar 15, 2021
2 tasks
ceyhun
approved these changes
Mar 16, 2021
There was a problem hiding this comment.
Tested locally running npm run native test:e2e:ios:local and it worked great! After that I also deleted WebDriverAgentRunner-Runner.app from the simulator and ran TEST_RN_PLATFORM=ios npm run native device-tests:local and it was installed again without building and the tests kept working 🎉
On the last iOS E2E tests on CI I saw that the app and WDA were built then I re-triggered and both builds were skipped as expected and the test passed. Downloaded appium logs and screen recordings and they also look good 🚀🚀🚀
jd-alexander
approved these changes
Mar 16, 2021
There was a problem hiding this comment.
Thanks for these changes @fluiddot This is a great improvement to CI pipeline 🚀
- Observed that the Build Web Driver Agent step didn't build the WDA if it's cached.
- Observed that the usePrebuiltWDA capability is logged in the Appium logs zip file after checking the downloaded zip file with TextEdit.
- Observed that all the e2e tests passed.
LGTM 🚢
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
gutenberg-mobilePR: wordpress-mobile/gutenberg-mobile#3266Description
After applying the fix from the PR for the iOS native e2e tests, we came up with an idea for reducing the build time (related comment).
The WDA (Web Driver Agent) is built every time the e2e tests are executed so it would be possible to cache it to save time.
This PR adds the following items:
How has this been tested?
Verify that e2e tests run locally
npm run native test:e2e:ios:local.Verify that e2e tests run on CI and cache the WDA
React Native E2E Tests (iOS)PR check (last run).Build Web Driver Agentstep doesn't build the WDA if it's cached.usePrebuiltWDAcapability is logged in the Appium logs (the logs can be taken from the run artifacts).Screenshots
N/A
Types of changes
New feature
Checklist: