[java] manage state in tests not in global hooks

[titusfortner]

Came across this when I was trying to solve the Chrome bug in Java the same way I did Ruby (c1f500a) and JS (81ef99c)

🔗 Related Issues

• Removes unnecessary items added in fix flaky Ruby test element_spec.rb #16709, Avoid sleep in tests #16713, [java] Automatically save screenshots & page sources for failed tests #16904 & [java] Fix "secure vs non-secure" error in tests #17046

💥 What does this PR do?

• Manages browser state in the tests that change that state rather than in global hooks
• Removes unnecessary navigations before every test
• Restores blank.html to a blank page (title and empty body)

🔧 Implementation Notes

• CookieImplementationTest and StorageCommandsTest already wipe cookies in @BeforeEach, WindowSwitchingTest already closes extra windows in @AfterEach, and the two BiDi tests that closed the focused window now switch back; the tests that open extra windows now close them through a @CloseExtraWindowsAfterTest marker handled by SeleniumExtension beside @SwitchToTopAfterTest (class-level on the six BiDi classes that open tabs in most tests, method-level on the individual tests that switch into a context they close), the Java shape of the Ruby target_locator_spec hook, so a single-JVM IDE run no longer needs the base class to do any of it
• CloseExtraWindowsTest covers the cleanup both directly and through the annotation; the BiDi tests that ran their teardown inline now do it in finally, and the geolocation null-coordinates test no longer closes the session's only window; cleanup errors other than a dead session log at warning level
• DefaultMouseTest (classic and BiDi) resets the pointer with moveToLocation(0, 0); the old offset from the body's center only stayed inside the viewport because every test used to start on the blank page
• The three contextCreated tests now wait for the event of the window they open instead of taking the first event: the spec replays contextCreated for existing navigables on subscription, and the Java client registers its handler only after the subscribe response, so that replayed event sometimes lands first
• No binding has a web storage test and nothing reads the test-name header, so neither part of blank.html ever did anything; the Python wait tests need its blank title and the Firefox extension tests need a body, both kept

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s): Claude Code (Fable 5.1)
  • What was generated: the audit of which test classes set cookies or open windows and whether they clean up, the change, and this description
  • I reviewed all AI output and can explain the change

🔄 Types of changes

• Cleanup (test harness only; no user-facing change)

[qodo-code-review]

PR Summary by Qodo

Move Java browser-state cleanup out of JupiterTestBase

🧪 Tests 🕐 10-20 Minutes

AI Description

• Let stateful tests manage cookies and windows instead of resetting every Java test.
• Restore blank.html as an empty page while preserving its title and body.
• Return to the original window after two BiDi context-destruction tests close a window.

Diagram

graph TD
  EXT["Selenium extension"] --> BASE["Jupiter test base"] --> TESTS["Stateful Java tests"]
  TESTS --> COOKIES["Cookie tests"]
  TESTS --> WINDOWS["Window tests"]
  TESTS --> BIDI["BiDi tests"]
  BLANK["Blank page"] --> SHARED["Cross-binding tests"]

Loading

High-Level Assessment

Keep cleanup with tests that change browser state. Retaining the global reset would add navigation and driver commands to every test, while the existing cookie and window tests already manage their state; the two BiDi tests now restore their own window focus.

Files changed (4) +5 / -59

Refactor (1) +0 / -28

JupiterTestBase.javaRemove global browser-state resets +0/-28

Remove global browser-state resets

• Stops navigating to blank pages and deleting cookies before every test. Removes the shared after-test window switch, leaving state cleanup to the tests that need it.

java/test/org/openqa/selenium/testing/JupiterTestBase.java

Tests (3) +5 / -31

blank.htmlRestore the minimal blank-page fixture +1/-31

Restore the minimal blank-page fixture

• Removes the styled header and script that cleared web storage and displayed a test name. Preserves the blank title and an empty body for tests using the page.

common/src/web/blank.html

BrowsingContextInspectorTest.javaRestore the original window in the inspector test +2/-0

Restore the original window in the inspector test

