[asolntsev]

User description

🔗 Related Issues

Fixes #16932

💥 What does this PR do?

This PR fixes SeleniumExtension, so that Selenium logs are written again when running Selenium own tests.

🔧 Implementation Notes

Now SeleniumExtension works as follows:

1. Replace ConsoleHandlers by RecordingHandler before each test
2. Restore original ConsoleHandlers after each test
3. For failed tests - write down ALL logs
4. For succeeded tests - write down WARN and ERROR logs (they might be also useful)

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Fix logging capture in Selenium tests by refactoring CaptureLoggingRule

• Replace console handlers with recording handler before each test

• Restore original handlers after each test completion

• Write all logs for failed tests, warn/error logs for passed tests

---

Diagram Walkthrough

flowchart LR
  A["Test Execution"] -->|beforeEach| B["Start Log Capture"]
  B -->|Remove ConsoleHandlers| C["Add RecordingHandler"]
  C -->|Test Runs| D["Logs Recorded"]
  D -->|afterEach| E["End Log Capture"]
  E -->|Check Test Status| F{Test Failed?}
  F -->|Yes| G["Write ALL Logs"]
  F -->|No| H["Write WARN/ERROR Logs"]
  G -->|Restore Handlers| I["Original Handlers"]
  H -->|Restore Handlers| I

Loading

File Walkthrough

	Relevant files
