From babd3b9bcfb7cca7519cf424946b059111e43ad0 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Tue, 15 Jul 2025 15:11:20 -0400 Subject: [PATCH] Clean up non-parameterized tests in the wrong place (#131049) In the course of other work I found a few places where we were creating non-parameterized tests from within the parameterized test drivers for a few ESQL functions. This causes those tests to be run for every parameter combination, even though the tests themselves do not change anything, resulting in a lot of extra test overhead for no additional coverage. I cleaned up three classes: EndsWithTests ran 832 tests before this, and now runs 640 tests StartsWithTests ran 208 before this, and now runs 160 InTests ran 168 before this, and now runs 28 So overall, that's 369 redundant test runs removed (including the fact that the 11 tests I moved still run once in their new classes), and further savings if we later expand those parameterized tests. --- .../scalar/string/EndsWithStaticTests.java | 59 +++++++++++++++++++ .../function/scalar/string/EndsWithTests.java | 41 ------------- .../scalar/string/StartsWithStaticTests.java | 59 +++++++++++++++++++ .../scalar/string/StartsWithTests.java | 41 ------------- .../operator/comparison/InStaticTests.java | 49 +++++++++++++++ .../operator/comparison/InTests.java | 39 ------------ 6 files changed, 167 insertions(+), 121 deletions(-) create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWithStaticTests.java create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWithStaticTests.java create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InStaticTests.java diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWithStaticTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWithStaticTests.java new file mode 100644 index 0000000000000..6cc383bd31c23 --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWithStaticTests.java @@ -0,0 +1,59 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.string; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; +import org.elasticsearch.xpack.esql.core.expression.Literal; +import org.elasticsearch.xpack.esql.core.querydsl.query.Query; +import org.elasticsearch.xpack.esql.core.querydsl.query.WildcardQuery; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.core.type.DataType; +import org.elasticsearch.xpack.esql.core.type.EsField; +import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates; +import org.elasticsearch.xpack.esql.planner.TranslatorHandler; + +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; + +public class EndsWithStaticTests extends ESTestCase { + public void testLuceneQuery_AllLiterals_NonTranslatable() { + EndsWith function = new EndsWith( + Source.EMPTY, + new Literal(Source.EMPTY, "test", DataType.KEYWORD), + new Literal(Source.EMPTY, "test", DataType.KEYWORD) + ); + + ESTestCase.assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false)); + } + + public void testLuceneQuery_NonFoldableSuffix_NonTranslatable() { + EndsWith function = new EndsWith( + Source.EMPTY, + new FieldAttribute(Source.EMPTY, "field", new EsField("field", DataType.KEYWORD, Map.of(), true)), + new FieldAttribute(Source.EMPTY, "field", new EsField("suffix", DataType.KEYWORD, Map.of(), true)) + ); + + assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false)); + } + + public void testLuceneQuery_NonFoldableSuffix_Translatable() { + EndsWith function = new EndsWith( + Source.EMPTY, + new FieldAttribute(Source.EMPTY, "field", new EsField("suffix", DataType.KEYWORD, Map.of(), true)), + new Literal(Source.EMPTY, "a*b?c\\", DataType.KEYWORD) + ); + + assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(true)); + + Query query = function.asQuery(TranslatorHandler.TRANSLATOR_HANDLER); + + assertThat(query, equalTo(new WildcardQuery(Source.EMPTY, "field", "*a\\*b\\?c\\\\", false))); + } +} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWithTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWithTests.java index a5fe9d7c78b68..c41b1e14257ee 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWithTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWithTests.java @@ -12,21 +12,14 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.xpack.esql.core.expression.Expression; -import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; -import org.elasticsearch.xpack.esql.core.expression.Literal; -import org.elasticsearch.xpack.esql.core.querydsl.query.WildcardQuery; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; -import org.elasticsearch.xpack.esql.core.type.EsField; import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase; import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; -import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates; -import org.elasticsearch.xpack.esql.planner.TranslatorHandler; import org.hamcrest.Matcher; import java.util.LinkedList; import java.util.List; -import java.util.Map; import java.util.function.Supplier; import static org.hamcrest.Matchers.equalTo; @@ -105,38 +98,4 @@ private static TestCaseSupplier.TestCase testCase( protected Expression build(Source source, List args) { return new EndsWith(source, args.get(0), args.get(1)); } - - public void testLuceneQuery_AllLiterals_NonTranslatable() { - var function = new EndsWith( - Source.EMPTY, - new Literal(Source.EMPTY, "test", DataType.KEYWORD), - new Literal(Source.EMPTY, "test", DataType.KEYWORD) - ); - - assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false)); - } - - public void testLuceneQuery_NonFoldableSuffix_NonTranslatable() { - var function = new EndsWith( - Source.EMPTY, - new FieldAttribute(Source.EMPTY, "field", new EsField("field", DataType.KEYWORD, Map.of(), true)), - new FieldAttribute(Source.EMPTY, "field", new EsField("suffix", DataType.KEYWORD, Map.of(), true)) - ); - - assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false)); - } - - public void testLuceneQuery_NonFoldableSuffix_Translatable() { - var function = new EndsWith( - Source.EMPTY, - new FieldAttribute(Source.EMPTY, "field", new EsField("suffix", DataType.KEYWORD, Map.of(), true)), - new Literal(Source.EMPTY, "a*b?c\\", DataType.KEYWORD) - ); - - assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(true)); - - var query = function.asQuery(TranslatorHandler.TRANSLATOR_HANDLER); - - assertThat(query, equalTo(new WildcardQuery(Source.EMPTY, "field", "*a\\*b\\?c\\\\"))); - } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWithStaticTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWithStaticTests.java new file mode 100644 index 0000000000000..b05a2facf430f --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWithStaticTests.java @@ -0,0 +1,59 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.string; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; +import org.elasticsearch.xpack.esql.core.expression.Literal; +import org.elasticsearch.xpack.esql.core.querydsl.query.WildcardQuery; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.core.type.DataType; +import org.elasticsearch.xpack.esql.core.type.EsField; +import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates; +import org.elasticsearch.xpack.esql.planner.TranslatorHandler; + +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; + +public class StartsWithStaticTests extends ESTestCase { + + public void testLuceneQuery_AllLiterals_NonTranslatable() { + var function = new StartsWith( + Source.EMPTY, + new Literal(Source.EMPTY, "test", DataType.KEYWORD), + new Literal(Source.EMPTY, "test", DataType.KEYWORD) + ); + + assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false)); + } + + public void testLuceneQuery_NonFoldablePrefix_NonTranslatable() { + var function = new StartsWith( + Source.EMPTY, + new FieldAttribute(Source.EMPTY, "field", new EsField("field", DataType.KEYWORD, Map.of(), true)), + new FieldAttribute(Source.EMPTY, "field", new EsField("prefix", DataType.KEYWORD, Map.of(), true)) + ); + + assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false)); + } + + public void testLuceneQuery_NonFoldablePrefix_Translatable() { + var function = new StartsWith( + Source.EMPTY, + new FieldAttribute(Source.EMPTY, "field", new EsField("prefix", DataType.KEYWORD, Map.of(), true)), + new Literal(Source.EMPTY, "a*b?c\\", DataType.KEYWORD) + ); + + assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(true)); + + var query = function.asQuery(TranslatorHandler.TRANSLATOR_HANDLER); + + assertThat(query, equalTo(new WildcardQuery(Source.EMPTY, "field", "a\\*b\\?c\\\\*", false))); + } +} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWithTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWithTests.java index 06d2757766060..789059fb7b6ba 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWithTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWithTests.java @@ -12,20 +12,13 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.xpack.esql.core.expression.Expression; -import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; -import org.elasticsearch.xpack.esql.core.expression.Literal; -import org.elasticsearch.xpack.esql.core.querydsl.query.WildcardQuery; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; -import org.elasticsearch.xpack.esql.core.type.EsField; import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase; import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; -import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates; -import org.elasticsearch.xpack.esql.planner.TranslatorHandler; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.function.Supplier; import static org.hamcrest.Matchers.equalTo; @@ -65,38 +58,4 @@ public static Iterable parameters() { protected Expression build(Source source, List args) { return new StartsWith(source, args.get(0), args.get(1)); } - - public void testLuceneQuery_AllLiterals_NonTranslatable() { - var function = new StartsWith( - Source.EMPTY, - new Literal(Source.EMPTY, "test", DataType.KEYWORD), - new Literal(Source.EMPTY, "test", DataType.KEYWORD) - ); - - assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false)); - } - - public void testLuceneQuery_NonFoldablePrefix_NonTranslatable() { - var function = new StartsWith( - Source.EMPTY, - new FieldAttribute(Source.EMPTY, "field", new EsField("field", DataType.KEYWORD, Map.of(), true)), - new FieldAttribute(Source.EMPTY, "field", new EsField("prefix", DataType.KEYWORD, Map.of(), true)) - ); - - assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false)); - } - - public void testLuceneQuery_NonFoldablePrefix_Translatable() { - var function = new StartsWith( - Source.EMPTY, - new FieldAttribute(Source.EMPTY, "field", new EsField("prefix", DataType.KEYWORD, Map.of(), true)), - new Literal(Source.EMPTY, "a*b?c\\", DataType.KEYWORD) - ); - - assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(true)); - - var query = function.asQuery(TranslatorHandler.TRANSLATOR_HANDLER); - - assertThat(query, equalTo(new WildcardQuery(Source.EMPTY, "field", "a\\*b\\?c\\\\*"))); - } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InStaticTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InStaticTests.java new file mode 100644 index 0000000000000..ca79d80ca1993 --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InStaticTests.java @@ -0,0 +1,49 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.predicate.operator.comparison; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.esql.core.expression.FoldContext; +import org.elasticsearch.xpack.esql.core.expression.Literal; + +import java.util.Arrays; + +import static org.elasticsearch.xpack.esql.EsqlTestUtils.L; +import static org.elasticsearch.xpack.esql.core.expression.Literal.NULL; +import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY; + +public class InStaticTests extends ESTestCase { + private static final Literal ONE = L(1); + private static final Literal TWO = L(2); + private static final Literal THREE = L(3); + + public void testInWithContainedValue() { + In in = new In(EMPTY, TWO, Arrays.asList(ONE, TWO, THREE)); + assertTrue((Boolean) in.fold(FoldContext.small())); + } + + public void testInWithNotContainedValue() { + In in = new In(EMPTY, THREE, Arrays.asList(ONE, TWO)); + assertFalse((Boolean) in.fold(FoldContext.small())); + } + + public void testHandleNullOnLeftValue() { + In in = new In(EMPTY, NULL, Arrays.asList(ONE, TWO, THREE)); + assertNull(in.fold(FoldContext.small())); + in = new In(EMPTY, NULL, Arrays.asList(ONE, NULL, THREE)); + assertNull(in.fold(FoldContext.small())); + + } + + public void testHandleNullsOnRightValue() { + In in = new In(EMPTY, THREE, Arrays.asList(ONE, NULL, THREE)); + assertTrue((Boolean) in.fold(FoldContext.small())); + in = new In(EMPTY, ONE, Arrays.asList(TWO, NULL, THREE)); + assertNull(in.fold(FoldContext.small())); + } +} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java index 62e3e1577740f..e736e0c58a238 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java @@ -13,8 +13,6 @@ import org.elasticsearch.geo.GeometryTestUtils; import org.elasticsearch.geo.ShapeTestUtils; import org.elasticsearch.xpack.esql.core.expression.Expression; -import org.elasticsearch.xpack.esql.core.expression.FoldContext; -import org.elasticsearch.xpack.esql.core.expression.Literal; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase; @@ -22,14 +20,10 @@ import org.junit.AfterClass; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.function.Supplier; -import static org.elasticsearch.xpack.esql.EsqlTestUtils.of; import static org.elasticsearch.xpack.esql.EsqlTestUtils.randomLiteral; -import static org.elasticsearch.xpack.esql.core.expression.Literal.NULL; -import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY; import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_POINT; import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_SHAPE; import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_POINT; @@ -45,39 +39,6 @@ public InTests(@Name("TestCase") Supplier testCaseSup this.testCase = testCaseSupplier.get(); } - private static final Literal ONE = L(1); - private static final Literal TWO = L(2); - private static final Literal THREE = L(3); - - public void testInWithContainedValue() { - In in = new In(EMPTY, TWO, Arrays.asList(ONE, TWO, THREE)); - assertTrue((Boolean) in.fold(FoldContext.small())); - } - - public void testInWithNotContainedValue() { - In in = new In(EMPTY, THREE, Arrays.asList(ONE, TWO)); - assertFalse((Boolean) in.fold(FoldContext.small())); - } - - public void testHandleNullOnLeftValue() { - In in = new In(EMPTY, NULL, Arrays.asList(ONE, TWO, THREE)); - assertNull(in.fold(FoldContext.small())); - in = new In(EMPTY, NULL, Arrays.asList(ONE, NULL, THREE)); - assertNull(in.fold(FoldContext.small())); - - } - - public void testHandleNullsOnRightValue() { - In in = new In(EMPTY, THREE, Arrays.asList(ONE, NULL, THREE)); - assertTrue((Boolean) in.fold(FoldContext.small())); - in = new In(EMPTY, ONE, Arrays.asList(TWO, NULL, THREE)); - assertNull(in.fold(FoldContext.small())); - } - - private static Literal L(Object value) { - return of(EMPTY, value); - } - @ParametersFactory public static Iterable parameters() { List suppliers = new ArrayList<>();