[java] Speed up tests, improve test logging and build script

[asolntsev]

User description

💥 What does this PR do?

Small improvements:

1. speed up tests: run "bazel build" only if the target file doesn't yet exist.
   • We were wasting ~1.1 seconds PER TEST for re-running the same "bazel build" commands before every test!
2. log the error if failed to close the browser
3. improve logging when preparing a browser for test
4. bugfix in build script: build "webdriver_prefs.json" before copying it
5. bugfix in build script: add missing dependency to javadoc generator

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)

---

PR Type

Bug fix, Enhancement

---

Description

• Speed up tests by skipping redundant bazel builds when target files exist

• Improve logging with lazy evaluation and better error context

• Fix browser quit exception handling with proper error logging

• Fix build script bugs: correct bazel target and add missing dependency

• Enhance test logging with full class names for better traceability

---

Diagram Walkthrough

flowchart LR
  A["Test Execution"] -->|Skip build if target exists| B["Faster Tests"]
  C["Exception Handling"] -->|Log errors properly| D["Better Debugging"]
  E["Build Scripts"] -->|Fix targets & dependencies| F["Correct Builds"]
  G["Logging"] -->|Lazy evaluation & context| H["Improved Traceability"]

Loading

File Walkthrough

	Relevant files
Error handling	JupiterTestBase.java Add exception logging to appServer restart handling            java/test/org/openqa/selenium/testing/JupiterTestBase.java Import Level for structured logging Log exceptions with proper severity level when appServer restart fails Change from ignoring to logging warnings with exception context +2/-1      OutOfProcessSeleniumServer.java Add proper exception logging for server startup                    java/test/org/openqa/selenium/testing/drivers/OutOfProcessSeleniumServer.java Import Level for structured logging Log server startup failures with proper severity and exception context Add logging for runfiles location resolution failures +3/-1
Enhancement	SeleniumExtension.java Improve logging with lazy evaluation and better context    java/test/org/openqa/selenium/testing/SeleniumExtension.java Import Level and @NonNull annotation Convert string concatenation logging to lazy evaluation with lambdas Add displayName() helper method to include class name in test logs Log browser quit failures with proper severity level +17/-6    StaticResources.java Skip redundant bazel builds when targets exist                      java/test/org/openqa/selenium/testing/StaticResources.java Refactor copy() method to accept Runnable build parameter Only execute bazel build if target file doesn't already exist Fix bazel target from webdriver_xpi to webdriver_json for prefs file Improve code organization with better formatting +18/-11  ExternalDriverSupplier.java Improve error handling and logging clarity                              java/test/org/openqa/selenium/testing/drivers/ExternalDriverSupplier.java Add explicit RuntimeException re-throw in exception handlers Improve error messages with server URL context Convert string concatenation to lazy evaluation in logging +8/-3
Bug fix	BUILD.bazel Fix missing dependency for javadoc generator                          java/src/dev/selenium/tools/javadoc/BUILD.bazel Fix dependency from //java/src/org/openqa/selenium/os to //java/src/org/openqa/selenium/io Correct module reference for TemporaryFilesystem class used by JavadocJarMaker +1/-1
Configuration changes	BUILD.bazel Add javadoc tool visibility to io module                                  java/src/org/openqa/selenium/io/BUILD.bazel Add visibility for //java/src/dev/selenium/tools/javadoc:__pkg__ Allow javadoc tool to access io module classes +1/-0
Dependencies	BUILD.bazel Add jspecify dependency for annotations                                    java/test/org/openqa/selenium/testing/BUILD.bazel Add org.jspecify:jspecify artifact dependency Support @NonNull annotation usage in test code +1/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
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: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed 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: 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: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Potential sensitive logging: The new log line LOG.info(() -> "CREATED " + driver) may serialize potentially sensitive runtime details from driver.toString() (e.g., remote endpoints/session identifiers), which should be verified as safe for logs. Referred Code LOG.info("CREATING DRIVER"); WebDriver driver = actuallyCreateDriver(); LOG.info(() -> "CREATED " + driver); return driver; 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
Possible issue	Propagate exception instead of falling through In the catch (IOException e) block, re-throw the exception as a RuntimeException instead of allowing execution to fall through, ensuring a fail-fast behavior with a clear stack trace. java/test/org/openqa/selenium/testing/drivers/OutOfProcessSeleniumServer.java [179-182] } catch (IOException e) { - LOG.log(Level.INFO, "Failed to build server", e); - // Fall through + LOG.log(Level.WARNING, "Failed to find server binary.", e); + throw new RuntimeException(e); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a significant issue where an IOException is caught and logged, but the program continues, likely leading to a less informative AssertionError later. Failing fast by re-throwing the exception improves error reporting and debuggability.	Medium
General	Check build outputs exist Add a check to verify the source file exists after the build runs and before attempting to copy it, throwing an IllegalStateException if it is not found. java/test/org/openqa/selenium/testing/StaticResources.java [89-91] build.run(); +if (!Files.exists(source)) { + throw new IllegalStateException("Expected source file not found: " + source); +} Files.createDirectories(dest.getParent()); Files.copy(source, dest); Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion improves robustness by adding an explicit check for the build artifact's existence before attempting a file copy. This provides a clearer, more immediate error message if the build fails silently, which is a good defensive programming practice.	Medium
	Use lazy logging Convert the LOG.info("CREATING DRIVER") statement to use a lambda (LOG.info(() -> "CREATING DRIVER")) for lazy evaluation, consistent with other log calls in the file. java/test/org/openqa/selenium/testing/SeleniumExtension.java [263] -LOG.info("CREATING DRIVER"); +LOG.info(() -> "CREATING DRIVER"); Apply / Chat Suggestion importance[1-10]: 4 __ Why: The suggestion correctly points out an inconsistency with other logging calls in the file and proposes using a lambda for lazy evaluation, which is a good practice for performance. However, this is a minor style and performance optimization.	Low
Learned best practice	Validate system property inputs Trim and validate the system property value (non-blank) before using it, so empty/whitespace values don't trigger confusing ClassNotFoundException/logging. java/test/org/openqa/selenium/testing/drivers/ExternalDriverSupplier.java [118-131] String delegateClassName = System.getProperty(DELEGATE_SUPPLIER_CLASS_PROPERTY); if (delegateClassName != null) { + delegateClassName = delegateClassName.trim(); +} +if (delegateClassName != null && !delegateClassName.isEmpty()) { try { LOG.info("Loading custom supplier: " + delegateClassName); Class<? extends Supplier<WebDriver>> clazz = (Class<? extends Supplier<WebDriver>>) Class.forName(delegateClassName); return Optional.of(clazz); } catch (RuntimeException e) { throw e; } catch (Exception e) { throw new RuntimeException(e); } } return Optional.empty(); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (e.g., system properties) by trimming inputs and checking presence before use.	Low
	Guard against missing runfile paths Guard against rlocation returning null/blank and avoid calling Paths.get with invalid values; log and fall through cleanly when location is unavailable. java/test/org/openqa/selenium/testing/drivers/OutOfProcessSeleniumServer.java [171-178] Runfiles.Preloaded runfiles = Runfiles.preload(); String location = runfiles.unmapped().rlocation("_main/java/src/org/openqa/selenium/grid/selenium_server"); +if (location != null) { + location = location.trim(); +} System.err.println("Location found is: " + location); -Path path = Paths.get(location); -if (Files.exists(path)) { - return location; +if (location != null && !location.isEmpty()) { + Path path = Paths.get(location); + if (Files.exists(path)) { + return location; + } } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (e.g., runfiles/env/system-resolved paths) by checking presence before use.	Low
Update