From 286ff2264c1adf7d8cca3d2cd8d5c63bdd5b6939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 4 Dec 2018 18:36:43 +0100 Subject: [PATCH 1/3] Make sure test don't use Math.random for reproducability Currently we use Math.random() in a few places in the tests which makes these tests not reproducable with the random seed mechanism that comes with ESTestCase. The change removes those instances. --- .../common/util/ArrayUtilsTests.java | 21 ++++++------------- .../aggregations/support/PathTests.java | 2 +- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/util/ArrayUtilsTests.java b/server/src/test/java/org/elasticsearch/common/util/ArrayUtilsTests.java index 8a346ed4fbebc..128a180249596 100644 --- a/server/src/test/java/org/elasticsearch/common/util/ArrayUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/common/util/ArrayUtilsTests.java @@ -30,15 +30,15 @@ public class ArrayUtilsTests extends ESTestCase { public void testBinarySearch() throws Exception { for (int j = 0; j < 100; j++) { - int index = Math.min(randomInt(0, 10), 9); - double tolerance = Math.random() * 0.01; - double lookForValue = randomFreq(0.9) ? -1 : Double.NaN; // sometimes we'll look for NaN + int index = randomIntBetween(0, 9); + double tolerance = randomDoubleBetween(0, 0.01, true); + double lookForValue = frequently() ? -1 : Double.NaN; // sometimes we'll look for NaN double[] array = new double[10]; for (int i = 0; i < array.length; i++) { double value; - if (randomFreq(0.9)) { - value = Math.random() * 10; - array[i] = value + ((randomFreq(0.5) ? 1 : -1) * Math.random() * tolerance); + if (frequently()) { + value = randomIntBetween(0, 10); + array[i] = value + ((randomBoolean() ? 1 : -1) * randomDouble() * tolerance); } else { // sometimes we'll have NaN in the array value = Double.NaN; @@ -73,15 +73,6 @@ public void testBinarySearch() throws Exception { } } - private boolean randomFreq(double freq) { - return Math.random() < freq; - } - - private int randomInt(int min, int max) { - int delta = (int) (Math.random() * (max - min)); - return min + delta; - } - public void testConcat() { assertArrayEquals(new String[]{"a", "b", "c", "d"}, ArrayUtils.concat(new String[]{"a", "b"}, new String[]{"c", "d"})); int firstSize = randomIntBetween(0, 10); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java index c5c681c117f1c..c18a3523e7a21 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java @@ -82,7 +82,7 @@ Tokens add(String name) { } Tokens add(String name, String key) { - if (Math.random() > 0.5) { + if (randomBoolean()) { tokens.add(new AggregationPath.PathElement(name + "." + key, name, key)); } else { tokens.add(new AggregationPath.PathElement(name + "[" + key + "]", name, key)); From a6462adf5d625ee2b893589da6d03d99b44a7df4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 5 Dec 2018 01:16:07 +0100 Subject: [PATCH 2/3] iter --- .../java/org/elasticsearch/common/util/ArrayUtilsTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/common/util/ArrayUtilsTests.java b/server/src/test/java/org/elasticsearch/common/util/ArrayUtilsTests.java index 128a180249596..8a81f9f818ff5 100644 --- a/server/src/test/java/org/elasticsearch/common/util/ArrayUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/common/util/ArrayUtilsTests.java @@ -37,7 +37,7 @@ public void testBinarySearch() throws Exception { for (int i = 0; i < array.length; i++) { double value; if (frequently()) { - value = randomIntBetween(0, 10); + value = randomDoubleBetween(0, 9, true); array[i] = value + ((randomBoolean() ? 1 : -1) * randomDouble() * tolerance); } else { // sometimes we'll have NaN in the array From 7a2b596189a3d82981e7c16d1edb54ee7676d9dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 5 Dec 2018 12:36:48 +0100 Subject: [PATCH 3/3] Add Math.random(() to forbidden-apis for tests --- buildSrc/src/main/resources/forbidden/es-test-signatures.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/buildSrc/src/main/resources/forbidden/es-test-signatures.txt b/buildSrc/src/main/resources/forbidden/es-test-signatures.txt index 08e591e1cfa40..766e13878cc25 100644 --- a/buildSrc/src/main/resources/forbidden/es-test-signatures.txt +++ b/buildSrc/src/main/resources/forbidden/es-test-signatures.txt @@ -25,3 +25,5 @@ org.apache.lucene.util.LuceneTestCase$Nightly @ We don't run nightly tests at th com.carrotsearch.randomizedtesting.annotations.Nightly @ We don't run nightly tests at this point! org.junit.Test @defaultMessage Just name your test method testFooBar + +java.lang.Math#random() @ Use one of the various randomization methods from LuceneTestCase or ESTestCase for reproducibility