Skip to content

[java] Fix falsy Spotbugs warning about not closed resource#16318

Merged
bonigarcia merged 1 commit intoSeleniumHQ:trunkfrom
asolntsev:fix/java-build-failure
Sep 11, 2025
Merged

[java] Fix falsy Spotbugs warning about not closed resource#16318
bonigarcia merged 1 commit intoSeleniumHQ:trunkfrom
asolntsev:fix/java-build-failure

Conversation

@asolntsev
Copy link
Contributor

@asolntsev asolntsev commented Sep 11, 2025 •
edited by cgoldberg
Loading

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     

@asolntsev
SeleniumHQ#16314 fix falsy Spotbugs warning about not closed resource
b14eee4 
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
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Sep 11, 2025

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
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Sep 11, 2025

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

Reviewers

@cgoldberg cgoldberg Awaiting requested review from cgoldberg

@bonigarcia bonigarcia Awaiting requested review from bonigarcia

Assignees

@asolntsev asolntsev

Labels

C-java Java Bindings Review effort 1/5

Projects

Selenium Manager
Status: Done

Milestone

Selenium 4.36

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

@asolntsev @selenium-ci @bonigarcia