-
Notifications
You must be signed in to change notification settings - Fork 7
Update to Appium 9.5.0 and Selenium 4.33.0 +semver:feature #45
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
WalkthroughThe updates include modifications to the CI pipeline and project dependencies, as well as test adjustments. The pipeline now uses Android SDK API 30, explicitly installs Java 8 with Homebrew, adds verbose logging to the emulator, implements a bounded emulator readiness check, and installs Appium version 2 explicitly. The Maven dependencies were updated to newer versions. Test logic was modified to swap focus between radio buttons and the URL for a web radio button test was changed. Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (3)
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 2
🧹 Nitpick comments (1)
azure-pipelines.yml (1)
78-80
:-verbose
emulator switch floods logs – consider redirecting to a fileThe added flag will stream tens of MBs per minute to the job log, making troubleshooting harder and slowing the UI. Recommend redirecting to
emulator.log
and uploading it only on failure.-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 > emulator.log 2>&1 &
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
azure-pipelines.yml
(2 hunks)pom.xml
(2 hunks)
⏰ 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). (3)
- GitHub Check: aquality-automation.aquality-appium-mobile-java (Run tests LOCAL_SERVER)
- GitHub Check: aquality-automation.aquality-appium-mobile-java (Analyse code with SonarQube)
- GitHub Check: aquality-automation.aquality-appium-mobile-java (Run tests REMOTE_SERVER)
🔇 Additional comments (3)
pom.xml (2)
188-199
: Cross-check transitive Selenium version after upgrading core & Appium client
aquality-selenium-core:4.6.0
andjava-client:9.5.0
both bring their own Selenium dependencies.
If your intention is to land on Selenium4.33.0
(per PR title) you should explicitly pin it under<dependencyManagement>
; otherwise Maven may keep whatever version is required by the two libs (historically < 4.20), yielding a runtime “NoSuchMethodError” cocktail.Example pin-in-BOM:
<dependencyManagement> <dependencies> <dependency> <groupId>org.seleniumhq.selenium</groupId> <artifactId>selenium-java</artifactId> <version>4.33.0</version> </dependency> </dependencies> </dependencyManagement>Please verify with
mvn dependency:tree
that the resolved Selenium version is the one you expect.
208-219
: Validate that the new Lang 3 / IO versions exist on Maven Central
commons-lang3:3.17.0
andcommons-io:2.19.0
are ahead of the latest GA versions I’m aware of (3.14 / 2.16). Builds will 404 if these artifacts have not yet been published. Double-check on search.maven.org before merging or pin to the latest available releases.azure-pipelines.yml (1)
64-67
: JAVA 8 install is local to this script only – confirm Maven still compiles with JDK 11Environment variables exported inside the
CmdLine@2
step do not survive into later tasks.
Given that the subsequent Maven steps explicitly request JDK 11 this is probably fine, but please sanity-check that:
- Appium/emulator really need 1.8 (many tools are OK with 11 +).
- The mixed toolchain doesn’t surface when running tests that spawn adb/Appium from Java code.
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
♻️ Duplicate comments (1)
azure-pipelines.yml (1)
93-93
: Appium version pinning addresses previous review feedback.The change from
appium@next
toappium@2
directly addresses the previous review comment about preventing spontaneous pipeline breakages. This ensures build stability while allowing minor/patch updates within the major version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
azure-pipelines.yml
(2 hunks)src/test/java/samples/android/ITestRadioButton.java
(1 hunks)src/test/java/samples/android/web/WebRadioButtonTest.java
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/test/java/samples/android/web/WebRadioButtonTest.java
⏰ 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). (3)
- GitHub Check: aquality-automation.aquality-appium-mobile-java (Run tests LOCAL_SERVER)
- GitHub Check: aquality-automation.aquality-appium-mobile-java (Analyse code with SonarQube)
- GitHub Check: aquality-automation.aquality-appium-mobile-java (Run tests REMOTE_SERVER)
🔇 Additional comments (4)
azure-pipelines.yml (3)
56-56
: Good upgrade to Android API 30 with x86_64 architecture.The upgrade from API 28 to API 30 and the switch to x86_64 architecture addresses the previous review feedback about avoiding Rosetta translation on Apple Silicon hosts. This should improve emulator performance and stability.
64-66
: Explicit Java 8 installation improves build reproducibility.Adding explicit Java 8 installation via Homebrew and proper environment variable configuration ensures consistent Java version across different macOS agents, which is essential for compatibility with the updated Appium and Selenium versions.
79-79
: Verbose logging will help with emulator troubleshooting.The addition of the
-verbose
flag to the emulator startup command will provide more detailed logs for debugging emulator issues, which is valuable for CI/CD troubleshooting.src/test/java/samples/android/ITestRadioButton.java (1)
14-20
: Test logic change maintains coverage while improving clarity.The modification to focus on the second radio button instead of the first maintains the same test coverage for radio button mutual exclusivity. The test sequence correctly verifies:
- Initial unchecked state of button2
- Checked state after clicking button2
- Unchecked state after clicking button1 (mutual exclusivity)
The test logic is sound and provides clear validation of radio button behavior.
|
No description provided.