Skip to content

Commit e03b669

Browse files
Clean up non-parameterized tests in the wrong place (#131049) (#131327)
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. Co-authored-by: Mark Tozzi <[email protected]>
1 parent 27534b3 commit e03b669

File tree

6 files changed

+167
-121
lines changed

6 files changed

+167
-121
lines changed
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.esql.expression.function.scalar.string;
9+
10+
import org.elasticsearch.test.ESTestCase;
11+
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
12+
import org.elasticsearch.xpack.esql.core.expression.Literal;
13+
import org.elasticsearch.xpack.esql.core.querydsl.query.Query;
14+
import org.elasticsearch.xpack.esql.core.querydsl.query.WildcardQuery;
15+
import org.elasticsearch.xpack.esql.core.tree.Source;
16+
import org.elasticsearch.xpack.esql.core.type.DataType;
17+
import org.elasticsearch.xpack.esql.core.type.EsField;
18+
import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates;
19+
import org.elasticsearch.xpack.esql.planner.TranslatorHandler;
20+
21+
import java.util.Map;
22+
23+
import static org.hamcrest.Matchers.equalTo;
24+
25+
public class EndsWithStaticTests extends ESTestCase {
26+
public void testLuceneQuery_AllLiterals_NonTranslatable() {
27+
EndsWith function = new EndsWith(
28+
Source.EMPTY,
29+
new Literal(Source.EMPTY, "test", DataType.KEYWORD),
30+
new Literal(Source.EMPTY, "test", DataType.KEYWORD)
31+
);
32+
33+
ESTestCase.assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false));
34+
}
35+
36+
public void testLuceneQuery_NonFoldableSuffix_NonTranslatable() {
37+
EndsWith function = new EndsWith(
38+
Source.EMPTY,
39+
new FieldAttribute(Source.EMPTY, "field", new EsField("field", DataType.KEYWORD, Map.of(), true)),
40+
new FieldAttribute(Source.EMPTY, "field", new EsField("suffix", DataType.KEYWORD, Map.of(), true))
41+
);
42+
43+
assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false));
44+
}
45+
46+
public void testLuceneQuery_NonFoldableSuffix_Translatable() {
47+
EndsWith function = new EndsWith(
48+
Source.EMPTY,
49+
new FieldAttribute(Source.EMPTY, "field", new EsField("suffix", DataType.KEYWORD, Map.of(), true)),
50+
new Literal(Source.EMPTY, "a*b?c\\", DataType.KEYWORD)
51+
);
52+
53+
assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(true));
54+
55+
Query query = function.asQuery(TranslatorHandler.TRANSLATOR_HANDLER);
56+
57+
assertThat(query, equalTo(new WildcardQuery(Source.EMPTY, "field", "*a\\*b\\?c\\\\", false)));
58+
}
59+
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWithTests.java

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,14 @@
1212

1313
import org.apache.lucene.util.BytesRef;
1414
import org.elasticsearch.xpack.esql.core.expression.Expression;
15-
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
16-
import org.elasticsearch.xpack.esql.core.expression.Literal;
17-
import org.elasticsearch.xpack.esql.core.querydsl.query.WildcardQuery;
1815
import org.elasticsearch.xpack.esql.core.tree.Source;
1916
import org.elasticsearch.xpack.esql.core.type.DataType;
20-
import org.elasticsearch.xpack.esql.core.type.EsField;
2117
import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase;
2218
import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
23-
import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates;
24-
import org.elasticsearch.xpack.esql.planner.TranslatorHandler;
2519
import org.hamcrest.Matcher;
2620

2721
import java.util.LinkedList;
2822
import java.util.List;
29-
import java.util.Map;
3023
import java.util.function.Supplier;
3124

