Update to Selenium 4.39.0 +semver:feature #156
Conversation
Update DevTools references to use V143 in tests
WalkthroughUpdated Maven dependency versions and advanced Selenium DevTools protocol imports; added new network emulation public APIs and test/fixture updates, plus small locator and test-constant adjustments. No public class signature removals. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Test Case
participant Network as NetworkHandling
participant DevTools as DevTools/CDP
participant Browser as Browser (navigator/network)
participant Page as Page/Form
Note over Test,Network: New network emulation flow (high level)
Test->>Network: emulateConditionsByRule(offline?, conditionsList)
Network->>DevTools: send Network.emulateNetworkConditions / setNetworkState
DevTools->>Browser: apply network metrics (latency, throughput, offline, connectionType)
Browser->>Page: simulate network behavior on reload/request
Page-->>Test: content load / timeout / error
Note over Test,Network: Later: Network.overrideState(...) to explicitly set navigator state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
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. Comment |
…e geolocation override test Update DevTools references to use V143 in the library and V142 in tests Update commons-lang3 reference
|
![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
🧹 Nitpick comments (5)
src/test/java/w3schools/forms/SelectMultipleForm.java (1)
29-30: Verify the updated locators work with the current W3Schools UI.The Accept Cookies locator has been updated to a more complex CSS selector, and a new iframe field has been added. These changes suggest W3Schools modified their cookie consent implementation. Please ensure the new locators correctly identify the elements on the current W3Schools page.
Optional: Consider the semantic accuracy of element types.
Using
ILabelfor an iframe element (line 30) is semantically incorrect, though it may work functionally. If the framework provides a more generic element type (e.g.,IElement) or supports creating custom element interfaces, consider using a more appropriate type for iframe elements.src/test/java/tests/usecases/devtools/OverrideGeolocationTest.java (1)
28-29: Consider adding delta tolerance for floating-point comparisons.Using
Assert.assertEqualsondoublevalues without a delta tolerance may cause flaky tests if there's any floating-point representation variance. Consider using the overload with a tolerance parameter.🔎 Proposed fix
- Assert.assertEquals(form.getLatitude(), LAT_FOR_OVERRIDE); - Assert.assertEquals(form.getLongitude(), LNG_FOR_OVERRIDE); + Assert.assertEquals(form.getLatitude(), LAT_FOR_OVERRIDE, 0.000001, "Latitude override mismatch"); + Assert.assertEquals(form.getLongitude(), LNG_FOR_OVERRIDE, 0.000001, "Longitude override mismatch");src/test/java/forms/MyLocationForm.java (2)
14-16: Form locator is quite generic.The locator
//*[contains(text(),'Location')]is broad and may match unintended elements on pages with the word "Location" anywhere. Consider making it more specific if stability issues arise.
18-23: Side effect in getter - consider documenting or refactoring.
getLatitude()now has a side effect of clicking the consent button. This may surprise callers who expect a pure getter. Consider either:
- Adding a Javadoc comment explaining this behavior, or
- Moving consent handling to a dedicated method called from the test setup.
src/main/java/aquality/selenium/browser/devtools/NetworkHandling.java (1)
359-361: Consider extracting the repeatedenable()call.Each method calls
enable(Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()). While this ensures network monitoring is active, consider extracting to a private helper for DRY and readability.🔎 Proposed refactor
private void ensureNetworkEnabled() { tools.sendCommand(enable(Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); }Then replace all occurrences with
ensureNetworkEnabled();Also applies to: 372-374, 386-388
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
pom.xmlsrc/main/java/aquality/selenium/browser/devtools/DevToolsHandling.javasrc/main/java/aquality/selenium/browser/devtools/EmulationHandling.javasrc/main/java/aquality/selenium/browser/devtools/JavaScriptHandling.javasrc/main/java/aquality/selenium/browser/devtools/NetworkHandling.javasrc/test/java/automationpractice/Constants.javasrc/test/java/forms/MyLocationForm.javasrc/test/java/tests/usecases/devtools/DeviceEmulationTest.javasrc/test/java/tests/usecases/devtools/NetworkSpeedEmulationTest.javasrc/test/java/tests/usecases/devtools/OverrideGeolocationTest.javasrc/test/java/tests/usecases/devtools/OverrideUserAgentTest.javasrc/test/java/w3schools/forms/SelectMultipleForm.java
✅ Files skipped from review due to trivial changes (3)
- src/test/java/tests/usecases/devtools/DeviceEmulationTest.java
- src/main/java/aquality/selenium/browser/devtools/DevToolsHandling.java
- src/test/java/tests/usecases/devtools/OverrideUserAgentTest.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mialeska
Repo: aquality-automation/aquality-selenium-java PR: 148
File: src/test/java/tests/usecases/devtools/DeviceEmulationTest.java:6-7
Timestamp: 2024-11-29T14:35:07.825Z
Learning: In this project, the use of custom DevTools Protocol version (v131) in test files is intentional, even if it differs from the version used in the main source code.
Learnt from: mialeska
Repo: aquality-automation/aquality-selenium-java PR: 150
File: src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java:121-121
Timestamp: 2025-05-28T19:55:33.768Z
Learning: The DevTools v137 `Emulation.setDeviceMetricsOverride` method has 14 parameters, not 13 as some documentation might suggest. Recent versions of the Selenium DevTools API have added additional parameters to this method.
📚 Learning: 2024-11-29T14:35:07.825Z
Learnt from: mialeska
Repo: aquality-automation/aquality-selenium-java PR: 148
File: src/test/java/tests/usecases/devtools/DeviceEmulationTest.java:6-7
Timestamp: 2024-11-29T14:35:07.825Z
Learning: In this project, the use of custom DevTools Protocol version (v131) in test files is intentional, even if it differs from the version used in the main source code.
Applied to files:
src/main/java/aquality/selenium/browser/devtools/EmulationHandling.javasrc/main/java/aquality/selenium/browser/devtools/JavaScriptHandling.java
📚 Learning: 2025-05-28T19:55:33.768Z
Learnt from: mialeska
Repo: aquality-automation/aquality-selenium-java PR: 150
File: src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java:121-121
Timestamp: 2025-05-28T19:55:33.768Z
Learning: The DevTools v137 `Emulation.setDeviceMetricsOverride` method has 14 parameters, not 13 as some documentation might suggest. Recent versions of the Selenium DevTools API have added additional parameters to this method.
Applied to files:
src/main/java/aquality/selenium/browser/devtools/EmulationHandling.javasrc/main/java/aquality/selenium/browser/devtools/NetworkHandling.java
🧬 Code graph analysis (2)
src/test/java/tests/usecases/devtools/NetworkSpeedEmulationTest.java (1)
src/main/java/aquality/selenium/browser/devtools/NetworkHandling.java (1)
NetworkHandling(31-410)
src/test/java/w3schools/forms/SelectMultipleForm.java (1)
src/main/java/aquality/selenium/browser/AqualityServices.java (1)
AqualityServices(16-176)
🔇 Additional comments (14)
src/test/java/w3schools/forms/SelectMultipleForm.java (2)
6-6: LGTM!The new imports are necessary for the updated cookie acceptance flow that handles iframe context switching.
Also applies to: 21-21
37-45: LGTM!The updated
acceptCookiesmethod correctly handles the iframe context for W3Schools' cookie consent UI. The logic:
- Conditionally switches to the iframe if it exists
- Waits for and clicks the Accept button
- Always ensures we're back in default content
- Waits for the iframe to disappear after acceptance
This gracefully handles both scenarios where the iframe exists or doesn't exist.
src/main/java/aquality/selenium/browser/devtools/JavaScriptHandling.java (1)
15-17: LGTM!The DevTools imports are correctly updated to v143, and all usages (
Page.removeScriptToEvaluateOnNewDocument,ScriptIdentifier,Runtime.addBinding,Runtime.removeBinding,Runtime.enable) are consistent with the new protocol version.src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java (1)
5-8: LGTM!The DevTools imports are correctly updated to v143 for
RGBA,Emulation,MediaFeature, andScreenOrientation. The existing usage patterns, including the 14-parametersetDeviceMetricsOverridecall, remain compatible with the updated protocol version. Based on learnings, this parameter count is expected for recent DevTools versions.src/test/java/tests/usecases/devtools/OverrideGeolocationTest.java (1)
13-14: LGTM - values adjusted for new test site.The geolocation override constants are appropriately rounded to 6 decimal places, which aligns with typical geolocation API precision.
src/test/java/forms/MyLocationForm.java (1)
10-12: LGTM - Flexible locators for the new test site.The union XPath expressions provide good fallback strategies for different page layouts. The consent button locator handles both
aria-labeland class-based selectors appropriately.src/test/java/tests/usecases/devtools/NetworkSpeedEmulationTest.java (2)
25-29: LGTM - Good refactoring with helper methods.Extracting
network()helper and movingWelcomeFormto a field improves readability and reduces duplication.
46-55: LGTM - Test correctly uses new network emulation APIs.The test properly combines
emulateConditionsByRuleandoverrideStateas documented in theNetworkHandlingclass, replacing the deprecatedemulateConditionsmethod.src/main/java/aquality/selenium/browser/devtools/NetworkHandling.java (4)
11-11: LGTM - Network model imports updated to v143.The wildcard import for
org.openqa.selenium.devtools.v143.network.model.*is consistent with the DevTools version upgrade.
341-349: LGTM - Appropriate deprecation with migration guidance.The deprecation annotation and Javadoc clearly direct users to the new
emulateConditionsByRuleandoverrideStatemethods. The method remains functional for backward compatibility.
351-362: LGTM - New emulateConditionsByRule API.The new method properly documents the difference from the deprecated approach: it affects individual requests via URL matching without modifying
navigatorstate.
364-389: LGTM - New overrideState APIs with clear purpose.Both overloads correctly implement navigator state override functionality. The 5-parameter version with
ConnectionTypeprovides fine-grained control.src/test/java/automationpractice/Constants.java (1)
8-8: URL change is verified and stable.The new geolocation test URL
https://whereamirightnow.orgis accessible with HTTP/2 200 response and proper server headers. The constant is used only inOverrideGeolocationTest.javafor its intended purpose.pom.xml (1)
78-78: The upgrade to commons-lang3 3.20.0 successfully resolves CVE-2025-48924.This is a valid release (November 12, 2025) that addresses the uncontrolled-recursion vulnerability in ClassUtils.getClass() which affected versions 3.0 through 3.17.x. No new vulnerabilities are known for version 3.20.0.
Update DevTools references to use V143 in tests