Skip to content

Commit

Permalink
Replace JSR-305 annotations with spotbugs annotations (#71)
Browse files Browse the repository at this point in the history
* update parent and minimum jenkins version

* use jenkins 2.414.3

* Replace JSR-305 annotations with spotbugs annotations

Annotations for Nonnull, CheckForNull, and several others were proposed
for Java as part of dormant Java specification request JSR-305. The
proposal never became a part of standard Java.

Jenkins plugins should switch from using JSR-305 annotations to use
Spotbugs annotations that provide the same semantics.

The [mailing list discussion](https://groups.google.com/g/jenkinsci-dev/c/uE1wwtVi1W0/m/gLxdEJmlBQAJ)
from James Nord describes the affected annotations and why they should
be replaced with annotations that are actively maintained.

The ["Improve a plugin" tutorial](https://www.jenkins.io/doc/developer/tutorial-improve/replace-jsr-305-annotations/)
provides instructions to perform this change.

An [OpenRewrite recipe](https://docs.openrewrite.org/recipes/jenkins/javaxannotationstospotbugs)
is also available and is even better than the tutorial.

Confirmed that automated tests pass on Linux with Java 21.

* Suppress spotbugs warning to not catch NPE

* Rely on spotbugs configuration from parent pom

* Use asm-api version provided by plugin BOM

* Use latest plugin bom version

* Remove uused code formatting version

* Resolve hpi plugin warning for developer URL

* Remove unused dependencies, reduce HPI file size

* Use parent pom 4.82

https://github.com/jenkinsci/plugin-pom/releases/tag/plugin-4.82

* Use plugin parent pom 4.82

* Revert incorrect change to README URL

* Remove extra blank line in test

* Use consistent source format in test

* Simplify the issue URL to modern URL and shorter string

* Remove blue ocean from CI URL

Blue Ocean enhancements have stopped.  In the future, Blue Ocean will
be removed from ci.jenkins.io.  Better to prepare for that removal now.

https://www.jenkins.io/doc/book/blueocean/ says:

Blue Ocean status

Blue Ocean will not receive further functionality updates. Blue Ocean
will continue to provide easy-to-use Pipeline visualization, but it
will not be enhanced further. It will only receive selective updates
for significant security issues or functional defects.

Alternative options for Pipeline visualization, such as the Pipeline:
Stage View and Pipeline Graph View plugins, are available and offer some
of the same functionality. While not complete replacements for Blue Ocean,
contributions are encouraged from the community for continued development
of these plugins.

The Pipeline syntax snippet generator assists users as they define
Pipeline steps with their arguments. It is the preferred tool for Jenkins
Pipeline creation, as it provides online help for the Pipeline steps
available in your Jenkins controller. It uses the plugins installed
on your Jenkins controller to generate the Pipeline syntax. Refer to
the Pipeline steps reference page for information on all available
Pipeline steps.

---------

Co-authored-by: Stefan Spieker <[email protected]>
  • Loading branch information
MarkEWaite and StefanSpieker committed Sep 5, 2024
1 parent 7b1f926 commit eae62fe
Show file tree
Hide file tree
Showing 15 changed files with 35 additions and 128 deletions.
116 changes: 8 additions & 108 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>4.32</version>
<version>4.82</version>
</parent>

<artifactId>robot</artifactId>
Expand Down Expand Up @@ -37,27 +37,21 @@
</developers>

<properties>
<jenkins.version>2.346.3</jenkins.version>
<junit.version>1166.va_436e268e972</junit.version>
<token-macro.version>2.6</token-macro.version>
<maven-git-code-format.version>1.25</maven-git-code-format.version>
<script-security.version>1229.v4880b_b_e905a_6</script-security.version>
<blueocean-rest-impl.version>1.25.0</blueocean-rest-impl.version>
<java.level>8</java.level>
<jenkins.version>2.414.3</jenkins.version>
</properties>

<scm>
<connection>scm:git:ssh://github.com/jenkinsci/robot-plugin.git</connection>
<developerConnection>scm:git:ssh://[email protected]/jenkinsci/robot-plugin.git</developerConnection>
<developerConnection>scm:git:[email protected]:jenkinsci/robot-plugin.git</developerConnection>
<url>https://www.github.com/jenkinsci/robot-plugin</url>
<tag>HEAD</tag>
</scm>
<dependencyManagement>
<dependencies>
<dependency>
<groupId>io.jenkins.tools.bom</groupId>
<artifactId>bom-2.346.x</artifactId>
<version>1742.vb_70478c1b_25f</version>
<artifactId>bom-2.414.x</artifactId>
<version>2982.vdce2153031a_0</version>
<scope>import</scope>
<type>pom</type>
</dependency>
Expand All @@ -76,7 +70,6 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>junit</artifactId>
<version>${junit.version}</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
Expand Down Expand Up @@ -119,19 +112,16 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>token-macro</artifactId>
<version>${token-macro.version}</version>
<optional>true</optional>
</dependency>
<dependency>
<groupId>io.jenkins.blueocean</groupId>
<artifactId>blueocean-rest-impl</artifactId>
<version>${blueocean-rest-impl.version}</version>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>script-security</artifactId>
<version>${script-security.version}</version>
<optional>true</optional>
</dependency>
<dependency>
Expand All @@ -145,90 +135,9 @@
<version>3.2.9</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
<version>3.12.0</version>
</dependency>

<!-- RequireUpperBounds -->
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>jackson2-api</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>display-url-api</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>scm-api</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>variant</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>credentials</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci</groupId>
<artifactId>symbol-annotation</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>cloudbees-folder</artifactId>
</dependency>
<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>snakeyaml-api</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>log4j-over-slf4j</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-jdk14</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>jcl-over-slf4j</artifactId>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm</artifactId>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm-tree</artifactId>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm-analysis</artifactId>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm-util</artifactId>
</dependency>
<dependency>
<groupId>org.kohsuke</groupId>
<artifactId>access-modifier-annotation</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci</groupId>
<artifactId>annotation-indexer</artifactId>
</dependency>
<dependency>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-annotations</artifactId>
<artifactId>commons-lang3-api</artifactId>
</dependency>

</dependencies>
Expand All @@ -253,15 +162,6 @@
</dependency>
</dependencies>
</plugin>
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<version>3.1.12</version>
<configuration>
<threshold>Low</threshold>
<effort>Max</effort>
</configuration>
</plugin>
</plugins>

<extensions>
Expand Down Expand Up @@ -292,10 +192,10 @@
</pluginRepositories>
<issueManagement>
<system>Jira</system>
<url>https://issues.jenkins-ci.org/issues/?jql=component%20%3D%20robot-plugin</url>
<url>https://issues.jenkins.io/issues/?jql=component%3Drobot-plugin</url>
</issueManagement>
<ciManagement>
<system>Jenkins</system>
<url>https://ci.jenkins.io/blue/organizations/jenkins/Plugins%2Frobot-plugin</url>
<url>https://ci.jenkins.io/job/Plugins/job/robot-plugin/</url>
</ciManagement>
</project>
2 changes: 1 addition & 1 deletion src/main/java/hudson/plugins/robot/RobotBuildAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ private XmlFile getDataFile() {

private void cacheRobotResult(RobotResult result) {
if (enableCache) {
resultReference = new WeakReference<RobotResult>(result);
resultReference = new WeakReference<>(result);
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/main/java/hudson/plugins/robot/RobotPublisher.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,16 @@
import java.util.ArrayList;
import java.util.Collection;

import javax.annotation.Nonnull;
import javax.servlet.ServletException;

import jenkins.tasks.SimpleBuildStep;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;

import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

public class RobotPublisher extends Recorder implements Serializable,
MatrixAggregatable, SimpleBuildStep {

Expand Down Expand Up @@ -250,7 +252,9 @@ protected RobotResult parse(String expandedTestResults, String expandedLogFileNa
* {@inheritDoc}
*/
@Override
public void perform(Run<?, ?> build, @Nonnull FilePath workspace, @Nonnull EnvVars buildEnv, @Nonnull Launcher launcher, @Nonnull TaskListener listener) throws InterruptedException, IOException {
@SuppressFBWarnings(value = "DCN_NULLPOINTER_EXCEPTION",
justification = "Lower risk to suppress the warning than to stop catching the null pointer exception")
public void perform(Run<?, ?> build, @NonNull FilePath workspace, @NonNull EnvVars buildEnv, @NonNull Launcher launcher, @NonNull TaskListener listener) throws InterruptedException, IOException {
if (build.getResult() != Result.ABORTED) {
PrintStream logger = listener.getLogger();
logger.println(Messages.robot_publisher_started());
Expand Down
7 changes: 3 additions & 4 deletions src/main/java/hudson/plugins/robot/RobotStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import java.util.Set;
import java.util.logging.Logger;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;

import org.apache.commons.lang.StringUtils;
import org.jenkinsci.plugins.workflow.steps.Step;
Expand All @@ -20,7 +20,6 @@
import hudson.FilePath;
import hudson.Launcher;
import hudson.Util;
import hudson.matrix.MatrixBuild;
import hudson.model.Run;
import hudson.model.TaskListener;

Expand All @@ -29,7 +28,7 @@ public class RobotStep extends Step {
private static final Logger logger = Logger.getLogger(RobotStep.class.getName());

private @CheckForNull String archiveDirName;
private final @Nonnull String outputPath;
private final @NonNull String outputPath;
private @CheckForNull String reportFileName;
private @CheckForNull String logFileName;
private @CheckForNull String outputFileName;
Expand Down
1 change: 0 additions & 1 deletion src/main/java/hudson/plugins/robot/graph/RobotGraph.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package hudson.plugins.robot.graph;

import hudson.model.AbstractBuild;
import hudson.model.Run;
import hudson.util.Graph;
import hudson.util.ShiftedCategoryAxis;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public static RobotGraph createTestResultsGraphForTestObject(RobotTestObject roo
upperbound = failed.intValue() + passed.intValue();
}


RobotBuildLabel label = new RobotBuildLabel(testObject,labelFormat);

values.add(passed);
Expand Down Expand Up @@ -119,7 +120,7 @@ public static RobotGraph createTestResultsGraphForTestObject(RobotTestObject roo
* @return Created graph
*/
public static RobotGraph createDurationGraphForTestObject(RobotTestObject rootObject, boolean hd, int maxBuildsToShow, String labelFormat, boolean preview) {
DataSetBuilder<String, RobotBuildLabel> builder = new DataSetBuilder<String, RobotBuildLabel>();
DataSetBuilder<String, RobotBuildLabel> builder = new DataSetBuilder<>();

int scale = 1;
int buildsLeftToShow = maxBuildsToShow > 0? maxBuildsToShow: -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public List<String> getTags(){

public String getCommaSeparatedTags(){
List<String> tags = getTags();
if (tags.size()==0)
if (tags.isEmpty())

Check warning on line 207 in src/main/java/hudson/plugins/robot/model/RobotCaseResult.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 207 is only partially covered, one branch is missing
return "";
else {
StringBuilder buf = new StringBuilder();
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/hudson/plugins/robot/model/RobotResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import java.io.IOException;
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -222,7 +223,7 @@ public double getSkipPercentage() {

private static double roundToDecimals(double value, int decimals){
BigDecimal bd = new BigDecimal(Double.toString(value));
bd = bd.setScale(decimals, BigDecimal.ROUND_DOWN);
bd = bd.setScale(decimals, RoundingMode.DOWN);
return bd.doubleValue();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ public void tally(RobotBuildAction parentAction) {
* @return Found Object
*/
public RobotTestObject findObjectById(String id) {
if(id.indexOf("/") >= 0){
if(id.contains("/")){
String suiteName = id.substring(0, id.indexOf("/"));
String childId = id.substring(id.indexOf("/")+1, id.length());
RobotSuiteResult suite = children.get(suiteName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.model.AbstractBuild;
import hudson.model.Hudson;
import hudson.plugins.robot.RobotBuildAction;
import jenkins.model.Jenkins;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ public void testShouldNotDisplayGraph() throws IOException {
FreeStyleBuild build2 = spy(build);
when(p.getLastBuild()).thenReturn(build2);
doReturn(null).when(build2).getPreviousBuild();
doReturn(null).when(build2).getAction(RobotBuildAction.class);
doReturn(null).when(build2).getAction(AggregatedRobotAction.class);

RobotProjectAction action = new RobotProjectAction(p);
assertFalse(action.isDisplayGraph());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
import hudson.plugins.robot.model.RobotResult;
import jenkins.model.Jenkins;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

Expand All @@ -44,9 +44,9 @@
import org.jvnet.hudson.test.JenkinsRule.WebClient;
import org.jvnet.hudson.test.recipes.LocalData;

import com.gargoylesoftware.htmlunit.WebAssert;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import com.gargoylesoftware.htmlunit.html.HtmlTable;
import org.htmlunit.WebAssert;
import org.htmlunit.html.HtmlPage;
import org.htmlunit.html.HtmlTable;

public class RobotPublisherSystemTest {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package hudson.plugins.robot;

import com.gargoylesoftware.htmlunit.IncorrectnessListener;
import org.htmlunit.IncorrectnessListener;

/*
* get rid of verbose warnings in system tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ protected void setUp() throws Exception {
c.setTimeInMillis(0L);
when(mockBuild1.getTimestamp()).thenReturn(c);
when(mockBuild2.getTimestamp()).thenReturn(c);
when(mockBuild1.getTime()).thenReturn(c.getTime());
when(mockBuild2.getTime()).thenReturn(c.getTime());
when(mockBuild1.getDisplayName()).thenReturn("1.2.3");
when(mockBuild2.getDisplayName()).thenReturn("3.2.1");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void shouldAcceptMultipleSuitesAsChildren(){
public void shouldReturnEmptyCollectionIfNoChildren(){
RobotSuiteResult result = new RobotSuiteResult();
assertNotNull("Return value was null", result.getChildSuites());
assertTrue("Collection was not empty", result.getChildSuites().size() == 0);
assertEquals("Collection was not empty", 0, result.getChildSuites().size());
}

@Test
Expand Down Expand Up @@ -94,6 +94,6 @@ public void shouldAcceptMultipleCaseResults() {
public void shouldReturnEmptyCollectionIfNoTestCases() throws DocumentException {
RobotSuiteResult result = new RobotSuiteResult();
assertNotNull("Return value was null", result.getCaseResults());
assertTrue("Collection was not empty", result.getCaseResults().size() == 0);
assertEquals("Collection was not empty", 0, result.getCaseResults().size());
}
}

0 comments on commit eae62fe

Please sign in to comment.