SeleniumHQ / SeleniumHQ/selenium

[java] LoggingOptionsTest#captureStderrDuring doesn't clean up leaked root-logger handlers

Open Beginner friendly
#17,843 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
34.5k
Forks
8.7k
Avg merge
2d 1h
Merged PRs (30d)
92

Description

### Feature and motivation

Follow-up from #17841 (found by an AI agent while fixing two qodo-code-review findings on that PR's tests, deliberately left out to keep that PR scoped to the findings actually flagged there).

`java/test/org/openqa/selenium/grid/log/LoggingOptionsTest.java` has two stream-capture helpers: `captureStdOutAndErrDuring` and `captureStderrDuring`. `LoggingOptions.configureLogging()` installs root-logger handlers bound to whatever `System.out`/`System.err` are current at call time (via `getOutputStream()`). If a test temporarily swaps those streams to capture output, those handlers persist after the helper restores the real streams, keep writing into the now-discarded capture buffer, and can pollute later tests/output in the same JVM.

This exact bug was already fixed in `captureStdOutAndErrDuring` (PR #17841, snapshotting the root logger's handlers before the wrapped action and removing/closing anything added during it), but `captureStderrDuring` — the simpler, stderr-only variant used by other tests in the same file — was never given the same treatment.

### Suggested fix

Mirror `captureStdOutAndErrDuring`'s fix in `captureStderrDuring`: snapshot `LogManager.getLogManager().getLogger("").getHandlers()` before swapping `System.err`, and in the `finally` block, remove+close any handler that wasn't present in that snapshot.

### Context

Found while addressing qodo-code-review findings on #17841 (the same PR that fixed the sibling helper). Not itself flagged by a review comment, so tracked here rather than folded into that PR's scope.

Contributor guide

Open the contributing guide

Research direction

Open java/test/org/openqa/selenium/grid/log/LoggingOptionsTest.java and compare captureStderrDuring with the already-fixed captureStdOutAndErrDuring helper. Trace the root logger handlers around the temporary System.err swap, then run the relevant LoggingOptionsTest tests. Done means handlers added during capture are removed and closed during cleanup, preventing later output pollution.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
testing
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.