• Saves the original window handle and switches back after closing the new window in the context-destruction test.

java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java

BrowsingContextModuleTest.javaRestore the original window in the protocol test +2/-0

Restore the original window in the protocol test

• Saves the original window handle and switches back after closing the window whose destruction event is under test.

java/test/org/openqa/selenium/bidi/protocol/module/BrowsingContextModuleTest.java

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

1. Window cleanup is not verified by tests ✓ Resolved

Description

CloseExtraWindowsRule.apply() adds teardown that closes windows and restores the initial window,
but no focused test checks the resulting window handles or selected window. The newly annotated
browser tests assert their own operations before teardown, so they can pass even if the next test
inherits an extra or incorrectly selected window.

Code

java/test/org/openqa/selenium/testing/SeleniumExtension.java[R436-441]

+        for (String handle : handles) {
+          if (!handle.equals(keep)) {
+            driver.switchTo().window(handle).close();
+          }
+        }
+        driver.switchTo().window(keep);

Evidence

Rule 3 calls for focused verification of changed behavior. The added teardown closes windows after
the annotated test body, while the testing package's small-test target lists only
IgnoreComparatorUnitTest.

AGENTS.md: Provide Focused Tests Without Misleading Mocks
java/test/org/openqa/selenium/testing/SeleniumExtension.java[417-444]
java/test/org/openqa/selenium/testing/BUILD.bazel[67-90]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new teardown behavior is not directly tested; the annotated tests finish before its result can be asserted.

## Fix Focus Areas
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[417-444]
- java/test/org/openqa/selenium/testing/BUILD.bazel[67-90]

## Recommended Fix
Add a focused test that exercises cleanup with an extra window and verifies that only the initial window remains selected afterward. Cover the case where the initial window has already closed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Later tests can start on a closed window ✓ Resolved

Description

CloseExtraWindowsRule.apply only acts when a class or method has @CloseExtraWindowsAfterTest,
and now that JupiterTestBase.switchToInitialWindow is gone it is the only thing that restores
window focus. But several classes that switch focus to a new browsing context and then close it were
never annotated, for example
SetGeolocationOverrideTest.canSetGeolocationOverrideWithMultipleUserContexts (no
@NeedsFreshDriver, ends after switchTo().window(context2) + context2.close()) and
SetTimezoneOverrideTest, SetNetworkConditionsTest, SetScreenOrientationOverrideTest,
StorageCommandsTest, DefaultWheelTest and BrowserCommandsTest. When the next test reuses the
same driver on that thread (single-JVM/IDE runs, which the PR says it is meant to support), its
first command fails with NoSuchWindowException because the driver is still pointed at the closed
window.

Code

java/test/org/openqa/selenium/testing/SeleniumExtension.java[R418-421]

+      if (findAnnotation(context.getTestClass(), CloseExtraWindowsAfterTest.class).isEmpty()
+          && findAnnotation(context.getTestMethod(), CloseExtraWindowsAfterTest.class).isEmpty()) {
+        return;
+      }

Evidence

The rule returns early unless the annotation is present. SetGeolocationOverrideTest switches focus
to context2 and closes it without switching back, and the class has no
@CloseExtraWindowsAfterTest. Before this PR, JupiterTestBase's switchToInitialWindow @AfterEach
covered this case. Other emulation tests also switch to a context and close it without being
annotated.

java/test/org/openqa/selenium/testing/SeleniumExtension.java[417-445]
java/test/org/openqa/selenium/bidi/emulation/SetGeolocationOverrideTest.java[103-163]
java/test/org/openqa/selenium/bidi/emulation/SetTimezoneOverrideTest.java[85-120]
java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java[71-104]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
With the global switch back to the initial window removed, only tests marked `@CloseExtraWindowsAfterTest` get their window focus restored. Several BiDi tests switch WebDriver focus to a new context and then close it, but they are not annotated. That leaves the reused driver pointed at a closed window.

