From a422c5a225c7e3b392b1efbfac291296d528740e Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Fri, 17 May 2024 15:09:02 +0200 Subject: [PATCH] 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. --- .../release-notes-5.11.0-M2.adoc | 3 +- .../org/junit/jupiter/api/ClassOrderer.java | 38 ++----------- .../org/junit/jupiter/api/MethodOrderer.java | 38 +++---------- .../junit/jupiter/api/RandomOrdererUtils.java | 53 +++++++++++++++++++ .../RandomlyOrderedTests.java | 18 +++---- 5 files changed, 74 insertions(+), 76 deletions(-) create mode 100644 junit-jupiter-api/src/main/java/org/junit/jupiter/api/RandomOrdererUtils.java rename junit-jupiter-engine/src/test/java/org/junit/jupiter/{engine/extension => api}/RandomlyOrderedTests.java (84%) 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 index 43cec87bfd3..181edc369f5 100644 --- 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 index 10c232ea816..e27c05abb92 100644 --- 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) { *

Custom Seed

* *

By default, the random seed 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 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 index c84f8770733..4a71d594455 100644 --- 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) { *

Custom Seed

* *

By default, the random seed 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} * configuration parameter 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 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 new file mode 100644 index 00000000000..35b5dc6208a --- /dev/null +++ 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> configurationParameterLookup, Logger logger) { + return getCustomSeed(configurationParameterLookup, logger).orElse(DEFAULT_SEED); + } + + private static Optional getCustomSeed(Function> 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/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/RandomlyOrderedTests.java b/junit-jupiter-engine/src/test/java/org/junit/jupiter/api/RandomlyOrderedTests.java similarity index 84% rename from junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/RandomlyOrderedTests.java rename to junit-jupiter-engine/src/test/java/org/junit/jupiter/api/RandomlyOrderedTests.java index 8c75b56d01a..f43b586c03d 100644 --- a/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/RandomlyOrderedTests.java +++ b/junit-jupiter-engine/src/test/java/org/junit/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,7 +34,7 @@ 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", @@ -47,7 +42,7 @@ void randomSeedForClassAndMethodOrderingIsDeterministic() { }); } - 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 { }