[java][bidi]: add test for onHistoryUpdated event

[navin772]

User description

🔗 Related Issues

💥 What does this PR do?

Adds test for onHistoryUpdated event.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

• Tests

---

PR Type

Tests

---

Description

• Add test for onHistoryUpdated BiDi event

• Test uses history.pushState to trigger event

• Validates browsing context ID and URL changes

---

Diagram Walkthrough

flowchart LR
  A["Test Setup"] --> B["Navigate to Page"]
  B --> C["Register Event Listener"]
  C --> D["Execute history.pushState"]
  D --> E["Verify Event Data"]

Loading

File Walkthrough

Relevant files

Tests

BrowsingContextInspectorTest.java Add history updated event test                                                      java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java Add import for Optional and Script module Add canListenToHistoryUpdatedEvent test method Test registers listener and triggers event with history.pushState Validates event contains correct browsing context ID and URL +25/-0

---

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 5678 - Partially compliant Compliant requirements: Non-compliant requirements: Diagnose and address "ConnectFailure" errors for multiple ChromeDriver instances. Ensure subsequent ChromeDriver instantiations do not produce connection refused errors. Provide environment-specific fix/workaround for Ubuntu 16.04.4 with given versions. Validate behavior across multiple driver instances. Requires further human verification: 1234 - Partially compliant Compliant requirements: Non-compliant requirements: Investigate and fix JS-in-href click behavior regression in 2.48.* with Firefox 42. Reproduce and validate behavior. Requires further human verification:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Flaky Test Risk The test relies on receiving a HistoryUpdated event within 5 seconds after history.pushState; event timing could be flaky on slower environments. Consider increasing timeout or adding retry/wait for readiness to reduce flakiness. @Test @NeedsFreshDriver void canListenToHistoryUpdatedEvent() throws ExecutionException, InterruptedException, TimeoutException { try (BrowsingContextInspector inspector = new BrowsingContextInspector(driver); Script script = new Script(driver)) { CompletableFuture<HistoryUpdated> future = new CompletableFuture<>(); BrowsingContext context = new BrowsingContext(driver, driver.getWindowHandle()); context.navigate(appServer.whereIs("/simpleTest.html"), ReadinessState.COMPLETE); inspector.onHistoryUpdated(future::complete); // Use history.pushState to trigger history updated event script.evaluateFunctionInBrowsingContext( context.getId(), "history.pushState({}, '', '/new-path')", false, Optional.empty()); HistoryUpdated historyUpdated = future.get(5, TimeUnit.SECONDS); assertThat(historyUpdated.getBrowsingContextId()).isEqualTo(context.getId()); assertThat(historyUpdated.getUrl()).contains("/new-path"); } API Usage Assumption The evaluateFunctionInBrowsingContext call uses "history.pushState({}, '', '/new-path')" without awaiting completion or verifying execution result; if execution is async or sandboxed, event may not fire. Consider checking evaluation result or ensuring user activation/context permissions if required. // Use history.pushState to trigger history updated event script.evaluateFunctionInBrowsingContext( context.getId(), "history.pushState({}, '', '/new-path')", false, Optional.empty()); HistoryUpdated historyUpdated = future.get(5, TimeUnit.SECONDS); assertThat(historyUpdated.getBrowsingContextId()).isEqualTo(context.getId()); assertThat(historyUpdated.getUrl()).contains("/new-path");

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Remove non-closeable from resources Avoid using try-with-resources for a type that does not implement AutoCloseable. Instantiate Script as a regular variable inside the block so the code compiles across Selenium versions. java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java [289-290] -try (BrowsingContextInspector inspector = new BrowsingContextInspector(driver); - Script script = new Script(driver)) { +try (BrowsingContextInspector inspector = new BrowsingContextInspector(driver)) { + Script script = new Script(driver); Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that Script does not implement AutoCloseable and therefore cannot be used in a try-with-resources statement, which would cause a compilation error.	High
	Wrap expression in arrow function evaluateFunctionInBrowsingContext expects a function declaration, not a raw expression. Wrap the call in an arrow function to prevent an "invalid functionDeclaration" error at runtime. java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java [299-300] script.evaluateFunctionInBrowsingContext( - context.getId(), "history.pushState({}, '', '/new-path')", false, Optional.empty()); + context.getId(), "() => history.pushState({}, '', '/new-path')", false, Optional.empty()); Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly points out that evaluateFunctionInBrowsingContext expects a function declaration, and the provided code passes a raw expression, which would cause a runtime error. The proposed fix is correct.	High
Learned best practice	Use precise URL path assertion Parse the URL and assert on its path to validate precisely that the navigation updated the expected path segment. java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java [304] -assertThat(historyUpdated.getUrl()).contains("/new-path"); +assertThat(java.net.URI.create(historyUpdated.getUrl()).getPath()).isEqualTo("/new-path"); Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add precise validation checks instead of substring assertions to reduce false positives.	Low
Update

[navin772]

I will merge this since the testing logic is similar to what we do in python.