fix(workflow): 🐛 fixing android test with appium v3#1071
Conversation
WalkthroughUpdates CI to Appium 3, adjusts Android emulator settings, introduces per-driver insecure flags in server configuration and usage, tweaks a UI test assertion, updates Android versions and allow_insecure in test configs, narrows a TestNG suite scope, removes Dependabot reviewers, and bumps minor/patch package versions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Tester
participant ServiceManager
participant ServerSetting
participant Appium as Appium Server
Tester->>ServiceManager: startServer()
ServiceManager->>ServerSetting: getAllowInsecure(driverName)
ServiceManager->>ServerSetting: getDenyInsecure(driverName)
Note over ServiceManager,ServerSetting: Build per-driver insecure tokens (driver:flag)
ServiceManager->>Appium: launch with args (--allow-insecure [...], --deny-insecure [...])
Appium-->>ServiceManager: server ready
ServiceManager-->>Tester: session can start
sequenceDiagram
autonumber
actor CI
participant Runner as GitHub Runner
participant Setup as Setup Steps
participant Appium as Appium 3
CI->>Runner: dispatch workflow
Runner->>Setup: install platform deps (ffmpeg/applesimutils)
Setup->>Setup: install drivers conditionally (Android/iOS/macOS)
Setup->>Appium: start with per-driver --allow-insecure tokens
Appium-->>Runner: ready for tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/_test-core-java.yml (1)
98-104: Apt commands are interactive; job can hang.Use
apt-getwith-y(or drop upgrade entirely for speed/stability).- if [ "${{ startsWith(inputs.runs-on, 'ubuntu') }}" == "true" ]; then - sudo apt update && sudo apt upgrade - sudo apt install ffmpeg + if [ "${{ startsWith(inputs.runs-on, 'ubuntu') }}" == "true" ]; then + sudo apt-get update -y + sudo apt-get install -y ffmpeg else brew install ffmpeg brew tap wix/brew && brew install wix/brew/applesimutils fi - npm install -g appium + npm install -g appium@^3
🧹 Nitpick comments (7)
.github/workflows/test-core.yml (1)
139-141: AVD naming vs profile mismatch; API level bump to 35You’re creating an AVD named Pixel_8_Pro but using the pixel_7_pro hardware profile. Likely fine, but it’s confusing during triage. Consider aligning the AVD name with the profile to avoid misreads in logs. Also ensure your reusable workflow pulls a system image for apiLevel 35 (google_apis, x86_64) to prevent cold-download timeouts.
If you want, I can propose a small change to make
emulator-name: Pixel_7_Profor consistency.core-java/src/test/java/io/github/boykaframework/testng/ui/theinternet/DoubleClickTest.java (1)
79-81: Assertion too loose; keep flexibility but assert intended color
startsWith("rgba")may pass on any color. Consider anchoring to the expected RGB to keep the test meaningful while allowing alpha/spacing drift.Apply:
- onElement (doubleClickPage ().getDoubleClick ()).verifyStyle ("background-color") - .startsWith ("rgba"); + onElement (doubleClickPage ().getDoubleClick ()).verifyStyle ("background-color") + .startsWith ("rgba(147, 203, 90");If available, an even better option is a contains/matches on
147, 203, 90.core-java/src/main/java/io/github/boykaframework/manager/ServiceManager.java (1)
209-212: Per-driver allow/deny wiring looks right; minor readability tweak.Avoid repeating
getDriverName()and reduce chance of mismatch by caching it locally.- setArgument (USE_DRIVERS, this.setting.getDriver () - .getDriverName ()); - setArgument (ALLOW_INSECURE, this.setting.getAllowInsecure (this.setting.getDriver () - .getDriverName ())); - setArgument (DENY_INSECURE, this.setting.getDenyInsecure (this.setting.getDriver () - .getDriverName ())); + final var driverName = this.setting.getDriver ().getDriverName (); + setArgument (USE_DRIVERS, driverName); + setArgument (ALLOW_INSECURE, this.setting.getAllowInsecure (driverName)); + setArgument (DENY_INSECURE, this.setting.getDenyInsecure (driverName));.github/workflows/_test-core-java.yml (2)
112-116: mac2 driver install guard is fine. Small portability nit.Good guard; no change required. Optional: move this logic to a dedicated step with
if: ${{ startsWith(inputs.runs-on, 'macos') && !inputs.run-android && !inputs.run-ios }}to keep scripts simpler.
117-119: Make log dir creation idempotent and quote allow-insecure safely.Use
-pfor mkdir. Also, a single-quoted, comma-separated value avoids shell concat quirks.- mkdir core-java/logs - appium server --address 127.0.0.1 --port 4724 --use-drivers uiautomator2 --allow-insecure "uiautomator2:get_server_logs","uiautomator2:chromedriver_autodownload" > core-java/logs/appium-android-server.log 2>&1 & + mkdir -p core-java/logs + appium server --address 127.0.0.1 --port 4724 --use-drivers uiautomator2 --allow-insecure 'uiautomator2:get_server_logs,uiautomator2:chromedriver_autodownload' > core-java/logs/appium-android-server.log 2>&1 &core-java/src/main/java/io/github/boykaframework/config/ui/mobile/server/ServerSetting.java (2)
69-84: Guard null/empty inputs and de-dup entries.Prevent
null:prefixes and unnecessary allocations; filter blanks anddistinct()the result.- public List<String> getAllowInsecure (final String driverName) { - if (nonNull (this.allowInsecure)) { - return this.allowInsecure.stream () - .map (s -> format ("{0}:{1}", driverName, s)) - .toList (); - } - return emptyList (); - } + public List<String> getAllowInsecure (final String driverName) { + if (isEmpty (driverName) || this.allowInsecure == null || this.allowInsecure.isEmpty ()) { + return emptyList (); + } + return this.allowInsecure.stream () + .filter (s -> s != null && !s.isBlank ()) + .map (s -> format ("{0}:{1}", driverName, s)) + .distinct () + .toList (); + }
98-113: Mirror the same guards and filters for deny list.- public List<String> getDenyInsecure (final String driverName) { - if (nonNull (this.denyInsecure)) { - return this.denyInsecure.stream () - .map (s -> format ("{0}:{1}", driverName, s)) - .toList (); - } - return emptyList (); - } + public List<String> getDenyInsecure (final String driverName) { + if (isEmpty (driverName) || this.denyInsecure == null || this.denyInsecure.isEmpty ()) { + return emptyList (); + } + return this.denyInsecure.stream () + .filter (s -> s != null && !s.isBlank ()) + .map (s -> format ("{0}:{1}", driverName, s)) + .distinct () + .toList (); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.github/dependabot.yml(0 hunks).github/workflows/_test-core-java.yml(3 hunks).github/workflows/test-core.yml(1 hunks)core-java/src/main/java/io/github/boykaframework/config/ui/mobile/server/ServerSetting.java(3 hunks)core-java/src/main/java/io/github/boykaframework/manager/ServiceManager.java(1 hunks)core-java/src/test/java/io/github/boykaframework/testng/ui/theinternet/DoubleClickTest.java(1 hunks)core-java/src/test/resources/boyka-config.json(3 hunks)core-java/test-suites/testng-web-local.xml(0 hunks)package.json(1 hunks)website/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- core-java/test-suites/testng-web-local.xml
- .github/dependabot.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: test-mobile-android-local / test
- GitHub Check: test-mobile-lt / test
- GitHub Check: test-web-bs / test
- GitHub Check: test-mobile-ios-local / test
- GitHub Check: test-mobile-bs / test
- GitHub Check: test-desktop-macos / test
- GitHub Check: test-web-lt / test
- GitHub Check: test-web-local / test
🔇 Additional comments (7)
package.json (1)
64-64: release-it bump: OK; confirm plugin compatibility matrix19.0.4 should be a safe patch upgrade. Please double-check compatibility with @release-it-plugins/lerna-changelog (^8.0.1) and your current Node (engines >=22) to avoid CI surprises.
Run locally/CI:
pnpm release --versionandpnpm release --ci --dry-runto validate.website/package.json (1)
22-22: Patch bump to @mdx-js/react: LGTMNo breaking API; safe update for Docusaurus site.
core-java/src/test/resources/boyka-config.json (4)
272-272: Android 15 bump aligns with emulator API 35Version set to "15" for local Sauce Android looks good and matches the workflow’s emulator-version: 35.
311-311: Android 15 for WDIO localConsistent with the workflow update; no issues spotted.
352-352: Grid WDIO Android to 15: OK; external Appium node must matchLooks fine. Ensure the Grid node’s Appium instance (if any) also runs with the required allow-insecure per-driver flags, as these cannot be injected via Selenium Grid caps.
303-308: Driver name mapping is correct; external server must include flags
getDriverName() returns “uiautomator2” for UI_AUTOMATOR, so the insecure flags will be prefixed properly. Since this config sets"external": true, Boyka won’t launch Appium with these arguments—ensure your standalone Appium server is started with the same--allow-insecureflags.core-java/src/main/java/io/github/boykaframework/config/ui/mobile/server/ServerSetting.java (1)
19-22: Static imports improve readability.Good choice; keeps stream mapping concise.
| - name: Install Appium 3.0 | ||
| if: inputs.run-appium |
There was a problem hiding this comment.
Broken step condition: missing expression delimiters.
if: must use expression syntax; otherwise the step may not evaluate as intended.
- - name: Install Appium 3.0
- if: inputs.run-appium
+ - name: Install Appium 3.0
+ if: ${{ inputs.run-appium }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install Appium 3.0 | |
| if: inputs.run-appium | |
| - name: Install Appium 3.0 | |
| if: ${{ inputs.run-appium }} |
🤖 Prompt for AI Agents
.github/workflows/_test-core-java.yml around lines 95 to 96: the step condition
uses bare `if: inputs.run-appium` which is invalid; wrap the input in an
expression so GitHub evaluates it (e.g. replace with `if: ${{ inputs.run-appium
}}` or an explicit expression like `if: ${{ inputs.run-appium == 'true' }}`) to
ensure the step condition is evaluated correctly.
| - name: Enable KVM | ||
| if: inputs.run-android | ||
| if: inputs.run-android && startsWith(inputs.runs-on, 'ubuntu') | ||
| run: | |
There was a problem hiding this comment.
Another step condition missing expression delimiters.
Without ${{ }}, this may be parsed as a literal string.
- - name: Enable KVM
- if: inputs.run-android && startsWith(inputs.runs-on, 'ubuntu')
+ - name: Enable KVM
+ if: ${{ inputs.run-android && startsWith(inputs.runs-on, 'ubuntu') }}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/_test-core-java.yml lines 136-138: the step condition is
written as a plain string (`if: inputs.run-android && startsWith(inputs.runs-on,
'ubuntu')`) and will be treated literally; wrap the entire expression in GitHub
Actions expression delimiters so it reads `if: ${{ inputs.run-android &&
startsWith(inputs.runs-on, 'ubuntu') }}` to ensure the condition is evaluated.
|
|
🚀 This pull request is included in v2.9.0. See Release 2.9.0 for release notes. |
1 participant
Closes: #1070
Type of changes?
Checklist for Java project
Checklist for Website
Reviewers
@BoykaFramework/boyka-core
Important
Make sure to check the
Allow edits from maintainersbox below this windowSummary by CodeRabbit