Bug fix	CaptureLoggingRule.java Refactor logging capture with selective level-based output java/test/org/openqa/selenium/testing/CaptureLoggingRule.java Refactored to use instance logger instead of repeated lookups Changed constructor to startLogCapture() method for lifecycle management Added Level parameter to writeCapturedLogs() and endLogCapture() for selective logging RecordingHandler now stores original handlers to restore them after test Added generic handlers() helper method to filter handlers by type Modified write() method to filter logs by level before output +39/-31  SeleniumExtension.java Integrate selective log capture based on test outcome        java/test/org/openqa/selenium/testing/SeleniumExtension.java Initialize captureLoggingRule as instance field instead of in constructor Call startLogCapture() in beforeEach() to capture logs before test execution Modified afterEach() to determine log level based on test failure status Pass appropriate log level to endLogCapture() - Level.ALL for failed tests, Level.WARNING for passed tests Removed redundant writeCapturedLogs() call from testFailed() method Removed unnecessary blank lines for code cleanup +8/-15
Formatting	JupiterTestBase.java Make SeleniumExtension field final                                              java/test/org/openqa/selenium/testing/JupiterTestBase.java Made seleniumExtension field final for immutability Reformatted @RegisterExtension annotation placement +2/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Sensitive log exposure Description: Captured logs are written directly to System.out and, when invoked with Level.ALL (used for failed tests), may expose sensitive data (e.g., credentials, session tokens, internal URLs) into CI logs or other persisted test output. CaptureLoggingRule.java [59-109] Referred Code private void writeCapturedLogs(Level writeLogsHigherThan) { handlers(RecordingHandler.class) .forEach(recordingHandler -> recordingHandler.write(writeLogsHigherThan)); } public void endLogCapture(Level writeLogsHigherThan) { writeCapturedLogs(writeLogsHigherThan); if (isDebugging()) { return; } // Find our recording handler List<RecordingHandler> recordingHandlers = handlers(RecordingHandler.class); // Restore the original handlers for (RecordingHandler recordingHandler : recordingHandlers) { logger.removeHandler(recordingHandler); recordingHandler.originalHandlers.forEach(logger::addHandler); } } ... (clipped 30 lines)
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Possible double teardown: captureLoggingRule.endLogCapture(...) is invoked in both afterEach and testFailed, which may result in double-removal/restoration of handlers depending on JUnit callback ordering and could lead to inconsistent logging state. Referred Code public void afterEach(ExtensionContext context) throws Exception { // SwitchToTopRule SwitchToTopRule switchToTopRule = new SwitchToTopRule(context); switchToTopRule.apply(); Level logLevel = context.getExecutionException().map(testFailed -> Level.ALL).orElse(Level.WARNING); captureLoggingRule.endLogCapture(logLevel); LOG.info(() -> "<<< Finished " + displayName(context)); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Stacktrace exposure risk: The new behavior explicitly writes all captured logs on failures (Level.ALL), which can include exception stack traces and internal details in test output that may be undesirable in some environments (e.g., shared CI logs). Referred Code Level logLevel = context.getExecutionException().map(testFailed -> Level.ALL).orElse(Level.WARNING); captureLoggingRule.endLogCapture(logLevel); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Unfiltered log dumping: RecordingHandler.write(Level.ALL) prints all recorded messages to System.out, which may unintentionally include secrets/PII present in logs when tests fail, so safeguards or redaction may be needed depending on CI log exposure. Referred Code public void write(Level writeLogsHigherThan) { Formatter formatter = new OurFormatter(); records.stream() .filter(record -> record.getLevel().intValue() >= writeLogsHigherThan.intValue()) .forEach(record -> System.out.print(formatter.format(record))); } Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Remove redundant log capture call In testFailed, remove the redundant call to captureLoggingRule.endLogCapture(Level.ALL) because the afterEach method already handles ending the log capture for both passed and failed tests. java/test/org/openqa/selenium/testing/SeleniumExtension.java [191-209] @Override public void testFailed(ExtensionContext context, Throwable cause) { TestWatcher.super.testFailed(context, cause); // ManageDriverRule.failed Browser current = Objects.requireNonNull(Browser.detect()); Optional<Method> testMethod = context.getTestMethod(); Optional<NoDriverAfterTest> noDriverAfterTest = findAnnotation(testMethod, NoDriverAfterTest.class); if (noDriverAfterTest.isPresent()) { NoDriverAfterTest annotation = noDriverAfterTest.get(); if (current.matches(annotation.value())) { System.out.println("Restarting driver after failed test " + context.getDisplayName()); removeDriver(); } } - - captureLoggingRule.endLogCapture(Level.ALL); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that endLogCapture is called twice for failed tests, once in afterEach and again in testFailed, which is redundant and can cause logs to be printed multiple times.	Medium
	Clear log records after writing In the write method of RecordingHandler, clear the records list after printing the logs to free up memory and prevent duplicate output. java/test/org/openqa/selenium/testing/CaptureLoggingRule.java [104-109] public void write(Level writeLogsHigherThan) { Formatter formatter = new OurFormatter(); records.stream() .filter(record -> record.getLevel().intValue() >= writeLogsHigherThan.intValue()) .forEach(record -> System.out.print(formatter.format(record))); + records.clear(); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: This is a good suggestion for improving resource management by clearing the log records after they are written, preventing potential memory leaks and ensuring logs are not printed multiple times if write is called again.	Medium
	Skip capture in debug mode In endLogCapture, move the isDebugging() check to the beginning of the method to avoid unnecessarily writing logs when debugging. java/test/org/openqa/selenium/testing/CaptureLoggingRule.java [64-69] public void endLogCapture(Level writeLogsHigherThan) { - writeCapturedLogs(writeLogsHigherThan); if (isDebugging()) { return; } + writeCapturedLogs(writeLogsHigherThan); // ... } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out that moving the isDebugging() check to the top of endLogCapture improves efficiency by avoiding an unnecessary call to writeCapturedLogs in debug mode.	Low
Learned best practice	Defensive copy stored collections Store a defensive immutable copy of originalHandlers (e.g., List.copyOf) so future mutations of the passed list cannot affect handler restoration behavior. java/test/org/openqa/selenium/testing/CaptureLoggingRule.java [85-87] -private final List<? extends Handler> originalHandlers; +private final List<Handler> originalHandlers; ... private RecordingHandler(List<? extends Handler> originalHandlers) { - this.originalHandlers = originalHandlers; + this.originalHandlers = List.copyOf(originalHandlers); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 5 __ Why: Relevant best practice - When storing collection-like state from callers, store a defensive/immutable copy to prevent accidental external mutation.	Low
Update

[bonigarcia]

LGTM. Thanks, @asolntsev!