Add mix64 to XORShiftRandom constructor to make newRandom(random.next…

…Long()) better See jqwik-team#423
vlsi · Dec 12, 2022 · 3de9ca9 · 3de9ca9
1 parent e671d55
commit 3de9ca9
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 9 deletions.
diff --git a/engine/src/main/java/net/jqwik/engine/SourceOfRandomness.java b/engine/src/main/java/net/jqwik/engine/SourceOfRandomness.java
@@ -62,10 +62,20 @@ private XORShiftRandom() {
 		}
 
 		private XORShiftRandom(long seed) {
-			if (seed == 0l) {
-				throw new IllegalArgumentException("0L is not an allowed seed value");
+			this.seed = mix64(seed);
+			if (this.seed == 0) {
+				// 0 is invalid for XorShift seed, so we set it to a non-zero value
+				this.seed = 0xbf58476d1ce4e5b9L;
 			}
-			this.seed = seed;
+		}
+
+		/**
+		* See <a href="http://zimbry.blogspot.com/2011/09/better-bit-mixing-improving-on.html">Better Bit Mixing - Improving on MurmurHash3's 64-bit Finalizer</a>
+		*/
+		private static long mix64(long z) {
+			z = (z ^ (z >>> 30)) * 0xbf58476d1ce4e5b9L;
+			z = (z ^ (z >>> 27)) * 0x94d049bb133111ebL;
+			return z ^ (z >>> 31);
 		}
 
 		@Override

diff --git a/engine/src/test/java/net/jqwik/engine/properties/CheckedPropertyTests.java b/engine/src/test/java/net/jqwik/engine/properties/CheckedPropertyTests.java
@@ -3,6 +3,7 @@
 import java.util.*;
 import java.util.function.*;
 
+import org.assertj.core.api.*;
 import org.junit.platform.engine.reporting.*;
 import org.opentest4j.*;
 
@@ -16,6 +17,7 @@
 import net.jqwik.testing.*;
 
 import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.SoftAssertions.*;
 
 import static net.jqwik.api.GenerationMode.*;
 import static net.jqwik.engine.TestHelper.*;
@@ -263,25 +265,54 @@ void usePreviouslyFailedGeneration() {
 			Arbitrary<Integer> integers = Arbitraries.integers().between(1, 99);
 			GenerationInfo previousGenerationInfo = new GenerationInfo("41", 13);
 			// This is what's being generated from integers in the 13th attempt
-			List<Integer> previousSample = Arrays.asList(99, 97);
+			List<Integer> expectedParameterValues = Arrays.asList(65, 77);
 
-			CheckedFunction checkSample = params -> params.equals(previousSample);
+			CheckedFunction checkSample = params -> {
+				Assertions.assertThat(params)
+					.describedAs("sampleProperty initial params should reuse GenerationInfo supplied in the config. " +
+									"If you see failure here, then it looks like the random generation strategy has changed. " +
+									"You might need to adjust expectedParameterValues = ... in usePreviouslyFailedGeneration() property test.")
+					.isEqualTo(expectedParameterValues);
+				return true;
+			};
 
 			CheckedProperty checkedProperty = createCheckedProperty(
 				"sampleProperty", checkSample, getParametersForMethod("sampleProperty"),
 				p -> Collections.singleton(integers),
 				Optional.empty(),
 				aConfig()
 					.withPreviousFailureGeneration(previousGenerationInfo)
+					// Disable shrinking, so checkSample fails on the first attempt, and we can see the first failure,
+					// and not the result of the shrinking which will be always [1, 1]
+					.withShrinking(ShrinkingMode.OFF)
 					.withAfterFailure(AfterFailureMode.SAMPLE_ONLY).build(),
 				lifecycleContextForMethod("sampleProperty", int.class, int.class)
 			);
 
 			PropertyCheckResult check = checkedProperty.check(new Reporting[0]);
-			assertThat(check.countTries()).isEqualTo(1);
-			assertThat(check.seed()).isEqualTo(Optional.of("41"));
-			assertThat(check.checkStatus()).isEqualTo(SUCCESSFUL);
-			assertThat(check.falsifiedParameters()).isEmpty();
+			assertSoftly(
+				softly -> {
+					// Rethrow the error, so the stacktrace is meaningful, and the assert message in checkSample is printed
+					check.throwable().ifPresent(
+						e -> softly.assertThat(e)
+								   // softly.fail("...", e) would not print stacktrace for some reason
+								   .describedAs("checkSample property failed")
+								   .doesNotThrowAnyException()
+					);
+					softly.assertThat(check.countTries())
+						.describedAs("check.countTries()")
+						.isEqualTo(1);
+					softly.assertThat(check.seed())
+						.describedAs("check.seed()")
+						.contains("41");
+					softly.assertThat(check.checkStatus())
+						.describedAs("check.checkStatus()")
+						.isEqualTo(SUCCESSFUL);
+					softly.assertThat(check.falsifiedParameters())
+						.describedAs("check.falsifiedParameters()")
+						.isEmpty();
+				}
+			);
 		}
 
 		@SuppressWarnings("unchecked")