Skip to content

Conversation

asolntsev
Copy link
Contributor

@asolntsev asolntsev commented Sep 11, 2025

User description

🔗 Related Issues

Fixes #16314
Build failure: https://github.com/SeleniumHQ/selenium/actions/runs/17641086530/job/50128139484

💥 What does this PR do?

Fixes Spotbugs warning

🔧 Implementation Notes

I had to inline method findBinaryInClasspath() to make Spotbugs happy. I could not find any other way to suppress this falsy warning. :(

🔄 Types of changes

  • Bug fix (backwards compatible)

PR Type

Bug fix


Description

  • Inline findBinaryInClasspath() method to fix Spotbugs warning

  • Remove unused method to resolve build failure

  • Maintain same functionality with direct resource stream access


Diagram Walkthrough

flowchart LR
  A["findBinaryInClasspath() method"] --> B["Inline code"]
  B --> C["Direct getResourceAsStream() call"]
  C --> D["Spotbugs warning resolved"]
Loading

File Walkthrough

Relevant files
Bug fix
SeleniumManager.java
Inline binary classpath lookup method                                       

java/src/org/openqa/selenium/manager/SeleniumManager.java

  • Removed findBinaryInClasspath() method completely
  • Inlined resource stream logic directly in getBinary() method
  • Maintained same functionality with requireNonNull() wrapper
+3/-8     

I had to inline method `findBinaryInClasspath()` to make Spotbugs happy. I could not find any other way to suppress this falsy warning. :(
@asolntsev asolntsev self-assigned this Sep 11, 2025
@selenium-ci selenium-ci added the C-java Java Bindings label Sep 11, 2025
@asolntsev asolntsev added this to the Selenium 4.36 milestone Sep 11, 2025
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error Handling

Using requireNonNull without a message removes helpful diagnostics if the resource is missing. Consider the message-supplier overload or throwing a descriptive exception to aid debugging.

String binaryPathInJar = String.format("%s/%s%s", folder, SELENIUM_MANAGER, extension);
try (InputStream inputStream =
    requireNonNull(getClass().getResourceAsStream(binaryPathInJar))) {
Classpath Lookup

getClass().getResourceAsStream(binaryPathInJar) is package-relative; verify the resource path is correct in all packaging scenarios, or consider a leading '/' or using the class loader if absolute lookup is intended.

  requireNonNull(getClass().getResourceAsStream(binaryPathInJar))) {
Files.createDirectories(binary.getParent());

@cgoldberg cgoldberg changed the title #16314 fix falsy Spotbugs warning about not closed resource [java] Fix falsy Spotbugs warning about not closed resource Sep 11, 2025
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Add informative null-check message

Provide an explicit message to requireNonNull so missing resources fail with a
clear, actionable error instead of a generic NullPointerException.

java/src/org/openqa/selenium/manager/SeleniumManager.java [220-225]

 String binaryPathInJar = String.format("%s/%s%s", folder, SELENIUM_MANAGER, extension);
 try (InputStream inputStream =
-    requireNonNull(getClass().getResourceAsStream(binaryPathInJar))) {
+    requireNonNull(
+        getClass().getResourceAsStream(binaryPathInJar),
+        () -> "Resource not found in classpath: " + binaryPathInJar)) {
   Files.createDirectories(binary.getParent());
   saveToFileSafely(inputStream, binary);
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Add precise validation and defensive checks with clear error messages to aid debugging when expected resources are missing.

Low
  • More

@bonigarcia bonigarcia merged commit 6d3749b into SeleniumHQ:trunk Sep 11, 2025
35 checks passed
@asolntsev asolntsev deleted the fix/java-build-failure branch September 11, 2025 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: "Cannot run program "/home/runner/.cache/selenium/manager/0.4.35/selenium-manager": error=26, Text file busy"

3 participants