-
Notifications
You must be signed in to change notification settings - Fork 7
Update Appium version to 10 +semver: breaking #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Update Selenium reference to v.4.35.0
WalkthroughBumped two dependency versions in Changes
Sequence Diagram(s)sequenceDiagram
participant DevOps as CI Pipeline
participant AppiumCLI as Appium CLI
rect rgb(230, 245, 255)
DevOps->>AppiumCLI: install uiautomator2@4
AppiumCLI-->>DevOps: install specific driver v4 (success / error)
end
note right of DevOps #DFF2D8: Change pins driver to major v4
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 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 selected for processing (1)
pom.xml
(1 hunks)
🔇 Additional comments (1)
pom.xml (1)
193-193
: Validate Appium java-client 10.0.0 upgrade
- Compile and run full test suite against
io.appium:java-client:10.0.0
and verify all public APIs still compile.- Manually inspect Maven dependency tree for convergence issues (Selenium/Appium/OkHttp/Guava).
- If this introduces breaking API or behavior changes for consumers, bump your artifact to 6.0.0 and publish release notes.
- Optional: add Maven Enforcer (dependencyConvergence, requireJavaVersion ≥ 11) to catch future conflicts early.
…requires pre-release appium server version
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
azure-pipelines.yml (3)
76-81
: Bash var expansion bug: $(VAR) invokes a command, not a variableCurrent lines will fail by trying to execute commands named ANDROID_SDK_ID/ANDROID_EMU_NAME.
- echo "y" | $ANDROID_HOME/tools/bin/sdkmanager "$(ANDROID_SDK_ID)" - echo "no" | $ANDROID_HOME/tools/bin/avdmanager create avd -n "$(ANDROID_EMU_NAME)" -k "$(ANDROID_SDK_ID)" -f + echo "y" | "$ANDROID_HOME"/tools/bin/sdkmanager "$ANDROID_SDK_ID" + echo "no" | "$ANDROID_HOME"/tools/bin/avdmanager create avd -n "$ANDROID_EMU_NAME" -k "$ANDROID_SDK_ID" -f @@ - nohup $ANDROID_HOME/emulator/emulator -avd "$(ANDROID_EMU_NAME)" -gpu swiftshader_indirect -noaudio -no-boot-anim -no-snapshot -verbose > /dev/null 2>&1 & + nohup "$ANDROID_HOME"/emulator/emulator -avd "$ANDROID_EMU_NAME" -gpu swiftshader_indirect -noaudio -no-boot-anim -no-snapshot -verbose > /dev/null 2>&1 &
96-101
: Remove manual symlink and file-path PATH entry for AppiumGlobal npm install already places the appium binary on PATH; forcing a symlink and adding a file path to PATH is brittle.
- ln -fs /usr/local/lib/node_modules/appium/build/lib/main.js /usr/local/bin/appium - chmod +x /usr/local/bin/appium - export PATH=$PATH:/usr/local/bin/appium appium --version
108-114
: Drop deprecated request package and fix appium-doctor install/pathrequest is deprecated and unnecessary here. Also, the ln/export lines point to the old package layout and add a file path to PATH.
- npm install -g [email protected] - npm install @appium/doctor -g - ln -fs /usr/local/lib/node_modules/appium-doctor/appium-doctor.js /usr/local/bin/appium-doctor - chmod +x /usr/local/bin/appium-doctor - export PATH=$PATH:/usr/local/bin/appium-doctor - appium-doctor + npm install -g @appium/doctor + appium-doctor
🧹 Nitpick comments (4)
azure-pipelines.yml (4)
103-106
: Pinning UIAutomator2: prefer scoped package; verify compatibility with Java client 10 and Android 30Use the scoped driver name to avoid alias resolution issues and ensure the intended major is installed. Please also confirm v4 is the expected pair for Appium server v2 + java-client v10 and your Android API level.
- appium driver install uiautomator2@4 + appium driver install --source=npm @appium/uiautomator2-driver@4 + appium driver list --installed
65-75
: Harden the shell script for early failuresEnable strict mode to fail fast on errors and unset vars.
echo "Configuring Environment" + set -euo pipefail + set -x
66-74
: JDK mismatch across steps; avoid forcing Java 8 hereYou install Temurin 8 and export JAVA_HOME=1.8, while Maven steps use JDK 11 and Sonar uses JDK 21. This may create toolchain confusion.
If Java 8 isn’t strictly required by the Android tools on this image, consider removing the Temurin 8 install and JAVA_HOME override, or align all steps to JDK 11.
119-126
: Appium server readiness and condition expression
- Ensure the condition syntax matches Azure conventions (variables.isRemote is typically used), though variables['isRemote'] may work.
- Add a short wait-for-port loop to avoid race conditions before tests start.
inputs: script: | - appium --allow-insecure chromedriver_autodownload --base-path /wd/hub & + appium --allow-insecure chromedriver_autodownload --base-path /wd/hub & echo "Appium server started" + for i in {1..60}; do nc -z localhost 4723 && break; sleep 1; done + nc -z localhost 4723 || { echo "Appium not listening on 4723"; exit 1; } condition: eq(variables['isRemote'], 'true')
📜 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 selected for processing (1)
azure-pipelines.yml
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: mialeska
PR: aquality-automation/aquality-appium-mobile-java#53
File: pom.xml:187-187
Timestamp: 2025-08-29T14:08:56.398Z
Learning: In the aquality-appium-mobile-java project, the team intentionally relies on transitive dependency resolution for Selenium versions through aquality-selenium-core, rather than explicitly pinning Selenium dependencies in the POM.
🔇 Additional comments (2)
azure-pipelines.yml (2)
58-59
: Emulator ABI vs runner architecturemacOS-latest can be ARM-based. An x86_64 system image won’t boot on ARM runners. Please verify the current vmImage architecture and switch to an arm64 system image if needed.
Expected alternative:
- system-images;android-30;google_apis_playstore;arm64-v8a
1-1
: PR-level note: Selenium version pinningPer team practice, Selenium is resolved transitively via aquality-selenium-core. If this PR explicitly pins Selenium in pom.xml, consider reverting to transitive resolution to avoid version drift.
I can review the POM diff and suggest precise changes if you include it in the PR.
Update Selenium reference to v.4.35.0