## Fix Focus Areas
- java/test/org/openqa/selenium/bidi/emulation/SetGeolocationOverrideTest.java[40-41]
- java/test/org/openqa/selenium/bidi/emulation/SetTimezoneOverrideTest.java[1-60]
- java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java[1-50]
- java/test/org/openqa/selenium/bidi/emulation/SetScreenOrientationOverrideTest.java[1-50]
- java/test/org/openqa/selenium/bidi/emulation/SetScriptingEnabledTest.java[1-55]
- java/test/org/openqa/selenium/bidi/emulation/SetUserAgentOverrideTest.java[1-50]
- java/test/org/openqa/selenium/bidi/storage/StorageCommandsTest.java[90-165]
- java/test/org/openqa/selenium/bidi/input/DefaultWheelTest.java[85-175]
- java/test/org/openqa/selenium/bidi/browser/BrowserCommandsTest.java[175-190]

## Recommended Fix
Add `@CloseExtraWindowsAfterTest` at class level to every test class that calls `driver.switchTo().window(...)` or `switchTo().newWindow(...)` and does not switch back to the original handle. As an alternative, make `CloseExtraWindowsRule` always at least switch back to `initialWindowHandle` (or any remaining handle) when the current handle is gone, even when the class is not annotated.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Later tests use the wrong browser window ✓ Resolved

Description

Removing switchToInitialWindow() leaves
HistoryNavigationTest.testShouldDoNothingIfThereIsNothingToGoBackTo focused on the new window it
opens, and ImplicitWaitTest.testShouldRetainImplicitlyWaitFromTheReturnedWebDriverOfFrameSwitchTo
likewise ends in an extra window. When a later test reuses that driver on the same thread,
WindowSwitchingTest can record the extra window as its main window and close the original window
during its own cleanup.

Code

java/test/org/openqa/selenium/testing/JupiterTestBase.java[L97-99]

