Fix SpotBugs issues + Fix the default Java versioncapturing in PluginCompatTesterConfig#getTestJavaVersion() when other options are disabled #203
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[ERROR] org.jenkins.tools.test.maven.ExternalMavenRunner.run(MavenRunner$Config, File, File, String[]) may fail to close stream [org.jenkins.tools.test.maven.ExternalMavenRunner] At ExternalMavenRunner.java:[line 46] OS_OPEN_STREAM
[ERROR] org.jenkins.tools.test.PluginCompatTester.addSplitPluginDependencies(String, MavenRunner$Config, File, MavenPom, Map, Map, String, List) may fail to close stream [org.jenkins.tools.test.PluginCompatTester] At PluginCompatTester.java:[line 707] OS_OPEN_STREAM
[ERROR] Found reliance on default encoding in new org.jenkins.tools.test.logging.SystemIOLoggerFilter$SystemIOWrapper(SystemIOLoggerFilter, PrintStream): new org.jenkins.tools.test.logging.SystemIOLoggerFilter$SystemIOWrapper(OutputStream) [org.jenkins.tools.test.logging.SystemIOLoggerFilter$SystemIOWrapper] At SystemIOLoggerFilter.java:[line 58] DM_DEFAULT_ENCODING
[ERROR] Found reliance on default encoding in org.jenkins.tools.test.maven.ExternalMavenRunner.run(MavenRunner$Config, File, File, String[]): new java.io.InputStreamReader(InputStream) [org.jenkins.tools.test.maven.ExternalMavenRunner] At ExternalMavenRunner.java:[line 46] DM_DEFAULT_ENCODING
[ERROR] Found reliance on default encoding in org.jenkins.tools.test.maven.ExternalMavenRunner.run(MavenRunner$Config, File, File, String[]): new java.io.PrintWriter(OutputStream) [org.jenkins.tools.test.maven.ExternalMavenRunner] At ExternalMavenRunner.java:[line 43] DM_DEFAULT_ENCODING
[ERROR] Found reliance on default encoding in org.jenkins.tools.test.model.MavenPom.writeDocument(File, Document): new java.io.FileWriter(File) [org.jenkins.tools.test.model.MavenPom] At MavenPom.java:[line 247] DM_DEFAULT_ENCODING
[ERROR] Found reliance on default encoding in org.jenkins.tools.test.model.PluginCompatReport.save(File): new java.io.FileWriter(File) [org.jenkins.tools.test.model.PluginCompatReport] At PluginCompatReport.java:[line 87] DM_DEFAULT_ENCODING
[ERROR] Found reliance on default encoding in org.jenkins.tools.test.model.PluginRemoting.retrievePomContentFromHpi(): new String(byte[], int, int) [org.jenkins.tools.test.model.PluginRemoting] At PluginRemoting.java:[line 92] DM_DEFAULT_ENCODING
[ERROR] Found reliance on default encoding in org.jenkins.tools.test.PluginCompatTester.addSplitPluginDependencies(String, MavenRunner$Config, File, MavenPom, Map, Map, String, List): new java.io.FileReader(File) [org.jenkins.tools.test.PluginCompatTester] At PluginCompatTester.java:[line 706] DM_DEFAULT_ENCODING
[ERROR] org.jenkins.tools.test.model.comparators.MavenCoordinatesComparator implements Comparator but not Serializable [org.jenkins.tools.test.model.comparators.MavenCoordinatesComparator] At MavenCoordinatesComparator.java:[lines 37-40] SE_COMPARATOR_SHOULD_BE_SERIALIZABLE
[ERROR] org.jenkins.tools.test.model.comparators.VersionComparator implements Comparator but not Serializable [org.jenkins.tools.test.model.comparators.VersionComparator] At VersionComparator.java:[lines 35-74] SE_COMPARATOR_SHOULD_BE_SERIALIZABLE
[ERROR] Dead store to javaVersionOutput2 in org.jenkins.tools.test.model.PluginCompatTesterConfig.getTestJavaVersion() [org.jenkins.tools.test.model.PluginCompatTesterConfig] At PluginCompatTesterConfig.java:[line 346] DLS_DEAD_LOCAL_STORE
[ERROR] Useless object stored in variable pluginGroupIds of method org.jenkins.tools.test.PluginCompatTester.scanWAR(File, Map, String) [org.jenkins.tools.test.PluginCompatTester] At PluginCompatTester.java:[line 620] UC_USELESS_OBJECT
[ERROR] Possible null pointer dereference in org.jenkins.tools.test.PluginCompatTesterCli.main(String[]) due to return value of called method [org.jenkins.tools.test.PluginCompatTesterCli, org.jenkins.tools.test.PluginCompatTesterCli] Dereferenced at PluginCompatTesterCli.java:[line 127]Known null at PluginCompatTesterCli.java:[line 127] NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE
[ERROR] Possible null pointer dereference in org.jenkins.tools.test.PluginCompatTesterCli.main(String[]) due to return value of called method [org.jenkins.tools.test.PluginCompatTesterCli, org.jenkins.tools.test.PluginCompatTesterCli] Dereferenced at PluginCompatTesterCli.java:[line 135]Known null at PluginCompatTesterCli.java:[line 135] NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE
[ERROR] Possible null pointer dereference in org.jenkins.tools.test.PluginCompatTesterCli.main(String[]) due to return value of called method [org.jenkins.tools.test.PluginCompatTesterCli, org.jenkins.tools.test.PluginCompatTesterCli] Dereferenced at PluginCompatTesterCli.java:[line 141]Known null at PluginCompatTesterCli.java:[line 141] NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE
[ERROR] Exception is caught when Exception is not thrown in org.jenkins.tools.test.hook.MultiParentCompileHook.action(Map) [org.jenkins.tools.test.hook.MultiParentCompileHook] At MultiParentCompileHook.java:[line 68] REC_CATCH_EXCEPTION
basil
commented
Oct 11, 2019
| @@ -346,7 +346,7 @@ public String getTestJavaVersion() throws IOException { | |||
| final String javaVersionOutput2 = IOUtils.toString(process2.getInputStream()); | |||
| // Expected format is something like openjdk full version "1.8.0_181-8u181-b13-2~deb9u1-b13" | |||
| // We shorten it by removing the "full version" in the middle | |||
| return javaVersionOutput. | |||
| return javaVersionOutput2. | |||
There was a problem hiding this comment.
This was a simple bug; we were using the wrong variable here.
Comment on lines
-619
to
-621
| if (pluginGroupIds == null) { | ||
| pluginGroupIds = new HashMap<>(); | ||
| } |
There was a problem hiding this comment.
This was dead code. We were always passing in a non-null value for pluginGroupIds.
There was a problem hiding this comment.
I suggest adding annotation to the argument to indicate that
|
CC: @jenkinsci/hacktoberfest |
oleg-nenashev
requested changes
Dec 10, 2019
| try (Writer out = new FileWriter(tempReportPath)) { | ||
| File tempReportPath = new File(reportPath.getAbsolutePath() + ".tmp"); | ||
| try (Writer out = | ||
| Files.newBufferedWriter(tempReportPath.toPath(), Charset.defaultCharset())) { |
There was a problem hiding this comment.
There is an explicit encoding set in the line below. Should it use that one?
There was a problem hiding this comment.
Maybe, but in any case that is a pre-existing issue that wasn't introduced by my PR.
| @@ -346,7 +346,7 @@ public String getTestJavaVersion() throws IOException { | |||
| final String javaVersionOutput2 = IOUtils.toString(process2.getInputStream()); | |||
| // Expected format is something like openjdk full version "1.8.0_181-8u181-b13-2~deb9u1-b13" | |||
| // We shorten it by removing the "full version" in the middle | |||
| return javaVersionOutput. | |||
| return javaVersionOutput2. | |||
Comment on lines
-619
to
-621
| if (pluginGroupIds == null) { | ||
| pluginGroupIds = new HashMap<>(); | ||
| } |
There was a problem hiding this comment.
I suggest adding annotation to the argument to indicate that
| @@ -28,6 +29,7 @@ public MultiParentCompileHook() { | |||
|
|
|||
|
|
|||
| @Override | |||
| @SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "silly rule") | |||
There was a problem hiding this comment.
not informative. Is it for } catch (Exception e) { ?
|
Sorry, missed that |
oleg-nenashev
changed the title
oleg-nenashev
changed the title
oleg-nenashev
approved these changes
Dec 24, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes all SpotBugs issues and enables SpotBugs by default. Each commit fixes exactly one SpotBugs issue and should be self-explanatory. Let me know if I can explain any commit further or if any cleanup is undesired.