3225
import static org.hamcrest.Matchers.equalTo;
@@ -105,38 +98,4 @@ private static TestCaseSupplier.TestCase testCase(
10598
protected Expression build(Source source, List<Expression> args) {
10699
return new EndsWith(source, args.get(0), args.get(1));
107100
}
108-
109-
public void testLuceneQuery_AllLiterals_NonTranslatable() {
110-
var function = new EndsWith(
111-
Source.EMPTY,
112-
new Literal(Source.EMPTY, "test", DataType.KEYWORD),
113-
new Literal(Source.EMPTY, "test", DataType.KEYWORD)
114-
);
115-
116-
assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false));
117-
}
118-
119-
public void testLuceneQuery_NonFoldableSuffix_NonTranslatable() {
120-
var function = new EndsWith(
121-
Source.EMPTY,
122-
new FieldAttribute(Source.EMPTY, "field", new EsField("field", DataType.KEYWORD, Map.of(), true)),
123-
new FieldAttribute(Source.EMPTY, "field", new EsField("suffix", DataType.KEYWORD, Map.of(), true))
124-
);
125-
126-
assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false));
127-
}
128-
129-
public void testLuceneQuery_NonFoldableSuffix_Translatable() {
130-
var function = new EndsWith(
131-
Source.EMPTY,
132-
new FieldAttribute(Source.EMPTY, "field", new EsField("suffix", DataType.KEYWORD, Map.of(), true)),
133-
new Literal(Source.EMPTY, "a*b?c\\", DataType.KEYWORD)
134-
);
135-
136-
assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(true));
137-
138-
var query = function.asQuery(TranslatorHandler.TRANSLATOR_HANDLER);
139-
140-
assertThat(query, equalTo(new WildcardQuery(Source.EMPTY, "field", "*a\\*b\\?c\\\\")));
141-
}
142101
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.esql.expression.function.scalar.string;
9+
10+
import org.elasticsearch.test.ESTestCase;
11+
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
12+
import org.elasticsearch.xpack.esql.core.expression.Literal;
13+
import org.elasticsearch.xpack.esql.core.querydsl.query.WildcardQuery;
14+
import org.elasticsearch.xpack.esql.core.tree.Source;
15+
import org.elasticsearch.xpack.esql.core.type.DataType;
16+
import org.elasticsearch.xpack.esql.core.type.EsField;
17+
import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates;
18+
import org.elasticsearch.xpack.esql.planner.TranslatorHandler;
19+
20+
import java.util.Map;
21+
22+
import static org.hamcrest.Matchers.equalTo;
23+
24+
public class StartsWithStaticTests extends ESTestCase {
25+
26+
public void testLuceneQuery_AllLiterals_NonTranslatable() {
27+
var function = new StartsWith(
28+
Source.EMPTY,
29+
new Literal(Source.EMPTY, "test", DataType.KEYWORD),
30+
new Literal(Source.EMPTY, "test", DataType.KEYWORD)
31+
);
32+
33+
assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false));
34+
}
35+
36+
public void testLuceneQuery_NonFoldablePrefix_NonTranslatable() {
37+
var function = new StartsWith(
38+
Source.EMPTY,
39+
new FieldAttribute(Source.EMPTY, "field", new EsField("field", DataType.KEYWORD, Map.of(), true)),
40+
new FieldAttribute(Source.EMPTY, "field", new EsField("prefix", DataType.KEYWORD, Map.of(), true))
41+
);
42+
43+
assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false));
44+
}
45+
46+
public void testLuceneQuery_NonFoldablePrefix_Translatable() {
47+
var function = new StartsWith(
48+
Source.EMPTY,
49+
new FieldAttribute(Source.EMPTY, "field", new EsField("prefix", DataType.KEYWORD, Map.of(), true)),
50+
new Literal(Source.EMPTY, "a*b?c\\", DataType.KEYWORD)
51+
);
52+
53+
assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(true));
54+
55+
var query = function.asQuery(TranslatorHandler.TRANSLATOR_HANDLER);
56+
57+
assertThat(query, equalTo(new WildcardQuery(Source.EMPTY, "field", "a\\*b\\?c\\\\*", false)));
58+
}
59+
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWithTests.java

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,13 @@
1212

