From 2c99a5c3de7fe09c10bb72fabab63b16f02950b0 Mon Sep 17 00:00:00 2001 From: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com> Date: Wed, 26 Feb 2025 11:25:03 +0100 Subject: [PATCH 1/2] Migrate tests to JUnit5 * Migrate annotations and imports * Migrate assertions * Remove public visibility for test classes and methods * Minor code cleanup --- pom.xml | 2 +- .../metrics/RoundTripJCascMetricsTest.java | 18 ++-- .../api/HealthCheckProviderForTesting.java | 9 +- .../metrics/api/HealthCheckerTest.java | 19 ++-- .../metrics/api/MetricsAccessKeyTest.java | 25 +++--- .../java/jenkins/metrics/api/MetricsTest.java | 87 ++++++++++--------- .../impl/JenkinsMetricProviderImplTest.java | 42 ++++----- .../impl/ObjectNameFactoryImplTest.java | 33 ++++--- 8 files changed, 112 insertions(+), 123 deletions(-) diff --git a/pom.xml b/pom.xml index 6455659..23f7e69 100644 --- a/pom.xml +++ b/pom.xml @@ -89,7 +89,7 @@ io.jenkins.tools.bom bom-${jenkins.baseline}.x - 3893.v213a_42768d35 + 4136.vca_c3202a_7fd1 import pom diff --git a/src/test/java/jenkins/metrics/RoundTripJCascMetricsTest.java b/src/test/java/jenkins/metrics/RoundTripJCascMetricsTest.java index 7e6c647..dc6804a 100644 --- a/src/test/java/jenkins/metrics/RoundTripJCascMetricsTest.java +++ b/src/test/java/jenkins/metrics/RoundTripJCascMetricsTest.java @@ -2,24 +2,26 @@ import hudson.ExtensionList; import hudson.util.Secret; -import io.jenkins.plugins.casc.misc.RoundTripAbstractTest; +import io.jenkins.plugins.casc.misc.junit.jupiter.AbstractRoundTripTest; import jenkins.metrics.api.MetricsAccessKey; -import org.jvnet.hudson.test.RestartableJenkinsRule; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import java.util.List; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Configuration, validation, export and configuration during restart of the configuration of the metrics plugin. */ -public class RoundTripJCascMetricsTest extends RoundTripAbstractTest { +@WithJenkins +class RoundTripJCascMetricsTest extends AbstractRoundTripTest { @Override - public void assertConfiguredAsExpected(RestartableJenkinsRule j, String configContent) { + protected void assertConfiguredAsExpected(JenkinsRule j, String configContent) { List accessKeys = ExtensionList.lookup(MetricsAccessKey.DescriptorImpl.class).get(0).getAccessKeys(); assertThat("We have an access key of metrics configured in Configure System", accessKeys.size() == 4); @@ -35,7 +37,7 @@ public void assertConfiguredAsExpected(RestartableJenkinsRule j, String configCo } @Override - public String stringInLogExpected() { + protected String stringInLogExpected() { return "MetricsAccessKey.key = ****"; } } diff --git a/src/test/java/jenkins/metrics/api/HealthCheckProviderForTesting.java b/src/test/java/jenkins/metrics/api/HealthCheckProviderForTesting.java index ba253d4..b4f6212 100644 --- a/src/test/java/jenkins/metrics/api/HealthCheckProviderForTesting.java +++ b/src/test/java/jenkins/metrics/api/HealthCheckProviderForTesting.java @@ -23,14 +23,13 @@ */ package jenkins.metrics.api; -import java.util.Map; -import java.util.Map.Entry; - import com.codahale.metrics.health.HealthCheck; - import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; +import java.util.Map; +import java.util.Map.Entry; + @Extension public class HealthCheckProviderForTesting extends HealthCheckProvider { @@ -47,7 +46,7 @@ private Entry check(int i) { @Override protected Result check() throws Exception { runs++; - Thread.sleep(1 * 1000); + Thread.sleep(1000); return Result.unhealthy("some error message"); } }); diff --git a/src/test/java/jenkins/metrics/api/HealthCheckerTest.java b/src/test/java/jenkins/metrics/api/HealthCheckerTest.java index f1a380b..382f71d 100644 --- a/src/test/java/jenkins/metrics/api/HealthCheckerTest.java +++ b/src/test/java/jenkins/metrics/api/HealthCheckerTest.java @@ -23,25 +23,22 @@ */ package jenkins.metrics.api; -import static org.junit.Assert.*; - -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.JenkinsRule; - import jenkins.metrics.api.Metrics.HealthChecker; import jenkins.metrics.util.HealthChecksThreadPool; +import org.junit.jupiter.api.Test; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; + +import static org.junit.jupiter.api.Assertions.assertEquals; /** * Test the {@link HealthChecker} execution of health checks */ -public class HealthCheckerTest { - - @Rule - public JenkinsRule j = new JenkinsRule(); +@WithJenkins +class HealthCheckerTest { @Test - public void testHealthChecksAreNotRejected() throws Exception { + void testHealthChecksAreNotRejected(JenkinsRule j) throws Exception { while (HealthCheckProviderForTesting.runs < 6 && HealthChecksThreadPool.getRejectedExecutions() == 0) { Thread.sleep(1000); } diff --git a/src/test/java/jenkins/metrics/api/MetricsAccessKeyTest.java b/src/test/java/jenkins/metrics/api/MetricsAccessKeyTest.java index ce49e27..f8f1ef2 100644 --- a/src/test/java/jenkins/metrics/api/MetricsAccessKeyTest.java +++ b/src/test/java/jenkins/metrics/api/MetricsAccessKeyTest.java @@ -1,12 +1,10 @@ package jenkins.metrics.api; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.RuleChain; +import org.junit.jupiter.api.Test; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; -import org.jvnet.hudson.test.LoggerRule; -import org.jvnet.hudson.test.RealJenkinsRule; +import org.jvnet.hudson.test.LogRecorder; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import org.jvnet.hudson.test.recipes.LocalData; import java.util.logging.Level; @@ -18,19 +16,18 @@ import static org.hamcrest.Matchers.hasProperty; import static org.hamcrest.Matchers.startsWith; import static org.hamcrest.core.IsNot.not; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; -public class MetricsAccessKeyTest { - public JenkinsRule j = new JenkinsRule(); - public LoggerRule l = new LoggerRule().record("jenkins.metrics.api.MetricsAccessKey", Level.ALL).capture(1000); - @Rule - public RuleChain chain = RuleChain.outerRule(j).around(l); +@WithJenkins +class MetricsAccessKeyTest { + + private final LogRecorder l = new LogRecorder().record("jenkins.metrics.api.MetricsAccessKey", Level.ALL).capture(1000); @Test @Issue("SECURITY-1624") @LocalData - public void oldKeysAreConvertedAfterStartup() throws Exception { + void oldKeysAreConvertedAfterStartup(JenkinsRule j) throws Exception { j.waitUntilNoActivityUpTo(5000); assertThat(l.getRecords(), hasItem( allOf( @@ -44,7 +41,7 @@ public void oldKeysAreConvertedAfterStartup() throws Exception { @Test @Issue("SECURITY-1624") @LocalData - public void newKeysStayTheSame() throws Exception { + void newKeysStayTheSame(JenkinsRule j) throws Exception { j.waitUntilNoActivityUpTo(5000); assertThat(l.getRecords(), not(hasItem( allOf( diff --git a/src/test/java/jenkins/metrics/api/MetricsTest.java b/src/test/java/jenkins/metrics/api/MetricsTest.java index 4f405b6..24f30ae 100644 --- a/src/test/java/jenkins/metrics/api/MetricsTest.java +++ b/src/test/java/jenkins/metrics/api/MetricsTest.java @@ -3,17 +3,17 @@ import hudson.util.VersionNumber; import jenkins.model.Jenkins; import org.apache.commons.lang.ArrayUtils; -import org.junit.AfterClass; -import org.junit.Assume; -import org.junit.BeforeClass; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.RestartableJenkinsRule; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import javax.management.MBeanServer; import javax.management.ObjectName; +import java.io.File; import java.io.IOException; import java.lang.management.ManagementFactory; @@ -21,56 +21,59 @@ import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; import static org.hamcrest.core.IsNot.not; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; -public class MetricsTest { +@WithJenkins +class MetricsTest { - private static final String SYSPROP_FOR_LOGS_PATH = "hudson.triggers.SafeTimerTask.logsTargetDir"; - @Rule - public final TemporaryFolder folder = new TemporaryFolder(); - @Rule - public RestartableJenkinsRule story = new RestartableJenkinsRule(); + private static final String SYS_PROP_FOR_LOGS_PATH = "hudson.triggers.SafeTimerTask.logsTargetDir"; + @TempDir + private File folder; - @BeforeClass - @AfterClass - public static void cleanProperty() throws IOException { - System.clearProperty(SYSPROP_FOR_LOGS_PATH); + @BeforeAll + @AfterAll + static void cleanProperty() { + System.clearProperty(SYS_PROP_FOR_LOGS_PATH); } - @Issue("JENKINS-50291") @Test - public void logsDirectory() { + void logsDirectory(JenkinsRule j) throws Throwable { + assumeTrue(Jenkins.getVersion().isNewerThan(new VersionNumber("2.113"))); // JENKINS-50291 introduced in 2.114 - story.then(j -> { - Assume.assumeTrue(Jenkins.getVersion().isNewerThan(new VersionNumber("2.113"))); // JENKINS-50291 introduced in 2.114 + assertTrue(Metrics.HealthChecker.getLogFile(j.jenkins).getAbsolutePath().startsWith(j.getInstance().getRootDir().getAbsolutePath())); + System.setProperty("hudson.triggers.SafeTimerTask.logsTargetDir", newFolder(folder, "junit").getAbsolutePath()); - assertTrue(Metrics.HealthChecker.getLogFile(j.jenkins).getAbsolutePath().startsWith(j.getInstance().getRootDir().getAbsolutePath())); - System.setProperty("hudson.triggers.SafeTimerTask.logsTargetDir", folder.newFolder().getAbsolutePath()); - }); + j.restart(); - story.then(j -> assertFalse(Metrics.HealthChecker.getLogFile(j.jenkins).getAbsolutePath().startsWith(j.getInstance().getRootDir().getAbsolutePath()))); + assertFalse(Metrics.HealthChecker.getLogFile(j.jenkins).getAbsolutePath().startsWith(j.getInstance().getRootDir().getAbsolutePath())); } @Test - public void jmxDomain() { - story.then(r -> { - assertTrue(ArrayUtils.contains(ManagementFactory.getPlatformMBeanServer().getDomains(), "io.jenkins")); - }); + void jmxDomain(JenkinsRule j) { + assertTrue(ArrayUtils.contains(ManagementFactory.getPlatformMBeanServer().getDomains(), "io.jenkins")); } @Test - public void jmxMetricsExcluded() { - story.then(r -> { - MBeanServer mBeanServer = ManagementFactory.getPlatformMBeanServer(); - assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:type=Gc,*"), null), is(empty())); - assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:type=Vm,*"), null), is(empty())); - assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:*,name=History"), null), is(empty())); - assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:*,name=5m"), null), is(empty())); - assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:*,name=1h"), null), is(empty())); - assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:type=Project,*"), null), is(not(empty()))); - assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:type=Executor,*"), null), is(not(empty()))); - }); + void jmxMetricsExcluded(JenkinsRule j) throws Exception { + MBeanServer mBeanServer = ManagementFactory.getPlatformMBeanServer(); + assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:type=Gc,*"), null), is(empty())); + assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:type=Vm,*"), null), is(empty())); + assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:*,name=History"), null), is(empty())); + assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:*,name=5m"), null), is(empty())); + assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:*,name=1h"), null), is(empty())); + assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:type=Project,*"), null), is(not(empty()))); + assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:type=Executor,*"), null), is(not(empty()))); + } + + private static File newFolder(File root, String... subDirs) throws IOException { + String subFolder = String.join("/", subDirs); + File result = new File(root, subFolder); + if (!result.mkdirs()) { + throw new IOException("Couldn't create folders " + root); + } + return result; } } diff --git a/src/test/java/jenkins/metrics/impl/JenkinsMetricProviderImplTest.java b/src/test/java/jenkins/metrics/impl/JenkinsMetricProviderImplTest.java index f3323f0..e2ae442 100644 --- a/src/test/java/jenkins/metrics/impl/JenkinsMetricProviderImplTest.java +++ b/src/test/java/jenkins/metrics/impl/JenkinsMetricProviderImplTest.java @@ -34,18 +34,15 @@ import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; -import org.junit.ClassRule; -import org.junit.Ignore; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.BuildWatcher; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.SleepBuilder; import org.jvnet.hudson.test.TestExtension; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -59,15 +56,11 @@ import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.notNullValue; -public class JenkinsMetricProviderImplTest { - @Rule - public JenkinsRule j = new JenkinsRule(); - - @ClassRule - public static BuildWatcher w = new BuildWatcher(); +@WithJenkins +class JenkinsMetricProviderImplTest { @Test - public void given__a_job_which_is_cancelled() throws Exception { + void given__a_job_which_is_cancelled(JenkinsRule j) throws Exception { MyListener listener = ExtensionList.lookup(QueueItemMetricsListener.class).get(MyListener.class); assertThat(listener, notNullValue()); @@ -76,24 +69,23 @@ public void given__a_job_which_is_cancelled() throws Exception { QueueTaskFuture taskFuture = job.scheduleBuild2(10); taskFuture.cancel(true); - try{ + try { j.waitForCompletion(taskFuture.waitForStart()); - }catch (Throwable t){ + } catch (Throwable t) { //CancellationException expected } //wait for completion - while (!listener.state.toString().contains("C")){ + while (!listener.state.toString().contains("C")) { Thread.sleep(10); } assertThat(listener.state.toString(), is("QC")); - } - @Ignore("TODO allOf(greaterThan(2500L), lessThan(3800L)) flaky: 3999 observed") + @Disabled("TODO allOf(greaterThan(2500L), lessThan(3800L)) flaky: 3999 observed") @Test - public void given__a_job__when__built__then__events_and_metrics_include_build() throws Exception { + void given__a_job__when__built__then__events_and_metrics_include_build(JenkinsRule j) throws Exception { j.jenkins.setQuietPeriod(0); MyListener listener = ExtensionList.lookup(QueueItemMetricsListener.class).get(MyListener.class); assertThat(listener, notNullValue()); @@ -106,18 +98,18 @@ public void given__a_job__when__built__then__events_and_metrics_include_build() j.assertBuildStatusSuccess(p.scheduleBuild2(2)); //wait for completion - while (!listener.state.toString().contains("F")){ + while (!listener.state.toString().contains("F")) { Thread.sleep(10); } assertThat(listener.state.toString(), is("QSF")); assertThat(Metrics.metricRegistry().getTimers().get("jenkins.job.building.duration").getCount(), is(1L)); assertThat(Metrics.metricRegistry().getTimers().get("jenkins.job.building.duration").getSnapshot().getMean(), - allOf(greaterThan(TimeUnit.MILLISECONDS.toNanos(2500)*1.0), + allOf(greaterThan(TimeUnit.MILLISECONDS.toNanos(2500) * 1.0), lessThan(TimeUnit.MILLISECONDS.toNanos(3800) * 1.0))); List events = listener.getEvents(); assertThat(events.size(), is(3)); - Collections.sort(events, QueueItemMetricsEvent::compareEventSequence); + events.sort(QueueItemMetricsEvent::compareEventSequence); events.forEach(System.err::println); assertThat(events.get(0).getState(), is(QueueItemMetricsEvent.State.QUEUED)); @@ -145,10 +137,10 @@ public void given__a_job__when__built__then__events_and_metrics_include_build() @Test @Issue("JENKINS-69817") - public void given__a_job_with_subtasks__when__built__then__build_include_subtasks_metrics() throws Exception { + void given__a_job_with_subtasks__when__built__then__build_include_subtasks_metrics(JenkinsRule j) throws Exception { WorkflowJob p = j.createProject(WorkflowJob.class); p.setDefinition(new CpsFlowDefinition("node { echo 'subtask1' }" + - "\nnode { echo 'subtask2' }", true)); + "\nnode { echo 'subtask2' }", true)); WorkflowRun workflowRun = j.buildAndAssertSuccess(p); List timeInQueueActions = workflowRun.getActions(TimeInQueueAction.class); @@ -163,7 +155,7 @@ public void given__a_job_with_subtasks__when__built__then__build_include_subtask @TestExtension public static class MyListener extends QueueItemMetricsListener { private final List events = new ArrayList<>(); - public StringBuilder state = new StringBuilder(); + public final StringBuilder state = new StringBuilder(); @Override public void onQueued(QueueItemMetricsEvent event) { diff --git a/src/test/java/jenkins/metrics/impl/ObjectNameFactoryImplTest.java b/src/test/java/jenkins/metrics/impl/ObjectNameFactoryImplTest.java index 366c19a..7fcc4c2 100644 --- a/src/test/java/jenkins/metrics/impl/ObjectNameFactoryImplTest.java +++ b/src/test/java/jenkins/metrics/impl/ObjectNameFactoryImplTest.java @@ -1,36 +1,35 @@ package jenkins.metrics.impl; -import static org.junit.Assert.assertEquals; - -import jenkins.metrics.api.Metrics; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import javax.management.MalformedObjectNameException; import javax.management.ObjectName; -public class ObjectNameFactoryImplTest { +import static org.junit.jupiter.api.Assertions.assertEquals; + +class ObjectNameFactoryImplTest { private static final String JMX_DOMAIN = "io.jenkins"; private ObjectNameFactoryImpl instance; - @Before - public void setUp(){ + @BeforeEach + void setUp() { instance = new ObjectNameFactoryImpl(); } @Test - public void createName() throws MalformedObjectNameException { - assertObjectName( "jenkins.myname", "type=Myname"); - assertObjectName( "jenkins.plugins.enabled", "type=Plugins,name=Enabled"); - assertObjectName( "jenkins.plugins.disabled.value", "type=Plugins,name=Disabled"); - assertObjectName( "jenkins.project.disabled.count.value", "type=Project,name=Disabled"); - assertObjectName( "jenkins.job.blocked.duration", "type=Job,name=Blocked"); - assertObjectName( "http.responseCodes.ok", "type=Http,class=ResponseCodes,name=Ok"); - assertObjectName( "a.b.c.d.e.f", "type=A,class=B,name=C.d.e.f"); + void createName() throws MalformedObjectNameException { + assertObjectName("jenkins.myname", "type=Myname"); + assertObjectName("jenkins.plugins.enabled", "type=Plugins,name=Enabled"); + assertObjectName("jenkins.plugins.disabled.value", "type=Plugins,name=Disabled"); + assertObjectName("jenkins.project.disabled.count.value", "type=Project,name=Disabled"); + assertObjectName("jenkins.job.blocked.duration", "type=Job,name=Blocked"); + assertObjectName("http.responseCodes.ok", "type=Http,class=ResponseCodes,name=Ok"); + assertObjectName("a.b.c.d.e.f", "type=A,class=B,name=C.d.e.f"); } private void assertObjectName(String name, String objectName) throws MalformedObjectNameException { - assertEquals(new ObjectName(JMX_DOMAIN+":"+objectName), instance.createName("meters", JMX_DOMAIN, name)); + assertEquals(new ObjectName(JMX_DOMAIN + ":" + objectName), instance.createName("meters", JMX_DOMAIN, name)); } } From ffceeaaf94dd375f4450a796fb7b5448f470f146 Mon Sep 17 00:00:00 2001 From: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com> Date: Mon, 3 Mar 2025 15:25:58 +0100 Subject: [PATCH 2/2] Revert multiplication --- .../java/jenkins/metrics/api/HealthCheckProviderForTesting.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/jenkins/metrics/api/HealthCheckProviderForTesting.java b/src/test/java/jenkins/metrics/api/HealthCheckProviderForTesting.java index b4f6212..338598f 100644 --- a/src/test/java/jenkins/metrics/api/HealthCheckProviderForTesting.java +++ b/src/test/java/jenkins/metrics/api/HealthCheckProviderForTesting.java @@ -46,7 +46,7 @@ private Entry check(int i) { @Override protected Result check() throws Exception { runs++; - Thread.sleep(1000); + Thread.sleep(1 * 1000); return Result.unhealthy("some error message"); } });