-  @AfterEach
-  final void switchToInitialWindow() {
-    if (driver == null) {

Evidence

The history test switches to its named new window and never switches back; the implicit-wait test
also selects another window without restoring the original. The extension reuses its thread-local
driver for ordinary subsequent tests, while WindowSwitchingTest takes the currently selected handle
as its main window and closes every other handle afterward. The deleted base-class hook previously
restored the initially selected handle after each test.

java/test/org/openqa/selenium/HistoryNavigationTest.java[31-47]
java/test/org/openqa/selenium/ImplicitWaitTest.java[145-163]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[267-298]
java/test/org/openqa/selenium/WindowSwitchingTest.java[38-65]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Removing the base-class window restoration leaves the shared driver focused on extra windows after tests that switch to them without restoring the original.
## Fix Focus Areas
- java/test/org/openqa/selenium/HistoryNavigationTest.java[31-47]
- java/test/org/openqa/selenium/ImplicitWaitTest.java[145-163]
- java/test/org/openqa/selenium/testing/JupiterTestBase.java[73-86]
## Recommended Fix
Capture the original handle in each affected test and restore it in a `finally` block, including when an assertion or wait fails. Close the extra window where the test does not need to retain it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Window-cleanup test fails when the driver is reused 🐞 Bug ☼ Reliability ⭐ New

Description

leavesExtraWindowsOpenForTheAnnotationToClose saves driver.getWindowHandle() as the window to
keep and expects exactly 3 handles. The rule instead keeps Instances.initialWindowHandle, the
handle captured when the driver was created. If an earlier test in the same JVM left focus on a
different window or left extra windows open, which is the single-JVM IDE run this PR is meant to
support, startsOnTheInitialWindowWithNothingElseOpen checks against the wrong handle and
hasSize(3) fails.

Code

java/test/org/openqa/selenium/CloseExtraWindowsTest.java[R40-45]

+  void leavesExtraWindowsOpenForTheAnnotationToClose() {
+    windowBeforeOpener = driver.getWindowHandle();
+    driver.switchTo().newWindow(WindowType.TAB);
+    driver.switchTo().newWindow(WindowType.WINDOW);
+
+    assertThat(driver.getWindowHandles()).hasSize(3);

Evidence

actuallyCreateDriver records the initial handle only when it creates a new driver, and
CloseExtraWindowsRule.apply passes that stored handle to closeExtraWindows. The test compares
against its own getWindowHandle() taken at the start of test 1, so the two handles differ whenever
the driver came from an earlier test.

java/test/org/openqa/selenium/testing/SeleniumExtension.java[291-306]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[432-448]
java/test/org/openqa/selenium/CloseExtraWindowsTest.java[38-55]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
CloseExtraWindowsTest saves the current window handle in test 1, but the cleanup rule keeps the handle captured when the driver was created. When the driver comes from an earlier test, test 1's size check and test 2's handle check can fail.

## Fix Focus Areas
- java/test/org/openqa/selenium/CloseExtraWindowsTest.java[36-55]

## Recommended Fix
Put `@NeedsFreshDriver` on test 1 (`leavesExtraWindowsOpenForTheAnnotationToClose`). That way the current handle is also the handle the rule keeps, and no earlier windows are open, so the test always starts from a known state.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

5. Failed window cleanup goes unnoticed 🐞 Bug ◔ Observability

Description

CloseExtraWindowsRule.apply catches window-cleanup WebDriverExceptions and logs them only at
Level.FINE, while closeExtraWindows restores the intended window only if its loop completes. If
closing or restoring a window fails, later tests can reuse the same driver with extra windows or the
wrong window selected, causing failures that do not visibly point back to cleanup.

Code

java/test/org/openqa/selenium/testing/SeleniumExtension.java[R441-445]

+      try {
+        closeExtraWindows(currentInstances.driver, currentInstances.initialWindowHandle);
+      } catch (WebDriverException e) {
+        LOG.log(Level.FINE, "Unable to close extra windows after " + displayName(context), e);
+      }

Evidence

closeExtraWindows switches to and closes extra window handles in a loop, then switches back to
keep at the end, so an exception can interrupt cleanup and skip that final switch.
CloseExtraWindowsRule.apply catches the exception but emits only a FINE log, and
actuallyCreateDriver returns the existing thread-local driver while its session remains live; the
next test can therefore inherit the remaining windows and focus.

java/test/org/openqa/selenium/testing/SeleniumExtension.java[337-350]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[432-446]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[432-445]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[294-305]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A failed annotated window cleanup is hidden at FINE level and can leave a contaminated driver available to subsequent tests.

## Fix Focus Areas
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[337-350]
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[441-445]
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[294-305]

## Recommended Fix
Log cleanup exceptions at `Level.WARNING` with the test display name. Handle each per-handle close independently so one failure does not stop the remaining closes, and attempt the final `driver.switchTo().window(keep)` in a `finally` block. If cleanup still fails, remove the driver so the next test creates a fresh session.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

Context sources

✅ Web pages:

  🌐 browser.removeUserContext command - WebDriver | MDN

  🌐 user_context.py - mozsearch

  🌐 1874918 - Support "userContext" parameter for "browsingContext.create" command

  +2 more

Review mode: Auto: 🧠 Deep: Broad test-harness, CI, and release-script logic has multiple independent failure paths.

Tip of the day

💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 4ddf059

Results up to commit 6e50092 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Later tests use the wrong browser window ✓ Resolved

Description

Removing switchToInitialWindow() leaves
HistoryNavigationTest.testShouldDoNothingIfThereIsNothingToGoBackTo focused on the new window it
opens, and ImplicitWaitTest.testShouldRetainImplicitlyWaitFromTheReturnedWebDriverOfFrameSwitchTo
likewise ends in an extra window. When a later test reuses that driver on the same thread,
WindowSwitchingTest can record the extra window as its main window and close the original window
during its own cleanup.

Code

java/test/org/openqa/selenium/testing/JupiterTestBase.java[L97-99]

-  @AfterEach
-  final void switchToInitialWindow() {
-    if (driver == null) {

Evidence

The history test switches to its named new window and never switches back; the implicit-wait test
also selects another window without restoring the original. The extension reuses its thread-local
driver for ordinary subsequent tests, while WindowSwitchingTest takes the currently selected handle
as its main window and closes every other handle afterward. The deleted base-class hook previously
restored the initially selected handle after each test.

java/test/org/openqa/selenium/HistoryNavigationTest.java[31-47]
java/test/org/openqa/selenium/ImplicitWaitTest.java[145-163]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[267-298]
java/test/org/openqa/selenium/WindowSwitchingTest.java[38-65]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Removing the base-class window restoration leaves the shared driver focused on extra windows after tests that switch to them without restoring the original.
## Fix Focus Areas
- java/test/org/openqa/selenium/HistoryNavigationTest.java[31-47]
- java/test/org/openqa/selenium/ImplicitWaitTest.java[145-163]
- java/test/org/openqa/selenium/testing/JupiterTestBase.java[73-86]
## Recommended Fix
Capture the original handle in each affected test and restore it in a `finally` block, including when an assertion or wait fails. Close the extra window where the test does not need to retain it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit 0739977 🧠 Deep

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Window cleanup is not verified by tests ✓ Resolved

Description

CloseExtraWindowsRule.apply() adds teardown that closes windows and restores the initial window,
but no focused test checks the resulting window handles or selected window. The newly annotated
browser tests assert their own operations before teardown, so they can pass even if the next test
inherits an extra or incorrectly selected window.

Code

java/test/org/openqa/selenium/testing/SeleniumExtension.java[R436-441]

+        for (String handle : handles) {
+          if (!handle.equals(keep)) {
+            driver.switchTo().window(handle).close();
+          }
+        }
+        driver.switchTo().window(keep);

Evidence

Rule 3 calls for focused verification of changed behavior. The added teardown closes windows after
the annotated test body, while the testing package's small-test target lists only
IgnoreComparatorUnitTest.

AGENTS.md: Provide Focused Tests Without Misleading Mocks
java/test/org/openqa/selenium/testing/SeleniumExtension.java[417-444]
java/test/org/openqa/selenium/testing/BUILD.bazel[67-90]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new teardown behavior is not directly tested; the annotated tests finish before its result can be asserted.

## Fix Focus Areas
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[417-444]
- java/test/org/openqa/selenium/testing/BUILD.bazel[67-90]

## Recommended Fix
Add a focused test that exercises cleanup with an extra window and verifies that only the initial window remains selected afterward. Cover the case where the initial window has already closed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Later tests can start on a closed window ✓ Resolved

Description

CloseExtraWindowsRule.apply only acts when a class or method has @CloseExtraWindowsAfterTest,
and now that JupiterTestBase.switchToInitialWindow is gone it is the only thing that restores
window focus. But several classes that switch focus to a new browsing context and then close it were
never annotated, for example
SetGeolocationOverrideTest.canSetGeolocationOverrideWithMultipleUserContexts (no
@NeedsFreshDriver, ends after switchTo().window(context2) + context2.close()) and
SetTimezoneOverrideTest, SetNetworkConditionsTest, SetScreenOrientationOverrideTest,
StorageCommandsTest, DefaultWheelTest and BrowserCommandsTest. When the next test reuses the
same driver on that thread (single-JVM/IDE runs, which the PR says it is meant to support), its
first command fails with NoSuchWindowException because the driver is still pointed at the closed
window.

Code

java/test/org/openqa/selenium/testing/SeleniumExtension.java[R418-421]

+      if (findAnnotation(context.getTestClass(), CloseExtraWindowsAfterTest.class).isEmpty()
+          && findAnnotation(context.getTestMethod(), CloseExtraWindowsAfterTest.class).isEmpty()) {
+        return;
+      }

Evidence

The rule returns early unless the annotation is present. SetGeolocationOverrideTest switches focus
to context2 and closes it without switching back, and the class has no
@CloseExtraWindowsAfterTest. Before this PR, JupiterTestBase's switchToInitialWindow @AfterEach
covered this case. Other emulation tests also switch to a context and close it without being
annotated.

java/test/org/openqa/selenium/testing/SeleniumExtension.java[417-445]
java/test/org/openqa/selenium/bidi/emulation/SetGeolocationOverrideTest.java[103-163]
java/test/org/openqa/selenium/bidi/emulation/SetTimezoneOverrideTest.java[85-120]
java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java[71-104]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
With the global switch back to the initial window removed, only tests marked `@CloseExtraWindowsAfterTest` get their window focus restored. Several BiDi tests switch WebDriver focus to a new context and then close it, but they are not annotated. That leaves the reused driver pointed at a closed window.

## Fix Focus Areas
- java/test/org/openqa/selenium/bidi/emulation/SetGeolocationOverrideTest.java[40-41]
- java/test/org/openqa/selenium/bidi/emulation/SetTimezoneOverrideTest.java[1-60]
- java/test/org/openqa/selenium/bidi/emulation/SetNetworkConditionsTest.java[1-50]
- java/test/org/openqa/selenium/bidi/emulation/SetScreenOrientationOverrideTest.java[1-50]
- java/test/org/openqa/selenium/bidi/emulation/SetScriptingEnabledTest.java[1-55]
- java/test/org/openqa/selenium/bidi/emulation/SetUserAgentOverrideTest.java[1-50]
- java/test/org/openqa/selenium/bidi/storage/StorageCommandsTest.java[90-165]
- java/test/org/openqa/selenium/bidi/input/DefaultWheelTest.java[85-175]
- java/test/org/openqa/selenium/bidi/browser/BrowserCommandsTest.java[175-190]

## Recommended Fix
Add `@CloseExtraWindowsAfterTest` at class level to every test class that calls `driver.switchTo().window(...)` or `switchTo().newWindow(...)` and does not switch back to the original handle. As an alternative, make `CloseExtraWindowsRule` always at least switch back to `initialWindowHandle` (or any remaining handle) when the current handle is gone, even when the class is not annotated.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit a476b8f 🧠 Deep

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Failed window cleanup goes unnoticed 🐞 Bug ◔ Observability

Description

CloseExtraWindowsRule.apply catches window-cleanup WebDriverExceptions and logs them only at
Level.FINE, while closeExtraWindows restores the intended window only if its loop completes. If
closing or restoring a window fails, later tests can reuse the same driver with extra windows or the
wrong window selected, causing failures that do not visibly point back to cleanup.

Code

java/test/org/openqa/selenium/testing/SeleniumExtension.java[R441-445]

+      try {
+        closeExtraWindows(currentInstances.driver, currentInstances.initialWindowHandle);
+      } catch (WebDriverException e) {
+        LOG.log(Level.FINE, "Unable to close extra windows after " + displayName(context), e);
+      }

Evidence

closeExtraWindows switches to and closes extra window handles in a loop, then switches back to
keep at the end, so an exception can interrupt cleanup and skip that final switch.
CloseExtraWindowsRule.apply catches the exception but emits only a FINE log, and
actuallyCreateDriver returns the existing thread-local driver while its session remains live; the
next test can therefore inherit the remaining windows and focus.

java/test/org/openqa/selenium/testing/SeleniumExtension.java[337-350]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[432-446]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[432-445]
java/test/org/openqa/selenium/testing/SeleniumExtension.java[294-305]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A failed annotated window cleanup is hidden at FINE level and can leave a contaminated driver available to subsequent tests.

## Fix Focus Areas
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[337-350]
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[441-445]
- java/test/org/openqa/selenium/testing/SeleniumExtension.java[294-305]

## Recommended Fix
Log cleanup exceptions at `Level.WARNING` with the test display name. Handle each per-handle close independently so one failure does not stop the remaining closes, and attempt the final `driver.switchTo().window(keep)` in a `finally` block. If cleanup still fails, remove the driver so the next test creates a fresh session.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit 0739977

[qodo-code-review]

Code review by qodo was updated up to the latest commit a476b8f

[qodo-code-review]

Code review by qodo was updated up to the latest commit dca64f8

[qodo-code-review]

Code review by qodo was updated up to the latest commit 4ddf059