1313
import org.apache.lucene.util.BytesRef;
1414
import org.elasticsearch.xpack.esql.core.expression.Expression;
15-
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
16-
import org.elasticsearch.xpack.esql.core.expression.Literal;
17-
import org.elasticsearch.xpack.esql.core.querydsl.query.WildcardQuery;
1815
import org.elasticsearch.xpack.esql.core.tree.Source;
1916
import org.elasticsearch.xpack.esql.core.type.DataType;
20-
import org.elasticsearch.xpack.esql.core.type.EsField;
2117
import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase;
2218
import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
23-
import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates;
24-
import org.elasticsearch.xpack.esql.planner.TranslatorHandler;
2519

2620
import java.util.ArrayList;
2721
import java.util.List;
28-
import java.util.Map;
2922
import java.util.function.Supplier;
3023

3124
import static org.hamcrest.Matchers.equalTo;
@@ -65,38 +58,4 @@ public static Iterable<Object[]> parameters() {
6558
protected Expression build(Source source, List<Expression> args) {
6659
return new StartsWith(source, args.get(0), args.get(1));
6760
}
68-
69-
public void testLuceneQuery_AllLiterals_NonTranslatable() {
70-
var function = new StartsWith(
71-
Source.EMPTY,
72-
new Literal(Source.EMPTY, "test", DataType.KEYWORD),
73-
new Literal(Source.EMPTY, "test", DataType.KEYWORD)
74-
);
75-
76-
assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false));
77-
}
78-
79-
public void testLuceneQuery_NonFoldablePrefix_NonTranslatable() {
80-
var function = new StartsWith(
81-
Source.EMPTY,
82-
new FieldAttribute(Source.EMPTY, "field", new EsField("field", DataType.KEYWORD, Map.of(), true)),
83-
new FieldAttribute(Source.EMPTY, "field", new EsField("prefix", DataType.KEYWORD, Map.of(), true))
84-
);
85-
86-
assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(false));
87-
}
88-
89-
public void testLuceneQuery_NonFoldablePrefix_Translatable() {
90-
var function = new StartsWith(
91-
Source.EMPTY,
92-
new FieldAttribute(Source.EMPTY, "field", new EsField("prefix", DataType.KEYWORD, Map.of(), true)),
93-
new Literal(Source.EMPTY, "a*b?c\\", DataType.KEYWORD)
94-
);
95-
96-
assertThat(function.translatable(LucenePushdownPredicates.DEFAULT), equalTo(true));
97-
98-
var query = function.asQuery(TranslatorHandler.TRANSLATOR_HANDLER);
99-
100-
assertThat(query, equalTo(new WildcardQuery(Source.EMPTY, "field", "a\\*b\\?c\\\\*")));
101-
}
10261
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.esql.expression.predicate.operator.comparison;
9+
10+
import org.elasticsearch.test.ESTestCase;
11+
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
12+
import org.elasticsearch.xpack.esql.core.expression.Literal;
13+
14+
import java.util.Arrays;
15+
16+
import static org.elasticsearch.xpack.esql.EsqlTestUtils.L;
17+
import static org.elasticsearch.xpack.esql.core.expression.Literal.NULL;
18+
import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY;
19+
20+
public class InStaticTests extends ESTestCase {
21+
private static final Literal ONE = L(1);
22+
private static final Literal TWO = L(2);
23+
private static final Literal THREE = L(3);
24+
25+
public void testInWithContainedValue() {
26+
In in = new In(EMPTY, TWO, Arrays.asList(ONE, TWO, THREE));
27+
assertTrue((Boolean) in.fold(FoldContext.small()));
28+
}
29+
30+
public void testInWithNotContainedValue() {
31+
In in = new In(EMPTY, THREE, Arrays.asList(ONE, TWO));
32+
assertFalse((Boolean) in.fold(FoldContext.small()));
33+
}
34+
35+
public void testHandleNullOnLeftValue() {
36+
In in = new In(EMPTY, NULL, Arrays.asList(ONE, TWO, THREE));
37+
assertNull(in.fold(FoldContext.small()));
38+
in = new In(EMPTY, NULL, Arrays.asList(ONE, NULL, THREE));
39+
assertNull(in.fold(FoldContext.small()));
40+
41+
}
42+
43+
public void testHandleNullsOnRightValue() {
44+
In in = new In(EMPTY, THREE, Arrays.asList(ONE, NULL, THREE));
45+
assertTrue((Boolean) in.fold(FoldContext.small()));
46+
in = new In(EMPTY, ONE, Arrays.asList(TWO, NULL, THREE));
47+
assertNull(in.fold(FoldContext.small()));
48+
}
49+
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -13,23 +13,17 @@
1313
import org.elasticsearch.geo.GeometryTestUtils;
1414
import org.elasticsearch.geo.ShapeTestUtils;
1515
import org.elasticsearch.xpack.esql.core.expression.Expression;
16-
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
17-
import org.elasticsearch.xpack.esql.core.expression.Literal;
1816
import org.elasticsearch.xpack.esql.core.tree.Source;
1917
import org.elasticsearch.xpack.esql.core.type.DataType;
2018
import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase;
2119
import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
2220
import org.junit.AfterClass;
2321

2422
import java.util.ArrayList;
25-
import java.util.Arrays;
2623
import java.util.List;
2724
import java.util.function.Supplier;
2825

29-
import static org.elasticsearch.xpack.esql.EsqlTestUtils.of;
3026
import static org.elasticsearch.xpack.esql.EsqlTestUtils.randomLiteral;
31-
import static org.elasticsearch.xpack.esql.core.expression.Literal.NULL;
32-
import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY;
3327
import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_POINT;
3428
import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_SHAPE;
3529
import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_POINT;
@@ -45,39 +39,6 @@ public InTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseSup
4539
this.testCase = testCaseSupplier.get();
4640
}
4741

