[Mobile] Improve Screen recording on Android for Appium 2 #55443
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
The sdcard folder should exist in the emulator, if not there's a script that will create it. For skipped tests, it will remove empty folders that weren't needed. When having multiple Android emulators opened, it will use the deviceID that Appium is currently using.
geriux
added
[Type] Automated Testing
|
Size Change: 0 B Total Size: 1.66 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 70de242. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6560589158
|
dcalhoun
reviewed
Oct 18, 2023
| // Split by line and extract the device ID from the first line (excluding the header) | ||
| const lines = adbOutput | ||
| .split( '\n' ) | ||
| .filter( ( line ) => line && ! line.startsWith( 'List' ) ); |
There was a problem hiding this comment.
Currently, this function fails to meet the expected outcome in certain contexts.
❯ adb devices -l
List of devices attached
RFCNB05X69K device usb:34672640X product:r8quex model:SM_G781U1 device:r8q transport_id:10
emulator-5554 device product:sdk_gphone_arm64 model:sdk_gphone_arm64 device:emulator_arm64 transport_id:14
The above is my current environment. While the E2E test suite launches the emulator — presumably because it is explicitly set — this function returns the ID for the connected physical device.
Should we update this line to instead filter by emulator or is that not a name value that we can safely presume will be consistent?
Alternatively, is this a problem that doesn't need solving? Is it likely a CI environment will find itself a state with multiple devices? Are screen recordings even used in local environments where this is more likely to occur?
There was a problem hiding this comment.
Should we update this line to instead filter by emulator or is that not a name value that we can safely presume will be consistent?
Oh yeah, good idea, filtering by emulator makes sense when there's a real device connected.
Alternatively, is this a problem that doesn't need solving? Is it likely a CI environment will find itself a state with multiple devices? Are screen recordings even used in local environments where this is more likely to occur?
I thought about it but we do have plans to run our tests in our own building setup, so we will be running tests "locally" when that happens and we would need to publish these screen recordings as artifacts somewhere. Similar to what we do here in Gutenberg. This is not needed now because SauceLabs handles the recordings differently but once we migrate, we will need them.
There was a problem hiding this comment.
I've updated the logic in ef7e47f
I moved the function to its own file so it can be shared to other files.
I've updated it to filter the devices list for emulators only.
Now it sets the device's ID for local Android builds to avoid conflicts when different emulator/devices are connected.
… also adds specifying the device ID for Android local builds for cases when multiple devices/emulators are connected.
dcalhoun
approved these changes
Oct 19, 2023
packages/react-native-editor/__device-tests__/helpers/get-android-emulator-id.js
Outdated
Show resolved
Hide resolved
dcalhoun
added a commit
that referenced
this pull request
Oct 24, 2023
* Dependencies update: - Bumps Appium to version 2.0.0 - Removes wd in favor of WebdriverIO * - Adds appium:setup script to install needed drivers for Appium. - Updates test:e2e:build-wda with the new Appium home path for the xcuitest driver. - Updates test:e2e:android:local and test:e2e:ios:local to add the newly added appium:setup script to install drivers if needed. * Update Android Runner to cache the Appium drivers * Update iOS Runner - Bump Xcode to 14.2 * Update iOS Runner with these changes: - Move the Launch of the simulator step to the top so it starts booting up the simulator in the background, so it doesn't block the next steps while it's stating. - Install and cache the Appium drivers. * Updates the E2E capabilities settings: - It removes unneeded values after the migration. - Bumps the iOS platform version to 16.2. - Adds specific SauceLabs configuration that sets the Appium version to 2.0.0. - Adds a prefixKeysWithAppium util to append the "appium:" prefix for the Appium capabilities settings. This is added to simplify the modifications we do for some of the settings in runtime. * Updates the E2E serverConfigs file with the updated SauceLabs settings for Appium 2 and WebdriverIO * E2E Utils: - Remove import of wd and set it as null temporarily while we migrate its usage to WebdriverIO * E2E Utils: Update getIOSPlatformVersions to get the desired platform version from the capabilities settings instead of an inline value * E2E Utils: - Update Remote configuration with the new structure for WebdriverIO and Appium 2 - Uses the prefixKeysWithAppium util for the Appium capabilities, we cannot use "appium:options" since that's only available for real devices and we use simulators instead. - The app capability value was updated to the new storage:filename= prefix this was important to make SauceLabs to work correctly * E2E Utils - Update the stopDriver function with: - Changes sessionID to sessionId - Removes the hash value to append in the Job URL, these URLs are public so it's not needed to append the auth param. - Updates driver.quit() to driver.deleteSession() * E2E Utils - Updates swipeFromTo to WebdriverIO * E2E Utils - Updates isEditorVisible to WebdriverIO It adds a custom timeout to waitForDisplayed, the simulators in CI are slow so we need to give enough time to find the post title locator. * Remove usage of sauceJobStatus, WebdriverIO internally handles this * EditorPage - Remove import of wd and keep a null value while we migrate to WebdriverIO * EditorPage - Migrate initializeEditor to WebdriverIO * Gutenberg Editor Rendering Media Blocks test - Migrate to WebdriverIO * E2E Utils - Revert removing passing the auth token for the jobURL, it also updates the logic to the latest implementation from SauceLabs docs. * Remove idleTimeout desktop-only SauceLabs option * [Mobile] - Update E2E Block Insertion tests (#55167) * E2E Utils, updates the typing helpers: - Removes the platform specfic typing functionality in favor of WebdriverIO's addValue * E2E Utils - Updates getKeycode and pressKeycode to WebdriverIO * E2E Utils - Update tapStatusBariOS to WebdriverIO * E2E Utils - Update toggleHtmlMode to WebdriverIO * E2E Utils - Update waitForVisible to WebdriverIO * E2E Editor Page - Update getBlockAtPosition to WebdriverIO * E2E Editor Page - Update getTitleElement to WebdriverIO * E2E Editor Page - Update getHtmlContent to WebdriverIO * E2E Editor Page - Update swipeToolbarToElement to WebdriverIO * E2E Editor Page - Update addNewBlock to WebdriverIO * E2E Editor Page - Update findBlockButton to WebdriverIO * E2E Editor Page - Update assertSlashInserterPresent to WebdriverIO * E2E EditorPage - Update waitForElementToBeDisplayedById to WebdriverIO * E2E Test - Block Insertion - Update to WebdriverIO * Update typeString scroll delay comment Co-authored-by: David Calhoun <github@davidcalhoun.me> --------- Co-authored-by: David Calhoun <github@davidcalhoun.me> * test: Update Search tests for Appium 2 (#55170) * test: Update getSearchBlockTextElement query Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * test: Update waitForElementToBeDisplayedByXPath query Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * test: Update verifySearchElementText query Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * test: Update getSearchBlockTextElement query Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * E2E Editor Page - Update dismissKeyboard to use WebdiverIO's hideKeyboard (#55211) * test: Update Media Blocks tests for Appium 2 (#55222) * test: Replace sleep with pause utility Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * test: Replace elementsByAccessibilityId usage Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * test: Replace elementByXPath usage Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * test: Repair enterCaptionToSelectedImageBlock utility Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. Additionally, the `clearValue` method did not succeed on the caption Button element. Hence, the utility was expanded to instead type into the TextView. * test: Update Paragraph tests for Appium 2 (#55215) * test: Update clickMiddleOfElement helper Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * test: Update clickBeginningOfElement helper Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * test: Update getTextForParagraphBlockAtPosition helper Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * test: Replace elementsByXPath usage Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * E2E Utils - Migrate dragAndDropAfterElement to WebdriverIO (#55254) * [Mobile] - Move Appium drivers setup to bash script (#55257) * Move Appium drivers setup to bash script * Update drivers' names casing for consistency * Rename appium:setup to test:e2e:setup * test: Update E2E utilities (#55322) * test: Set static device pixel ratio WebdriverIO (the replacement for the deprecated WD driver library) does not have a method for retrieving a device's pixel ratio. Therefore, this sets an explicit value for each of our test devices. * test: Update the `getAddBlockButton` utility Update syntax for WebdriverIO, which is a part of the Appium 2 upgrade. * test: Update toggleDarkMode utility Seems this method has changed or been removed for Android. * test: Increase robustness of waitForKeyboardToBeHidden utility This utility appeared to fail occasionally due to an empty `addButton` value. * test: toggleDarkMode uses consistent approach across platforms This updates the iOS variant to mirror the structure of the Android variant in the name of consistency. * refactor: Rename devicePixelRatio to pixelRatio Mirror Android implementation found in Appium 2. * test: Increase logging levels Attempt to debug CI failures. ``` FAIL __device-tests__/gutenberg-editor-rendering-media-blocks.test.js (73.496 s) ● Gutenberg Editor Rendering Media Blocks test › should be able to render blocks correctly unknown method: Not implemented at getErrorFromResponseBody (file:/Users/runner/work/gutenberg/gutenberg/node_modules/webdriver/build/utils.js:194:12) at NodeJSRequest._request (file:/Users/runner/work/gutenberg/gutenberg/node_modules/webdriver/build/request/index.js:164:23) at Browser.wrapCommandFn (file:/Users/runner/work/gutenberg/gutenberg/node_modules/@wdio/utils/build/shim.js:81:29) ``` * refactor: Prefer `action` to `touchAction` Avoid the deprecated `touchAction` command. > The `touchAction` command is deprecated and will be removed in a future version. We recommend to use the action command instead with pointer type touch, e.g.: > > await browser.action('pointer', { > parameters: { pointerType: 'touch' } > }) https://webdriver.io/docs/api/element/touchAction/ This may also relate to the fact that Mobile JSON Wire Protocol is deprecated as well. > Mobile JSON Wire Protocol > > The Mobile JSON Wire Protocol is a super set of mobile commands on top of the JSON Wire Protocol. Given this one is deprecated the Mobile JSON Wire Protocol also got deprecated. Appium might still support some of its commands but it is not recommended to use them. https://webdriver.io/docs/api/protocols#mobile-json-wire-protocol * Revert "test: Increase logging levels" This reverts commit 7bb46de. * test: Set default `swipeFromTo` delay value This prevents `pause` from being called without a parameter, which throws an error. * test: Replace swipe gesture with simple tap (#55427) It was unclear as to why the swipe gesture failed to close the bottom sheet, but a simple tap towards the middle-top of the display achieves the same result. * [Mobile] - Appium 2 Migration - Updating more utils (#55420) * Updates the longPressMiddleOfElement util by using WebdriverIO's action. It adds tapping on the element first before long-pressing due to an issue with iOS not triggering the long-press event. The default waitTime was reduced as well to 1000. * Update selectTextFromElement to WebdriverIO. It also changes the logic for iOS where it uses the "Select All" option from the context menu instead of using "doubleTap". Timeouts have been reduced as well. * Refactors selectTextFromElement for Android, the previous implementation would only select the word that was located in the middle of the element, with the new changes it will select all text by long-pressing from the start of the element to the end. Unfortunately we can't access the context menu as we do on iOS so it has to be done manually. * Add inline duration value and adds inline comments to explain the functionality * [Mobile] - Update closePicker util and updates Image Block test data (#55484) * Update closePicker util name to closeMediaPicker, it also unifies the logic for both platforms by having the same locator and using WebdriverIO's waitForDisplayed. * Update test data for the image block, by changing the image caption to avoid issues in CI and autocorrect * [Mobile] Improve Screen recording on Android for Appium 2 (#55443) * Improve Screenrecording on Android by making sure of the following: The sdcard folder should exist in the emulator, if not there's a script that will create it. For skipped tests, it will remove empty folders that weren't needed. When having multiple Android emulators opened, it will use the deviceID that Appium is currently using. * Rename getAndroidDeviceID function to getDeviceID since it's not platform specific * Fix comment * Temp: log session * Use adb devices list to get the first active android emulator * Moves getAndroidEmulatorID to its own file and replaces its usage. It also adds specifying the device ID for Android local builds for cases when multiple devices/emulators are connected. * Remove unneeded filtering * Move setting the androidDeviceID value for Android cases only * docs: Update E2E test documentation (#55519) * docs: Rewrite end-to-end testing documentation Update for breaking changes from upgrading Appium 2. Reduce verbosity and duplication to improve the approachability and maintainability of the documentation. * docs: Remove E2E test contributing documentation This was replaced by a section in the directory's README documentation. * docs: Rewrite end-to-end testing Getting Started section Update for breaking changes from upgrading Appium 2. Reduce verbosity and duplication between the `WordPress/gutenberg` repository to improve the approachability and maintainability of the documentation. * docs: Update links in Gutenberg for Mobile Apps code documentation Documents have been updated and consolidated to reduce verbosity and duplication to improve the approachability and maintainability of the documentation. * [Mobile] Remove `wd` usage and removes disabled tests (#55541) * Device tests - Removes skipped tests that haven't been enabled back for a while. These were disabled due to flakiness most likely to the interactions with the context menu to copy/paste text. If needed this can be restored once the flakiness issues are worked on. * Device Tests - Drag & Drop: removes disabled tests. These used utils that were flaky and have been disabled for a long time, they can be added back once stable utils are in place for pasting content. * Editor Page - Removes setHtmlContent since this util is not being used anymore. It can be added back if its needed. * Device Tests Utils - Removes usage of the wd library. It also removes, doubleTap, clickElementOutsideOfTextInput, and related utils to the context menu due to them being flaky and not currenlty in use: tapSelectAllAboveElement, tapCopyAboveElement and tapPasteAboveElement. * test: Improve E2E test consistency (#55554) * test: Mitigate conflicts between development server caches Occasionally, cryptic npm package dependency errors can occur between test runs. They appear to originate from stale development caches, e.g. Metro server. Clearing these caches, which E2E tests should not rely upon mitigates the likelihood of these errors. * test: Prefix informative E2E test setup logs Merely an attempt to add structure to the outputted logs. * test: Avoid setup scripts during test runs There may be clarity and efficiency by having an explicit setup script. It was avoid running unnecessary scripts during test runs. It is also referenced in the current documentation as a script to run intentionally. * build: Ignore generated Android files (#55569) These assets are generated at build time for E2E testing. Seemingly, we do not leverage these generated files for the binary/bundle that we provide to the host apps. Therefore, we can safely ignore these files. * docs: Avoid GMA project name until it is approved Use of the "Gutenberg for Mobile Apps" name is still under review. In the meantime, we should use the existing "Mobile Gutenberg" project name. --------- Co-authored-by: David Calhoun <github@davidcalhoun.me>
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.
Part of #55166
What?
This PR updates the logic related to detecting the Android device and how it triggers the
adbcommands, now it will use the first emulator ID available to avoid issues of multiple devices connected.Why?
To avoid showing warnings when multiple devices were connected.
It also updates the logic related to screen recordings to avoid having empty files for
skippedtestsHow?
It adds a
getAndroidEmulatorIDfunction to get the first available emulator, this avoids issues when multiple devices are connected.It cleans up empty screen recordings for
skippedtests.Testing Instructions
CI checks should pass.
Screen recording artifacts should be stored.
Testing Instructions for Keyboard
N/A
Screenshots or screencast
N/A