Merge pull request #18042 from MaineC/tests/switch_to_random_value_other_than_for_sort

Get (mostly) rid of random test data generator for sort tests

core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java

@@ -45,8 +45,12 @@ import org.elasticsearch.index.mapper.Mapper.BuilderContext;
 import org.elasticsearch.index.mapper.core.LegacyDoubleFieldMapper.DoubleFieldType;
 import org.elasticsearch.index.mapper.object.ObjectMapper;
 import org.elasticsearch.index.mapper.object.ObjectMapper.Nested;
+import org.elasticsearch.index.query.IdsQueryBuilder;
+import org.elasticsearch.index.query.MatchAllQueryBuilder;
+import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.index.query.QueryShardContext;
+import org.elasticsearch.index.query.TermQueryBuilder;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache;
 import org.elasticsearch.indices.query.IndicesQueriesRegistry;
@@ -252,6 +256,18 @@ public abstract class AbstractSortTestCase<T extends SortBuilder<T>> extends EST
         return doubleFieldType;
     }
 
+    protected static QueryBuilder<?> randomNestedFilter() {
+        int id = randomIntBetween(0, 2);
+        switch(id) {
+            case 0: return (new MatchAllQueryBuilder()).boost(randomFloat());
+            case 1: return (new IdsQueryBuilder()).boost(randomFloat());
+            case 2: return (new TermQueryBuilder(
+                    randomAsciiOfLengthBetween(1, 10),
+                    randomDouble()).boost(randomFloat()));
+            default: throw new IllegalStateException("Only three query builders supported for testing sort");
+        }
+    }
+
     @SuppressWarnings("unchecked")
     private T copyItem(T original) throws IOException {
         try (BytesStreamOutput output = new BytesStreamOutput()) {

core/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java

@@ -27,6 +27,8 @@ import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.query.QueryParseContext;
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
 
 public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder> {
 
@@ -35,31 +37,38 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
         return randomFieldSortBuilder();
     }
 
-    public static FieldSortBuilder randomFieldSortBuilder() {
+    private List<Object> missingContent = Arrays.asList(
+            "_last",
+            "_first",
+            randomAsciiOfLength(10), randomUnicodeOfCodepointLengthBetween(5, 15),
+            randomInt());
+
+
+    public FieldSortBuilder randomFieldSortBuilder() {
         String fieldName = rarely() ? FieldSortBuilder.DOC_FIELD_NAME : randomAsciiOfLengthBetween(1, 10);
         FieldSortBuilder builder = new FieldSortBuilder(fieldName);
         if (randomBoolean()) {
-            builder.order(RandomSortDataGenerator.order(null));
+            builder.order(randomFrom(SortOrder.values()));
         }
 
         if (randomBoolean()) {
-            builder.missing(RandomSortDataGenerator.missing(builder.missing()));
+            builder.missing(randomFrom(missingContent));
         }
 
         if (randomBoolean()) {
-            builder.unmappedType(RandomSortDataGenerator.randomAscii(builder.unmappedType()));
+            builder.unmappedType(randomAsciiOfLengthBetween(1, 10));
         }
 
         if (randomBoolean()) {
-            builder.sortMode(RandomSortDataGenerator.mode(builder.sortMode()));
+            builder.sortMode(randomFrom(SortMode.values()));
         }
 
         if (randomBoolean()) {
-            builder.setNestedFilter(RandomSortDataGenerator.nestedFilter(builder.getNestedFilter()));
+            builder.setNestedFilter(randomNestedFilter());
         }
 
         if (randomBoolean()) {
-            builder.setNestedPath(RandomSortDataGenerator.randomAscii(builder.getNestedPath()));
+            builder.setNestedPath(randomAsciiOfLengthBetween(1, 10));
         }
 
         return builder;
@@ -71,22 +80,28 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
         int parameter = randomIntBetween(0, 5);
         switch (parameter) {
         case 0:
-            mutated.setNestedPath(RandomSortDataGenerator.randomAscii(mutated.getNestedPath()));
+            mutated.setNestedPath(randomValueOtherThan(
+                    original.getNestedPath(),
+                    () -> randomAsciiOfLengthBetween(1, 10)));
             break;
         case 1:
-            mutated.setNestedFilter(RandomSortDataGenerator.nestedFilter(mutated.getNestedFilter()));
+            mutated.setNestedFilter(randomValueOtherThan(
+                    original.getNestedFilter(),
+                    () -> randomNestedFilter()));
             break;
         case 2:
-            mutated.sortMode(RandomSortDataGenerator.mode(mutated.sortMode()));
+            mutated.sortMode(randomValueOtherThan(original.sortMode(), () -> randomFrom(SortMode.values())));
             break;
         case 3:
-            mutated.unmappedType(RandomSortDataGenerator.randomAscii(mutated.unmappedType()));
+            mutated.unmappedType(randomValueOtherThan(
+                    original.unmappedType(),
+                    () -> randomAsciiOfLengthBetween(1, 10)));
             break;
         case 4:
-            mutated.missing(RandomSortDataGenerator.missing(mutated.missing()));
+            mutated.missing(randomValueOtherThan(original.missing(), () -> randomFrom(missingContent)));
             break;
         case 5:
-            mutated.order(RandomSortDataGenerator.order(mutated.order()));
+            mutated.order(randomValueOtherThan(original.order(), () -> randomFrom(SortOrder.values())));
             break;
         default:
             throw new IllegalStateException("Unsupported mutation.");

core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java

@@ -82,16 +82,19 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
             result.unit(unit(result.unit()));
         }
         if (randomBoolean()) {
-            result.order(RandomSortDataGenerator.order(null));
+            result.order(randomFrom(SortOrder.values()));
         }
         if (randomBoolean()) {
             result.sortMode(mode(result.sortMode()));
         }
         if (randomBoolean()) {
-            result.setNestedFilter(RandomSortDataGenerator.nestedFilter(result.getNestedFilter()));
+            result.setNestedFilter(randomNestedFilter());
         }
         if (randomBoolean()) {
-            result.setNestedPath(RandomSortDataGenerator.randomAscii(result.getNestedPath()));
+            result.setNestedPath(
+                    randomValueOtherThan(
+                            result.getNestedPath(),
+                            () -> randomAsciiOfLengthBetween(1, 10)));
         }
         if (randomBoolean()) {
             result.coerce(! result.coerce());
@@ -167,16 +170,20 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
             result.unit(unit(original.unit()));
             break;
         case 4:
-            result.order(RandomSortDataGenerator.order(original.order()));
+            result.order(randomValueOtherThan(original.order(), () -> randomFrom(SortOrder.values())));
             break;
         case 5:
             result.sortMode(mode(original.sortMode()));
             break;
         case 6:
-            result.setNestedFilter(RandomSortDataGenerator.nestedFilter(original.getNestedFilter()));
+            result.setNestedFilter(randomValueOtherThan(
+                    original.getNestedFilter(),
+                    () -> randomNestedFilter()));
             break;
         case 7:
-            result.setNestedPath(RandomSortDataGenerator.randomAscii(original.getNestedPath()));
+            result.setNestedPath(randomValueOtherThan(
+                    result.getNestedPath(),
+                    () -> randomAsciiOfLengthBetween(1, 10)));
             break;
         case 8:
             result.coerce(! original.coerce());

core/src/test/java/org/elasticsearch/search/sort/RandomSortDataGenerator.java

@@ -1,124 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.search.sort;
-
-import org.elasticsearch.common.Nullable;
-import org.elasticsearch.index.query.IdsQueryBuilder;
-import org.elasticsearch.index.query.MatchAllQueryBuilder;
-import org.elasticsearch.index.query.QueryBuilder;
-import org.elasticsearch.index.query.TermQueryBuilder;
-import org.elasticsearch.test.ESTestCase;
-
-import java.util.HashSet;
-import java.util.Set;
-
-public class RandomSortDataGenerator {
-    private RandomSortDataGenerator() {
-        // this is a helper class only, doesn't need a constructor
-    }
-
-    public static QueryBuilder nestedFilter(QueryBuilder original) {
-        @SuppressWarnings("rawtypes")
-        QueryBuilder nested = null;
-        while (nested == null || nested.equals(original)) {
-            switch (ESTestCase.randomInt(2)) {
-            case 0:
-                nested = new MatchAllQueryBuilder();
-                break;
-            case 1:
-                nested = new IdsQueryBuilder();
-                break;
-            default:
-            case 2:
-                nested = new TermQueryBuilder(ESTestCase.randomAsciiOfLengthBetween(1, 10), ESTestCase.randomDouble());
-                break;
-            }
-            nested.boost((float) ESTestCase.randomDoubleBetween(0, 10, false));
-        }
-        return nested;
-    }
-
-    public static String randomAscii(String original) {
-        String nestedPath = ESTestCase.randomAsciiOfLengthBetween(1, 10);
-        while (nestedPath.equals(original)) {
-            nestedPath = ESTestCase.randomAsciiOfLengthBetween(1, 10);
-        }
-        return nestedPath;
-    }
-
-    public static SortMode mode(SortMode original) {
-        Set<SortMode> set = new HashSet<>();
-        set.add(original);
-        return mode(set);
-    }
-
-    public static SortMode mode(Set<SortMode> except) {
-        SortMode mode = ESTestCase.randomFrom(SortMode.values());
-        while (except.contains(mode)) {
-            mode = ESTestCase.randomFrom(SortMode.values());
-        }
-        return mode;
-    }
-
-    public static Object missing(Object original) {
-        Object missing = null;
-        Object otherMissing = original;
-
-        while (missing == null || missing.equals(otherMissing)) {
-          int missingId = ESTestCase.randomIntBetween(0, 4);
-          switch (missingId) {
-          case 0:
-              missing = ("_last");
-              break;
-          case 1:
-              missing = ("_first");
-              break;
-          case 2:
-              missing = ESTestCase.randomAsciiOfLength(10);
-              break;
-          case 3:
-              missing = ESTestCase.randomUnicodeOfCodepointLengthBetween(5, 15);
-              break;
-          case 4:
-              missing = ESTestCase.randomInt();
-              break;
-          default:
-              throw new IllegalStateException("Unknown missing type.");
-
-          }
-        }
-        return missing;
-    }
-
-    /**
-     * return a random {@link SortOrder} settings, except the one provided by parameter if set
-     */
-    public static SortOrder order(@Nullable SortOrder original) {
-        if (original == null) {
-            return ESTestCase.randomBoolean() ? SortOrder.ASC : SortOrder.DESC;
-        }
-        if (original.equals(SortOrder.ASC)) {
-            return SortOrder.DESC;
-        } else {
-            return SortOrder.ASC;
-        }
-    }
-
-}

core/src/test/java/org/elasticsearch/search/sort/ScoreSortBuilderTests.java

@@ -45,7 +45,7 @@ public class ScoreSortBuilderTests extends AbstractSortTestCase<ScoreSortBuilder
     @Override
     protected ScoreSortBuilder mutate(ScoreSortBuilder original) throws IOException {
         ScoreSortBuilder result = new ScoreSortBuilder();
-        result.order(RandomSortDataGenerator.order(original.order()));
+        result.order(randomValueOtherThan(original.order(), () -> randomFrom(SortOrder.values())));
         return result;
     }
 

core/src/test/java/org/elasticsearch/search/sort/ScriptSortBuilderTests.java

@@ -48,24 +48,24 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
         ScriptSortBuilder builder = new ScriptSortBuilder(new Script(randomAsciiOfLengthBetween(5, 10)),
                 type);
         if (randomBoolean()) {
-                builder.order(RandomSortDataGenerator.order(null));
+                builder.order(randomFrom(SortOrder.values()));
         }
         if (randomBoolean()) {
             if (type == ScriptSortType.NUMBER) {
-                builder.sortMode(RandomSortDataGenerator.mode(builder.sortMode()));
+                builder.sortMode(randomValueOtherThan(builder.sortMode(), () -> randomFrom(SortMode.values())));
             } else {
                 Set<SortMode> exceptThis = new HashSet<>();
                 exceptThis.add(SortMode.SUM);
                 exceptThis.add(SortMode.AVG);
                 exceptThis.add(SortMode.MEDIAN);
-                builder.sortMode(RandomSortDataGenerator.mode(exceptThis));
+                builder.sortMode(randomValueOtherThanMany(exceptThis::contains, () -> randomFrom(SortMode.values())));
             }
         }
         if (randomBoolean()) {
-            builder.setNestedFilter(RandomSortDataGenerator.nestedFilter(builder.getNestedFilter()));
+            builder.setNestedFilter(randomNestedFilter());
         }
         if (randomBoolean()) {
-            builder.setNestedPath(RandomSortDataGenerator.randomAscii(builder.getNestedPath()));
+            builder.setNestedPath(randomAsciiOfLengthBetween(1, 10));
         }
         return builder;
     }
@@ -101,7 +101,7 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
                 break;
             case 1:
                 if (original.type() == ScriptSortType.NUMBER) {
-                    result.sortMode(RandomSortDataGenerator.mode(original.sortMode()));
+                    result.sortMode(randomValueOtherThan(result.sortMode(), () -> randomFrom(SortMode.values())));
                 } else {
                     // script sort type String only allows MIN and MAX, so we only switch
                     if (original.sortMode() == SortMode.MIN) {
@@ -112,7 +112,9 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
                 }
                 break;
             case 2:
-                result.setNestedFilter(RandomSortDataGenerator.nestedFilter(original.getNestedFilter()));
+                result.setNestedFilter(randomValueOtherThan(
+                        original.getNestedFilter(),
+                        () -> randomNestedFilter()));
                 break;
             case 3:
                 result.setNestedPath(original.getNestedPath() + "_some_suffix");

test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

@@ -87,6 +87,7 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.function.BooleanSupplier;
 import java.util.function.Consumer;
+import java.util.function.Predicate;
 import java.util.function.Supplier;
 
 import static org.elasticsearch.common.util.CollectionUtils.arrayAsArrayList;
@@ -410,10 +411,21 @@ public abstract class ESTestCase extends LuceneTestCase {
      * helper to get a random value in a certain range that's different from the input
      */
     public static <T> T randomValueOtherThan(T input, Supplier<T> randomSupplier) {
+        if (input != null) {
+            return randomValueOtherThanMany(input::equals, randomSupplier);
+        }
+        
+        return(randomSupplier.get());
+    }
+
+    /**
+     * helper to get a random value in a certain range that's different from the input
+     */
+    public static <T> T randomValueOtherThanMany(Predicate<T> input, Supplier<T> randomSupplier) {
         T randomValue = null;
         do {
             randomValue = randomSupplier.get();
-        } while (randomValue.equals(input));
+        } while (input.test(randomValue));
         return randomValue;
     }