48-
private static final Literal ONE = L(1);
49-
private static final Literal TWO = L(2);
50-
private static final Literal THREE = L(3);
51-
52-
public void testInWithContainedValue() {
53-
In in = new In(EMPTY, TWO, Arrays.asList(ONE, TWO, THREE));
54-
assertTrue((Boolean) in.fold(FoldContext.small()));
55-
}
56-
57-
public void testInWithNotContainedValue() {
58-
In in = new In(EMPTY, THREE, Arrays.asList(ONE, TWO));
59-
assertFalse((Boolean) in.fold(FoldContext.small()));
60-
}
61-
62-
public void testHandleNullOnLeftValue() {
63-
In in = new In(EMPTY, NULL, Arrays.asList(ONE, TWO, THREE));
64-
assertNull(in.fold(FoldContext.small()));
65-
in = new In(EMPTY, NULL, Arrays.asList(ONE, NULL, THREE));
66-
assertNull(in.fold(FoldContext.small()));
67-
68-
}
69-
70-
public void testHandleNullsOnRightValue() {
71-
In in = new In(EMPTY, THREE, Arrays.asList(ONE, NULL, THREE));
72-
assertTrue((Boolean) in.fold(FoldContext.small()));
73-
in = new In(EMPTY, ONE, Arrays.asList(TWO, NULL, THREE));
74-
assertNull(in.fold(FoldContext.small()));
75-
}
76-
77-
private static Literal L(Object value) {
78-
return of(EMPTY, value);
79-
}
80-
8142
@ParametersFactory
8243
public static Iterable<Object[]> parameters() {
8344
List<TestCaseSupplier> suppliers = new ArrayList<>();

0 commit comments

Comments
 (0)