SeleniumHQ / SeleniumHQ/selenium
[java] LoggingOptionsTest#captureStderrDuring doesn't clean up leaked root-logger handlers
- 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
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