Skip to content

Commit

Permalink
Modernize (parent pom and Jenkins) (#87)
Browse files Browse the repository at this point in the history
Upgrading parent pom to be up-to-date.

Upgrading Jenkins as JCasC requires at least 2.60.3.

Fixing jdepend-maven-plugin version. There is no version 2.9.1. It looks
like this was a copy'n'paste error from maven-javadoc-plugin version.

Setting upper bound version for plugins via <dependencyManagement>
to be compliant with Maven enforcer rules.

Upgrading maven-plugin so that `mvn hpi:run` runs without errors.

Use SpotBugs instead of FindBugs in reporting (see change in parent pom).

Jenkins::getInstance is now annotated with @nonnull. Therefore removing
the unnecessary null checks that have been detected by SpotBugs.

HtmlUnit removed/changed some methods, adapting tests to compile again.

Plugin "maven-plugin" is not required to be loaded explicitly in a test.
It is loaded automatically because it is defined as dependency in pom.xml.
Therefore just verifying that it is loaded is enough.
  • Loading branch information
darxriggs authored and Jochen-A-Fuerbacher committed May 2, 2019
1 parent 2ccc458 commit cb04858
Show file tree
Hide file tree
Showing 13 changed files with 44 additions and 58 deletions.
21 changes: 16 additions & 5 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>2.30</version>
<version>3.42</version>
<relativePath />
</parent>

Expand Down Expand Up @@ -114,13 +114,24 @@
</developers>

<properties>
<jenkins.version>2.60.3</jenkins.version>
<maven-surefire-report-plugin.version>2.20</maven-surefire-report-plugin.version>
<maven-hpi-plugin.injectedTestName>InjectedIT</maven-hpi-plugin.injectedTestName>
<maven-checkstyle-plugin.version>2.17</maven-checkstyle-plugin.version>
<jdepend-maven-plugin.version>2.9.1</jdepend-maven-plugin.version>
<jdepend-maven-plugin.version>2.0</jdepend-maven-plugin.version>
<java.level>8</java.level>
</properties>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>commons-net</groupId>
<artifactId>commons-net</artifactId>
<version>3.6</version>
</dependency>
</dependencies>
</dependencyManagement>

<dependencies>
<dependency>
<groupId>org.xmlunit</groupId>
Expand All @@ -135,7 +146,7 @@
<dependency>
<groupId>org.jenkins-ci.main</groupId>
<artifactId>maven-plugin</artifactId>
<version>2.0</version>
<version>3.0</version>
<optional>true</optional>
</dependency>
<dependency>
Expand Down Expand Up @@ -331,8 +342,8 @@
</configuration>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>findbugs-maven-plugin</artifactId>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<configuration>
<xmlOutput>true</xmlOutput>
<threshold>Low</threshold>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class ComputerHistoryListener extends ComputerListener {

public ComputerHistoryListener() {
Jenkins jenkins = Jenkins.getInstance();
nodes = jenkins != null ? jenkins.getNodes() : null;
nodes = jenkins.getNodes();
}

@Override
Expand All @@ -58,8 +58,6 @@ public void onConfigurationChange() {
// during class initialization. NodeList will surely be defined
// on the first run of this method.
Jenkins jenkins = Jenkins.getInstance();
if(jenkins == null)
return;
if (nodes == null) {
nodes = jenkins.getNodes();
}
Expand All @@ -86,8 +84,6 @@ public void onConfigurationChange() {
*/
private void onAdd() {
Jenkins jenkins = Jenkins.getInstance();
if(jenkins == null)
return;
for (Node node : jenkins.getNodes()) {
if (!nodes.contains(node) && isTracked(node)) {
switchHistoryDao(node).createNewNode(node);
Expand All @@ -111,8 +107,6 @@ private boolean isTracked(Node node) {
*/
private void onRemove() {
Jenkins jenkins = Jenkins.getInstance();
if(jenkins == null)
return;
for (Node node : nodes) {
if (!jenkins.getNodes().contains(node)
&& isTracked(node)) {
Expand All @@ -127,8 +121,6 @@ && isTracked(node)) {
*/
private void onChange() {
Jenkins jenkins = Jenkins.getInstance();
if(jenkins == null)
return;
final JobConfigHistoryStrategy hdao = PluginUtils.getHistoryDao();
for (Node node : jenkins.getNodes()) {
if (!PluginUtils.isUserExcluded(PluginUtils.getPlugin())
Expand All @@ -145,8 +137,6 @@ && isTracked(node) && !hdao.hasDuplicateHistory(node)) {
private void onRename() {
Node originalNode = null;
Jenkins jenkins = Jenkins.getInstance();
if(jenkins == null)
return;
for (Node node : nodes) {
if (!jenkins.getNodes().contains(node)
&& isTracked(node)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public String createLink() {
*/
String getRootUrl() {
Jenkins jenkins = Jenkins.getInstance();
return jenkins != null ? jenkins.getRootUrl() : null;
return jenkins.getRootUrl();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ protected HistoryDao getHistoryDao() {
*/
protected File getJenkinsHome() {
Jenkins jenkins = Jenkins.getInstance();
return jenkins != null ? jenkins.root : null;
return jenkins.getRootDir();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,11 @@ public class JobConfigHistoryPurger extends PeriodicWork {
* Standard constructor using instance.
*/
public JobConfigHistoryPurger() {
Jenkins jenkins = Jenkins.getInstance();
if(jenkins == null)
return;
JobConfigHistory plugin = jenkins.getPlugin(JobConfigHistory.class);
assignValue(plugin,
(PluginUtils.getHistoryDao(plugin, null) instanceof Purgeable
? (Purgeable) PluginUtils.getHistoryDao(plugin, null)
: null),
PluginUtils.getHistoryDao(plugin, null));
JobConfigHistory plugin = PluginUtils.getPlugin();
JobConfigHistoryStrategy historyDao = PluginUtils.getAnonymousHistoryDao(plugin);
assignValue(plugin,
historyDao instanceof Purgeable ? (Purgeable) historyDao : null,
historyDao);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ HistoryDao getHistoryDao() {
*/
HistoryDao getHistoryDao(JobConfigHistory plugin) {
Jenkins jenkins = Jenkins.getInstance();
if(jenkins == null)
return null;
return (COMPLETED == jenkins.getInitLevel())
? PluginUtils.getHistoryDao(plugin)
: PluginUtils.getAnonymousHistoryDao(plugin);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public abstract class JobConfigHistoryStrategy
@SuppressWarnings("unchecked")
public final Descriptor<JobConfigHistoryStrategy> getDescriptor() {
Jenkins jenkins = Jenkins.getInstance();
return jenkins != null ? jenkins.getDescriptorOrDie(getClass()) : null;
return jenkins.getDescriptorOrDie(getClass());
}

/**
Expand All @@ -60,8 +60,6 @@ public final Descriptor<JobConfigHistoryStrategy> getDescriptor() {
*/
public static DescriptorExtensionList<JobConfigHistoryStrategy, JobConfigHistoryDescriptor<JobConfigHistoryStrategy>> all() {
Jenkins jenkins = Jenkins.getInstance();
if(jenkins == null)
return null;
return jenkins
.<JobConfigHistoryStrategy, JobConfigHistoryDescriptor<JobConfigHistoryStrategy>>getDescriptorList(
JobConfigHistoryStrategy.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ private PluginUtils() {
*/
public static JobConfigHistory getPlugin() {
Jenkins jenkins = Jenkins.getInstance();
return jenkins != null ? jenkins.getPlugin(JobConfigHistory.class) : null;
return jenkins.getPlugin(JobConfigHistory.class);
}

/**
Expand Down Expand Up @@ -113,8 +113,6 @@ public static JobConfigHistoryStrategy getHistoryDao(
maxHistoryEntries = 0;
}
Jenkins jenkins = Jenkins.getInstance();
if(jenkins == null)
return null;
return new FileHistoryDao(plugin.getConfiguredHistoryRootDir(),
new File(jenkins.root.getPath()), user,
maxHistoryEntries, !plugin.getSkipDuplicateHistory());
Expand Down Expand Up @@ -158,11 +156,9 @@ public static Date parsedDate(final String timeStamp) {
*/
public static boolean isMavenPluginAvailable() {
Jenkins jenkins = Jenkins.getInstance();
if(jenkins == null)
return false;
try {
Plugin plugin = jenkins.getPlugin("maven-plugin");
return plugin.getWrapper().isActive();
return plugin != null && plugin.getWrapper().isActive();
} catch (Exception e) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,10 @@ public void testClearDuplicateLines() throws Exception {
final HtmlPage historyPage = webClient
.goTo("job/" + jobName + "/" + JobConfigHistoryConsts.URLNAME);
final HtmlForm diffFilesForm = historyPage.getFormByName("diffFiles");
final HtmlPage diffPage = (HtmlPage) last(diffFilesForm.getHtmlElementsByTagName("button")).click();
final HtmlPage diffPage = last(diffFilesForm.getElementsByTagName("button")).click();
assertStringContains(diffPage.asText(), "<daysToKeep>42</daysToKeep>");
assertStringContains(diffPage.asText(), "<numToKeep>42</numToKeep>");
assertStringContains(diffPage.asText(), "<daysToKeep>47</daysToKeep>");
assertStringContains(diffPage.asText(), "<numToKeep>47</numToKeep>");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
Expand All @@ -21,7 +22,6 @@
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.recipes.WithPlugin;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;

Expand Down Expand Up @@ -95,9 +95,9 @@ public void testGetIconFileNameSaveProjectNonMavenModules() {
/**
* Test of getIconFileName method, of class JobConfigHistoryProjectAction.
*/
@WithPlugin("maven-plugin.hpi")
@Test
public void testGetIconFileNameSaveMavenModules() {
assertNotNull(j.jenkins.getPlugin("maven-plugin"));
when(mockedMavenModule.hasPermission(AbstractProject.CONFIGURE))
.thenReturn(true);
when(mockedPlugin.getSaveModuleConfiguration()).thenReturn(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public void testSingleSystemHistoryPage() {
page.split("Changed").length > 2);

final HtmlForm diffFilesForm = htmlPage.getFormByName("diffFiles");
final HtmlPage diffPage = (HtmlPage) last(diffFilesForm.getHtmlElementsByTagName("button")).click();
final HtmlPage diffPage = last(diffFilesForm.getElementsByTagName("button")).click();
assertStringContains(diffPage.asText(), firstMessage);
assertStringContains(diffPage.asText(), secondMessage);
} catch (Exception ex) {
Expand Down Expand Up @@ -249,8 +249,7 @@ public void testDeletedAfterDisabled() throws Exception {

final HtmlPage htmlPage = webClient
.goTo(JobConfigHistoryConsts.URLNAME + "/?filter=deleted");
final HtmlAnchor rawLink = (HtmlAnchor) htmlPage
.getAnchorByText("(RAW)");
final HtmlAnchor rawLink = htmlPage.getAnchorByText("(RAW)");
final String rawPage = ((TextPage) rawLink.click()).getContent();
Assert.assertTrue("Verify config file is shown",
rawPage.contains(description));
Expand Down Expand Up @@ -280,7 +279,7 @@ public void testRestoreAfterDisabled() throws Exception {
System.out.println(historyAsXml);
Assert.assertTrue("History page should contain 'Deleted' entry",
historyAsXml.contains("Deleted"));
final List<HtmlAnchor> hrefs = (List<HtmlAnchor>) historyPage
final List<HtmlAnchor> hrefs = historyPage
.getByXPath("//a[contains(@href, \"configOutput?type=xml\")]");
Assert.assertTrue(hrefs.size() > 2);
}
Expand Down Expand Up @@ -351,7 +350,7 @@ private HtmlPage restoreProject() throws Exception {
.goTo(JobConfigHistoryConsts.URLNAME + "/?filter=deleted");
final HtmlAnchor restoreLink = (HtmlAnchor) htmlPage
.getElementById("restore");
final HtmlPage reallyRestorePage = (HtmlPage) restoreLink.click();
final HtmlPage reallyRestorePage = restoreLink.click();
final HtmlForm restoreForm = reallyRestorePage.getFormByName("restore");
final HtmlPage jobPage = submit(restoreForm, "Submit");
return jobPage;
Expand All @@ -373,9 +372,9 @@ public void testRestoreFromHistoryPage() throws Exception {

final HtmlPage htmlPage = webClient
.goTo(JobConfigHistoryConsts.URLNAME + "/?filter=deleted");
final List<HtmlAnchor> hrefs = (List<HtmlAnchor>) htmlPage
final List<HtmlAnchor> hrefs = htmlPage
.getByXPath("//a[contains(@href, \"TestProject_deleted_\")]");
final HtmlPage historyPage = (HtmlPage) hrefs.get(0).click();
final HtmlPage historyPage = hrefs.get(0).click();
final HtmlPage reallyRestorePage = submit(
historyPage.getFormByName("forward"), "Submit");
final HtmlPage jobPage = submit(
Expand Down
16 changes: 7 additions & 9 deletions src/test/java/hudson/plugins/jobConfigHistory/PluginIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ private class ConfigPage {
private final HtmlPage configPage;

/**
* @param configPage
* @throws SAXException
* @throws IOException
*/
Expand Down Expand Up @@ -81,7 +80,7 @@ public BasicHistoryPage(HtmlPage historyPage)
}

public List<HtmlAnchor> getConfigOutputLinks(final String type) {
final List<HtmlAnchor> hrefs = (List<HtmlAnchor>) historyPage.getByXPath(
final List<HtmlAnchor> hrefs = historyPage.getByXPath(
"//a[contains(@href, \"configOutput?type=" + type + "\")]");
return hrefs;
}
Expand Down Expand Up @@ -110,7 +109,7 @@ public HistoryPage() throws IOException, SAXException {
public HtmlPage getDiffPage() throws IOException {
final HtmlForm diffFilesForm = historyPage
.getFormByName("diffFiles");
return (HtmlPage) last(diffFilesForm.getHtmlElementsByTagName("button")).click();
return last(diffFilesForm.getElementsByTagName("button")).click();
}

public void setCheckedTimestamp1RadioButton(int index,
Expand Down Expand Up @@ -174,7 +173,7 @@ public void testSaveConfiguration() throws Exception {
.getConfigOutputLinks("xml");
Assert.assertTrue(hrefs.size() >= 1);
final HtmlAnchor xmlAnchor = hrefs.get(0);
final XmlPage xmlPage = (XmlPage) xmlAnchor.click();
final XmlPage xmlPage = xmlAnchor.click();
assertThat(xmlPage.asXml(), containsString(firstDescription));
}
{
Expand All @@ -183,7 +182,7 @@ public void testSaveConfiguration() throws Exception {
.getConfigOutputLinks("raw");
Assert.assertTrue(hrefs.size() >= 2);
final HtmlAnchor rawAnchor = hrefs.get(0);
final TextPage firstRaw = (TextPage) rawAnchor.click();
final TextPage firstRaw = rawAnchor.click();
assertThat(firstRaw.getContent(),
containsString(secondDescription));
}
Expand All @@ -195,7 +194,7 @@ public void testSaveConfiguration() throws Exception {
final List<? extends HtmlAnchor> allXmlHRefs = allJobConfigHistoryPage
.getConfigOutputLinks("xml");
Assert.assertTrue(allXmlHRefs.size() >= 2);
final TextPage firstRawOfAll = (TextPage) allRawHRefs.get(0).click();
final TextPage firstRawOfAll = allRawHRefs.get(0).click();
assertThat(firstRawOfAll.getContent(),
containsString(secondDescription));
final HistoryPage historyPage = new HistoryPage();
Expand Down Expand Up @@ -234,13 +233,12 @@ public void testHistoryPageOfSingleSystemConfig() {
try {
final HtmlPage historyPage = webClient.goTo(
JobConfigHistoryConsts.URLNAME + "/history?name=config");
final List<HtmlAnchor> allRawHRefs = (List<HtmlAnchor>) historyPage.getByXPath(
final List<HtmlAnchor> allRawHRefs = historyPage.getByXPath(
"//a[contains(@href, \"configOutput?type=raw\")]");
Assert.assertTrue(
"Check that there are at least 2 links for raw output.",
allRawHRefs.size() >= 2);
final TextPage firstRawOfAll = (TextPage) allRawHRefs.get(0)
.click();
final TextPage firstRawOfAll = allRawHRefs.get(0).click();
assertThat(
"Check that the first raw output link leads to the right target.",
firstRawOfAll.getContent(),
Expand Down
Binary file removed src/test/resources/plugins/maven-plugin.hpi
Binary file not shown.

0 comments on commit cb04858

Please sign in to comment.