[java][BiDi] implement emulation #16070

Delta456 · 2025-07-18T11:33:13Z

User description

🔗 Related Issues

💥 What does this PR do?

Implements emulation BiDi spec in Java

🔧 Implementation Notes

Uses same BiDi approach which is used for other modules

💡 Additional Considerations

🔄 Types of changes

New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement

Description

Implements BiDi emulation module for geolocation override
Adds GeolocationCoordinates class with validation
Creates comprehensive test suite for emulation functionality
Updates build configuration for new emulation package

Diagram Walkthrough

flowchart LR
  A["Emulation Module"] --> B["GeolocationCoordinates"]
  A --> C["SetGeolocationOverrideParameters"]
  A --> D["GeolocationPositionError"]
  B --> E["Validation Logic"]
  C --> F["Parameter Mapping"]
  A --> G["BiDi Command Execution"]
  H["Test Suite"] --> I["Context Testing"]
  H --> J["User Context Testing"]
  H --> K["Error Handling"]

File Walkthrough

	Relevant files

qodo-merge-pro · 2025-07-18T11:33:49Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌ 5678 - Not compliant Non-compliant requirements: Fix ChromeDriver connection failure error after first instance Resolve "ConnectFailure (Connection refused)" error for subsequent ChromeDriver instances Ensure proper ChromeDriver instantiation without connection issues 1234 - Not compliant Non-compliant requirements: Fix JavaScript execution in link href on click() for Firefox Ensure JavaScript alerts work properly with Selenium 2.48+ versions Restore functionality that worked in version 2.47.1
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Hardcoded Value The type field is hardcoded to "positionUnavailable" without any way to customize it or handle different error types that might be needed for comprehensive geolocation error handling. String type = "positionUnavailable"; Package Visibility The constructor has package-private visibility which may limit extensibility and proper instantiation from outside the package, potentially requiring a builder pattern or factory method. SetGeolocationOverrideParameters( GeolocationCoordinates coordinates, Test Assertions Using basic assert statements instead of proper JUnit assertions reduces test readability and debugging capabilities when tests fail. assert !r.containsKey("error"); double latitude = ((Number) r.get("latitude")).doubleValue(); double longitude = ((Number) r.get("longitude")).doubleValue(); double accuracy = ((Number) r.get("accuracy")).doubleValue(); assert abs(latitude - coords.getLatitude()) < 0.0001; assert abs(longitude - coords.getLongitude()) < 0.0001; assert abs(accuracy - coords.getAccuracy()) < 0.0001;

qodo-merge-pro · 2025-07-18T11:34:36Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Use proper JUnit assertions Replace primitive `assert` statements with proper JUnit assertions for better test failure reporting and debugging. JUnit assertions provide clearer error messages when tests fail. java/test/org/openqa/selenium/bidi/emulation/EmulationTest.java [74-82] -assert !r.containsKey("error"); +assertThat(r).doesNotContainKey("error"); double latitude = ((Number) r.get("latitude")).doubleValue(); double longitude = ((Number) r.get("longitude")).doubleValue(); double accuracy = ((Number) r.get("accuracy")).doubleValue(); -assert abs(latitude - coords.getLatitude()) < 0.0001; -assert abs(longitude - coords.getLongitude()) < 0.0001; -assert abs(accuracy - coords.getAccuracy()) < 0.0001; +assertThat(abs(latitude - coords.getLatitude())).isLessThan(0.0001); +assertThat(abs(longitude - coords.getLongitude())).isLessThan(0.0001); +assertThat(abs(accuracy - coords.getAccuracy())).isLessThan(0.0001); Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly recommends replacing Java `assert` statements with JUnit assertions, which provide more informative failure messages and are standard practice in tests.	Low
General	Make field private and final The `type` field should be private and final to ensure proper encapsulation and immutability. Add a constructor to initialize the field properly. java/src/org/openqa/selenium/bidi/emulation/GeolocationPositionError.java [5-11] public class GeolocationPositionError { - String type = "positionUnavailable"; + private final String type; + + public GeolocationPositionError() { + this.type = "positionUnavailable"; + } public Map<String, Object> toMap() { return Map.of("type", type); } } Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly improves encapsulation and immutability by making the `type` field private and final, which is a good coding practice.	Low
Learned best practice	Add null check for BiDi instance Add null check for the BiDi instance returned from getBiDi() method before assigning it to prevent potential NullPointerException. The getBiDi() method could return null in some implementations. java/src/org/openqa/selenium/bidi/emulation/Emulation.java [20] -this.bidi = ((HasBiDi) driver).getBiDi(); +BiDi biDiInstance = ((HasBiDi) driver).getBiDi(); +if (biDiInstance == null) { + throw new IllegalStateException("BiDi instance cannot be null"); +} +this.bidi = biDiInstance; Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add null checks and validation for parameters, properties, and return values before using them to prevent NullReferenceException and other runtime errors.	Low
More

Copilot

Pull Request Overview

This PR implements the BiDi emulation module for Java, specifically focusing on geolocation override functionality. The implementation follows the W3C WebDriver BiDi emulation specification and provides a comprehensive API for controlling geolocation behavior in browser contexts.

Adds core emulation classes including Emulation, GeolocationCoordinates, and parameter classes
Implements geolocation override functionality with proper validation and error handling
Creates extensive test coverage for both browsing contexts and user contexts

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`java/src/org/openqa/selenium/bidi/emulation/Emulation.java`	Main emulation module class providing BiDi command execution
`java/src/org/openqa/selenium/bidi/emulation/GeolocationCoordinates.java`	Geolocation data class with coordinate validation
`java/src/org/openqa/selenium/bidi/emulation/SetGeolocationOverrideParameters.java`	Parameter class for geolocation override commands
`java/src/org/openqa/selenium/bidi/emulation/GeolocationPositionError.java`	Error representation for geolocation failures
`java/test/org/openqa/selenium/bidi/emulation/EmulationTest.java`	Comprehensive test suite covering various emulation scenarios
`java/src/org/openqa/selenium/bidi/emulation/BUILD.bazel`	Build configuration for emulation package
`java/test/org/openqa/selenium/bidi/emulation/BUILD.bazel`	Test build configuration
`java/src/org/openqa/selenium/remote/BUILD.bazel`	Updated remote package dependencies
`java/src/org/openqa/selenium/bidi/module/BUILD.bazel`	Updated module package dependencies

Copilot · 2025-07-18T11:40:18Z

java/src/org/openqa/selenium/bidi/emulation/SetGeolocationOverrideParameters.java

+  private final List<String> contexts;
+  private final List<String> userContexts;
+
+  SetGeolocationOverrideParameters(


The constructor has package-private visibility, which may limit API usability. Consider making it public to allow external instantiation of parameters.

Suggested change

SetGeolocationOverrideParameters(

public SetGeolocationOverrideParameters(

Copilot · 2025-07-18T11:40:19Z

java/src/org/openqa/selenium/bidi/emulation/GeolocationPositionError.java

+import java.util.Map;
+
+public class GeolocationPositionError {
+  String type = "positionUnavailable";


The field should be private and final since it's not intended to be modified. Consider adding 'private final' modifiers.

Suggested change

String type = "positionUnavailable";

private final String type = "positionUnavailable";

Copilot · 2025-07-18T11:40:19Z

java/test/org/openqa/selenium/bidi/emulation/EmulationTest.java

+    BrowsingContext context2 = new BrowsingContext(driver, WindowType.TAB);
+
+    GeolocationCoordinates coords =
+        new GeolocationCoordinates(45.5, -122.4194, 10.0, null, null, null, null);


The constructor call has 7 arguments but the GeolocationCoordinates constructor only accepts 6 parameters (excluding the 7-parameter constructor). This will cause a compilation error.

Suggested change

new GeolocationCoordinates(45.5, -122.4194, 10.0, null, null, null, null);

new GeolocationCoordinates(45.5, -122.4194, 10.0, null, null, null);

Copilot · 2025-07-18T11:40:19Z

java/test/org/openqa/selenium/bidi/emulation/EmulationTest.java

+            driver, new CreateContextParameters(WindowType.TAB).userContext(userContext2));
+
+    GeolocationCoordinates coords =
+        new GeolocationCoordinates(45.5, -122.4194, 10.0, null, null, null, null);


The constructor call has 7 arguments but the GeolocationCoordinates constructor only accepts 6 parameters (excluding the 7-parameter constructor). This will cause a compilation error.

Suggested change

new GeolocationCoordinates(45.5, -122.4194, 10.0, null, null, null, null);

new GeolocationCoordinates(45.5, -122.4194, 10.0, null, null, null);

Copilot · 2025-07-18T11:40:20Z

java/test/org/openqa/selenium/bidi/emulation/EmulationTest.java

+    assert !r.containsKey("error");
+
+    double latitude = ((Number) r.get("latitude")).doubleValue();
+    double longitude = ((Number) r.get("longitude")).doubleValue();
+    double accuracy = ((Number) r.get("accuracy")).doubleValue();
+
+    assert abs(latitude - coords.getLatitude()) < 0.0001;
+    assert abs(longitude - coords.getLongitude()) < 0.0001;
+    assert abs(accuracy - coords.getAccuracy()) < 0.0001;


Using assert statements in tests is not recommended. Consider using JUnit assertions like assertFalse() for better error reporting and test reliability.

Suggested change

assert !r.containsKey("error");

double latitude = ((Number) r.get("latitude")).doubleValue();

double longitude = ((Number) r.get("longitude")).doubleValue();

double accuracy = ((Number) r.get("accuracy")).doubleValue();

assert abs(latitude - coords.getLatitude()) < 0.0001;

assert abs(longitude - coords.getLongitude()) < 0.0001;

assert abs(accuracy - coords.getAccuracy()) < 0.0001;

assertFalse(r.containsKey("error"));

double latitude = ((Number) r.get("latitude")).doubleValue();

double longitude = ((Number) r.get("longitude")).doubleValue();

double accuracy = ((Number) r.get("accuracy")).doubleValue();

assertTrue(abs(latitude - coords.getLatitude()) < 0.0001);

assertTrue(abs(longitude - coords.getLongitude()) < 0.0001);

assertTrue(abs(accuracy - coords.getAccuracy()) < 0.0001);

Copilot · 2025-07-18T11:40:20Z

java/test/org/openqa/selenium/bidi/emulation/EmulationTest.java

+    assert !r.containsKey("error");
+
+    double latitude = ((Number) r.get("latitude")).doubleValue();
+    double longitude = ((Number) r.get("longitude")).doubleValue();
+    double accuracy = ((Number) r.get("accuracy")).doubleValue();
+
+    assert abs(latitude - coords.getLatitude()) < 0.0001;
+    assert abs(longitude - coords.getLongitude()) < 0.0001;
+    assert abs(accuracy - coords.getAccuracy()) < 0.0001;


Using assert statements in tests is not recommended. Consider using JUnit assertions like assertTrue() or assertEquals() with delta for better error reporting.

Suggested change

assert !r.containsKey("error");

double latitude = ((Number) r.get("latitude")).doubleValue();

double longitude = ((Number) r.get("longitude")).doubleValue();

double accuracy = ((Number) r.get("accuracy")).doubleValue();

assert abs(latitude - coords.getLatitude()) < 0.0001;

assert abs(longitude - coords.getLongitude()) < 0.0001;

assert abs(accuracy - coords.getAccuracy()) < 0.0001;

assertTrue(!r.containsKey("error"));

double latitude = ((Number) r.get("latitude")).doubleValue();

double longitude = ((Number) r.get("longitude")).doubleValue();

double accuracy = ((Number) r.get("accuracy")).doubleValue();

assertEquals(coords.getLatitude(), latitude, 0.0001);

assertEquals(coords.getLongitude(), longitude, 0.0001);

assertEquals(coords.getAccuracy(), accuracy, 0.0001);

[java][BiDi] implement emulation

3e93134

selenium-ci added C-java Java Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related labels Jul 18, 2025

Delta456 marked this pull request as draft July 18, 2025 11:33

qodo-merge-pro bot added the Review effort 3/5 label Jul 18, 2025

Delta456 requested a review from Copilot July 18, 2025 11:39

Copilot AI reviewed Jul 18, 2025

View reviewed changes

add license

7be9945

Delta456 force-pushed the java_bidi_emulation branch from e5300bf to 7be9945 Compare July 18, 2025 11:46

Delta456 added 7 commits July 18, 2025 17:16

Merge branch 'trunk' into java_bidi_emulation

6a8a069

add license

26a1129

Merge branch 'trunk' into java_bidi_emulation

2a8d42c

Merge branch 'trunk' into java_bidi_emulation

2ccc6db

try to fix remote

18ad7d8

try again

9159f6f

comment out code

216dde6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[java][BiDi] implement emulation #16070

[java][BiDi] implement emulation #16070

Uh oh!

Delta456 commented Jul 18, 2025 •

edited by qodo-merge-pro bot

Loading

Uh oh!

qodo-merge-pro bot commented Jul 18, 2025

Uh oh!

qodo-merge-pro bot commented Jul 18, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Uh oh!

	SetGeolocationOverrideParameters(
	public SetGeolocationOverrideParameters(

	String type = "positionUnavailable";
	private final String type = "positionUnavailable";

	new GeolocationCoordinates(45.5, -122.4194, 10.0, null, null, null, null);
	new GeolocationCoordinates(45.5, -122.4194, 10.0, null, null, null);

Uh oh!

[java][BiDi] implement emulation #16070

Are you sure you want to change the base?

[java][BiDi] implement emulation #16070

Uh oh!

Conversation

Delta456 commented Jul 18, 2025 • edited by qodo-merge-pro bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-merge-pro bot commented Jul 18, 2025

PR Reviewer Guide 🔍

Uh oh!

qodo-merge-pro bot commented Jul 18, 2025

PR Code Suggestions ✨

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Delta456 commented Jul 18, 2025 •

edited by qodo-merge-pro bot

Loading