Use same default seed for method and class ordering (#3821)

Prior to this commit the default seeds were generated separately but the configuration parameter that allows using a fixed seed only allowed to set both to the same value making it impossible to reproduce a failure for different default seeds. Since the default seed is now identical, this scenario is avoided. Fixes #3817.
junit-team · May 17, 2024 · a422c5a · a422c5a
1 parent 2a777a8
commit a422c5a
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 76 deletions.
diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0-M2.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0-M2.adoc
@@ -62,7 +62,8 @@ repository on GitHub.
 [[release-notes-5.11.0-M2-junit-jupiter-bug-fixes]]
 ==== Bug Fixes
 
-* ❓
+* `MethodOrderer.Random` and `ClassOrderer.Random` now use the same default seed that is
+  generated during class initialization.
 
 [[release-notes-5.11.0-M2-junit-jupiter-deprecations-and-breaking-changes]]
 ==== Deprecations and Breaking Changes

diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/ClassOrderer.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/ClassOrderer.java
@@ -15,7 +15,6 @@
 
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.Optional;
 
 import org.apiguardian.api.API;
 import org.junit.platform.commons.logging.Logger;
@@ -185,8 +184,8 @@ private static int getOrder(ClassDescriptor descriptor) {
 	 * <h2>Custom Seed</h2>
 	 *
 	 * <p>By default, the random <em>seed</em> used for ordering classes is the
-	 * value returned by {@link System#nanoTime()} during static initialization
-	 * of this class. In order to support repeatable builds, the value of the
+	 * value returned by {@link System#nanoTime()} during static class
+	 * initialization. In order to support repeatable builds, the value of the
 	 * default random seed is logged at {@code CONFIG} level. In addition, a
 	 * custom seed (potentially the default seed from the previous test plan
 	 * execution) may be specified via the {@value Random#RANDOM_SEED_PROPERTY_NAME}
@@ -202,15 +201,8 @@ class Random implements ClassOrderer {
 
 		private static final Logger logger = LoggerFactory.getLogger(Random.class);
 
-		/**
-		 * Default seed, which is generated during initialization of this class
-		 * via {@link System#nanoTime()} for reproducibility of tests.
-		 */
-		private static final long DEFAULT_SEED;
-
 		static {
-			DEFAULT_SEED = System.nanoTime();
-			logger.config(() -> "ClassOrderer.Random default seed: " + DEFAULT_SEED);
+			logger.config(() -> "ClassOrderer.Random default seed: " + RandomOrdererUtils.DEFAULT_SEED);
 		}
 
 		/**
@@ -231,7 +223,7 @@ class Random implements ClassOrderer {
 		 *
 		 * @see MethodOrderer.Random
 		 */
-		public static final String RANDOM_SEED_PROPERTY_NAME = MethodOrderer.Random.RANDOM_SEED_PROPERTY_NAME;
+		public static final String RANDOM_SEED_PROPERTY_NAME = RandomOrdererUtils.RANDOM_SEED_PROPERTY_NAME;
 
 		public Random() {
 		}
@@ -243,27 +235,7 @@ public Random() {
 		@Override
 		public void orderClasses(ClassOrdererContext context) {
 			Collections.shuffle(context.getClassDescriptors(),
-				new java.util.Random(getCustomSeed(context).orElse(DEFAULT_SEED)));
-		}
-
-		private Optional<Long> getCustomSeed(ClassOrdererContext context) {
-			return context.getConfigurationParameter(RANDOM_SEED_PROPERTY_NAME).map(configurationParameter -> {
-				Long seed = null;
-				try {
-					seed = Long.valueOf(configurationParameter);
-					logger.config(
-						() -> String.format("Using custom seed for configuration parameter [%s] with value [%s].",
-							RANDOM_SEED_PROPERTY_NAME, configurationParameter));
-				}
-				catch (NumberFormatException ex) {
-					logger.warn(ex,
-						() -> String.format(
-							"Failed to convert configuration parameter [%s] with value [%s] to a long. "
-									+ "Using default seed [%s] as fallback.",
-							RANDOM_SEED_PROPERTY_NAME, configurationParameter, DEFAULT_SEED));
-				}
-				return seed;
-			});
+				new java.util.Random(RandomOrdererUtils.getSeed(context::getConfigurationParameter, logger)));
 		}
 	}
 

diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/MethodOrderer.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/MethodOrderer.java
@@ -248,11 +248,11 @@ private static int getOrder(MethodDescriptor descriptor) {
 	 * <h2>Custom Seed</h2>
 	 *
 	 * <p>By default, the random <em>seed</em> used for ordering methods is the
-	 * value returned by {@link System#nanoTime()} during static initialization
-	 * of this class. In order to support repeatable builds, the value of the
+	 * value returned by {@link System#nanoTime()} during static class
+	 * initialization. In order to support repeatable builds, the value of the
 	 * default random seed is logged at {@code CONFIG} level. In addition, a
 	 * custom seed (potentially the default seed from the previous test plan
-	 * execution) may be specified via the {@value ClassOrderer.Random#RANDOM_SEED_PROPERTY_NAME}
+	 * execution) may be specified via the {@value Random#RANDOM_SEED_PROPERTY_NAME}
 	 * <em>configuration parameter</em> which can be supplied via the {@code Launcher}
 	 * API, build tools (e.g., Gradle and Maven), a JVM system property, or the JUnit
 	 * Platform configuration file (i.e., a file named {@code junit-platform.properties}
@@ -265,15 +265,8 @@ class Random implements MethodOrderer {
 
 		private static final Logger logger = LoggerFactory.getLogger(Random.class);
 
-		/**
-		 * Default seed, which is generated during initialization of this class
-		 * via {@link System#nanoTime()} for reproducibility of tests.
-		 */
-		private static final long DEFAULT_SEED;
-
 		static {
-			DEFAULT_SEED = System.nanoTime();
-			logger.config(() -> "MethodOrderer.Random default seed: " + DEFAULT_SEED);
+			logger.config(() -> "MethodOrderer.Random default seed: " + RandomOrdererUtils.DEFAULT_SEED);
 		}
 
 		/**
@@ -294,7 +287,7 @@ class Random implements MethodOrderer {
 		 *
 		 * @see ClassOrderer.Random
 		 */
-		public static final String RANDOM_SEED_PROPERTY_NAME = "junit.jupiter.execution.order.random.seed";
+		public static final String RANDOM_SEED_PROPERTY_NAME = RandomOrdererUtils.RANDOM_SEED_PROPERTY_NAME;
 
 		public Random() {
 		}
@@ -306,28 +299,9 @@ public Random() {
 		@Override
 		public void orderMethods(MethodOrdererContext context) {
 			Collections.shuffle(context.getMethodDescriptors(),
-				new java.util.Random(getCustomSeed(context).orElse(DEFAULT_SEED)));
+				new java.util.Random(RandomOrdererUtils.getSeed(context::getConfigurationParameter, logger)));
 		}
 
-		private Optional<Long> getCustomSeed(MethodOrdererContext context) {
-			return context.getConfigurationParameter(RANDOM_SEED_PROPERTY_NAME).map(configurationParameter -> {
-				Long seed = null;
-				try {
-					seed = Long.valueOf(configurationParameter);
-					logger.config(
-						() -> String.format("Using custom seed for configuration parameter [%s] with value [%s].",
-							RANDOM_SEED_PROPERTY_NAME, configurationParameter));
-				}
-				catch (NumberFormatException ex) {
-					logger.warn(ex,
-						() -> String.format(
-							"Failed to convert configuration parameter [%s] with value [%s] to a long. "
-									+ "Using default seed [%s] as fallback.",
-							RANDOM_SEED_PROPERTY_NAME, configurationParameter, DEFAULT_SEED));
-				}
-				return seed;
-			});
-		}
 	}
 
 }
diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/RandomOrdererUtils.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/RandomOrdererUtils.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2015-2024 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v2.0 which
+ * accompanies this distribution and is available at
+ *
+ * https://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.jupiter.api;
+
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.junit.platform.commons.logging.Logger;
+
+/**
+ * Shared utility methods for ordering test classes and test methods randomly.
+ *
+ * @since 5.11
+ * @see ClassOrderer.Random
+ * @see MethodOrderer.Random
+ */
+class RandomOrdererUtils {
+
+	static final String RANDOM_SEED_PROPERTY_NAME = "junit.jupiter.execution.order.random.seed";
+
+	static final long DEFAULT_SEED = System.nanoTime();
+
+	static Long getSeed(Function<String, Optional<String>> configurationParameterLookup, Logger logger) {
+		return getCustomSeed(configurationParameterLookup, logger).orElse(DEFAULT_SEED);
+	}
+
+	private static Optional<Long> getCustomSeed(Function<String, Optional<String>> configurationParameterLookup,
+			Logger logger) {
+		return configurationParameterLookup.apply(RANDOM_SEED_PROPERTY_NAME).map(configurationParameter -> {
+			try {
+				logger.config(() -> String.format("Using custom seed for configuration parameter [%s] with value [%s].",
+					RANDOM_SEED_PROPERTY_NAME, configurationParameter));
+				return Long.valueOf(configurationParameter);
+			}
+			catch (NumberFormatException ex) {
+				logger.warn(ex,
+					() -> String.format(
+						"Failed to convert configuration parameter [%s] with value [%s] to a long. "
+								+ "Using default seed [%s] as fallback.",
+						RANDOM_SEED_PROPERTY_NAME, configurationParameter, DEFAULT_SEED));
+				return null;
+			}
+		});
+	}
+}
diff --git a/...ngine/extension/RandomlyOrderedTests.java → ...nit/jupiter/api/RandomlyOrderedTests.java b/...ngine/extension/RandomlyOrderedTests.java → ...nit/jupiter/api/RandomlyOrderedTests.java
@@ -8,7 +8,7 @@
  * https://www.eclipse.org/legal/epl-v20.html
  */
 
-package org.junit.jupiter.engine.extension;
+package org.junit.jupiter.api;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.engine.Constants.DEFAULT_TEST_CLASS_ORDER_PROPERTY_NAME;
@@ -20,11 +20,6 @@
 import java.util.Set;
 import java.util.stream.IntStream;
 
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.ClassOrderer;
-import org.junit.jupiter.api.MethodOrderer;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.TestInfo;
 import org.junit.platform.testkit.engine.EngineTestKit;
 import org.junit.platform.testkit.engine.Events;
 
@@ -39,15 +34,15 @@ class RandomlyOrderedTests {
 	void randomSeedForClassAndMethodOrderingIsDeterministic() {
 		IntStream.range(0, 20).forEach(i -> {
 			callSequence.clear();
-			var tests = executeTests(1618034L);
+			var tests = executeTests(1618034);
 
 			tests.assertStatistics(stats -> stats.succeeded(callSequence.size()));
 			assertThat(callSequence).containsExactlyInAnyOrder("B_TestCase#b", "B_TestCase#c", "B_TestCase#a",
 				"C_TestCase#b", "C_TestCase#c", "C_TestCase#a", "A_TestCase#b", "A_TestCase#c", "A_TestCase#a");
 		});
 	}
 
-	private Events executeTests(long randomSeed) {
+	private Events executeTests(@SuppressWarnings("SameParameterValue") long randomSeed) {
 		// @formatter:off
 		return EngineTestKit
 				.engine("junit-jupiter")
@@ -64,8 +59,8 @@ abstract static class BaseTestCase {
 
 		@BeforeEach
 		void trackInvocations(TestInfo testInfo) {
-			var testClass = testInfo.getTestClass().get();
-			var testMethod = testInfo.getTestMethod().get();
+			var testClass = testInfo.getTestClass().orElseThrow();
+			var testMethod = testInfo.getTestMethod().orElseThrow();
 
 			callSequence.add(testClass.getSimpleName() + "#" + testMethod.getName());
 		}
@@ -83,12 +78,15 @@ void c() {
 		}
 	}
 
+	@SuppressWarnings("NewClassNamingConvention")
 	static class A_TestCase extends BaseTestCase {
 	}
 
+	@SuppressWarnings("NewClassNamingConvention")
 	static class B_TestCase extends BaseTestCase {
 	}
 
+	@SuppressWarnings("NewClassNamingConvention")
 	static class C_TestCase extends BaseTestCase {
 	}