Skip to content

Commit 1224db9

Browse files
not-napoleonnot-napoleon
authored
[ESQL] clean up date trunc tests (#116111) (#116179)
While working on #110008 I discovered that the Date Trunc tests were only running in folding mode, because the interval types are marked as not representable. The correct way to test this is to set the forceLiteral flag for those fields, which will (as the name suggests) force them to be literals even in non-folding tests. Doing that turned up errors in the evaluatorToString tests, which I fixed. There are two big changes here. First, the second parameter to the evaluator is a Rounding instance, not the actual interval. Since Rounding includes some information about the specific rounding in the toString results, I am just using a starts with matcher to validate the majority of the string, rather than trying to reconstruct the expected rounding string. Second, passing in a literal null for the interval parameter folds the whole expression to null, and thus a completely different toString. I added a clause in AnyNullIsNull to account for this. While I was in there, I moved some specific test cases to a different file. I know moving code is something we're trying to minimize right now, but this seemed worth it. The tests in question do not depend on the parameters of the test case, but all methods in the class get run for every set of parameters. This was causing these tests to be run many times with the same values, which bloats our test run time and test count. Moving them to a distinct class means they'll only be executed once per test run. I feel like this benefit outweighs the cost of git history complexity.
1 parent 323401d commit 1224db9

File tree

5 files changed

+123
-91
lines changed

5 files changed

+123
-91
lines changed

‎docs/reference/esql/functions/kibana/definition/date_diff.json‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/reference/esql/functions/signature/match.svg‎

Lines changed: 1 addition & 1 deletion
Loading

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
import org.elasticsearch.xpack.esql.planner.PlannerUtils;
6868
import org.elasticsearch.xpack.esql.session.Configuration;
6969
import org.hamcrest.Matcher;
70+
import org.hamcrest.Matchers;
7071
import org.junit.After;
7172
import org.junit.AfterClass;
7273

@@ -170,7 +171,7 @@ protected static List<TestCaseSupplier> anyNullIsNull(boolean entirelyNullPreser
170171
(nullPosition, nullValueDataType, original) -> entirelyNullPreservesType == false
171172
&& nullValueDataType == DataType.NULL
172173
&& original.getData().size() == 1 ? DataType.NULL : original.expectedType(),
173-
(nullPosition, nullData, original) -> original
174+
(nullPosition, nullData, original) -> nullData.isForceLiteral() ? Matchers.equalTo("LiteralsEvaluator[lit=null]") : original
174175
);
175176
}
176177

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateTruncRoundingTests.java‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
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.date;
9+
10+
import org.elasticsearch.common.Rounding;
11+
import org.elasticsearch.test.ESTestCase;
12+
13+
import java.time.Duration;
14+
import java.time.Instant;
15+
import java.time.Period;
16+
17+
import static org.elasticsearch.xpack.esql.expression.function.scalar.date.DateTrunc.createRounding;
18+
import static org.elasticsearch.xpack.esql.expression.function.scalar.date.DateTrunc.process;
19+
import static org.hamcrest.Matchers.containsString;
20+
21+
/**
22+
* This class supplements {@link DateTruncTests}. The tests in this class are not run via the parametrized runner,
23+
* and exercise specific helper functions within the class.
24+
*/
25+
public class DateTruncRoundingTests extends ESTestCase {
26+
27+
public void testCreateRoundingDuration() {
28+
Rounding.Prepared rounding;
29+
30+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> createRounding(Duration.ofHours(0)));
31+
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
32+
33+
e = expectThrows(IllegalArgumentException.class, () -> createRounding(Duration.ofHours(-10)));
34+
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
35+
36+
rounding = createRounding(Duration.ofHours(1));
37+
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.HOUR_OF_DAY), 0d);
38+
39+
rounding = createRounding(Duration.ofHours(10));
40+
assertEquals(10, rounding.roundingSize(Rounding.DateTimeUnit.HOUR_OF_DAY), 0d);
41+
42+
rounding = createRounding(Duration.ofMinutes(1));
43+
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.MINUTES_OF_HOUR), 0d);
44+
45+
rounding = createRounding(Duration.ofMinutes(100));
46+
assertEquals(100, rounding.roundingSize(Rounding.DateTimeUnit.MINUTES_OF_HOUR), 0d);
47+
48+
rounding = createRounding(Duration.ofSeconds(1));
49+
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.SECOND_OF_MINUTE), 0d);
50+
51+
rounding = createRounding(Duration.ofSeconds(120));
52+
assertEquals(120, rounding.roundingSize(Rounding.DateTimeUnit.SECOND_OF_MINUTE), 0d);
53+
54+
rounding = createRounding(Duration.ofSeconds(60).plusMinutes(5).plusHours(1));
55+
assertEquals(1 + 5 + 60, rounding.roundingSize(Rounding.DateTimeUnit.MINUTES_OF_HOUR), 0d);
56+
}
57+
58+
public void testCreateRoundingPeriod() {
59+
Rounding.Prepared rounding;
60+
61+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> createRounding(Period.ofMonths(0)));
62+
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
63+
64+
e = expectThrows(IllegalArgumentException.class, () -> createRounding(Period.ofYears(-10)));
65+
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
66+
67+
e = expectThrows(IllegalArgumentException.class, () -> createRounding(Period.of(0, 1, 1)));
68+
assertThat(e.getMessage(), containsString("Time interval with multiple periods is not supported"));
69+
70+
rounding = createRounding(Period.ofDays(1));
71+
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.DAY_OF_MONTH), 0d);
72+
73+
rounding = createRounding(Period.ofDays(4));
74+
assertEquals(4, rounding.roundingSize(Rounding.DateTimeUnit.DAY_OF_MONTH), 0d);
75+
76+
rounding = createRounding(Period.ofDays(7));
77+
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.WEEK_OF_WEEKYEAR), 0d);
78+
79+
rounding = createRounding(Period.ofMonths(1));
80+
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.MONTH_OF_YEAR), 0d);
81+
82+
rounding = createRounding(Period.ofMonths(3));
83+
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.QUARTER_OF_YEAR), 0d);
84+
85+
rounding = createRounding(Period.ofYears(1));
86+
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.YEAR_OF_CENTURY), 0d);
87+
88+
e = expectThrows(IllegalArgumentException.class, () -> createRounding(Period.ofYears(3)));
89+
assertThat(e.getMessage(), containsString("Time interval is not supported"));
90+
}
91+
92+
public void testCreateRoundingNullInterval() {
93+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> createRounding(null));
94+
assertThat(e.getMessage(), containsString("Time interval is not supported"));
95+
}
96+
97+
public void testDateTruncFunction() {
98+
long ts = toMillis("2023-02-17T10:25:33.38Z");
99+
100+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> process(ts, createRounding(Period.ofDays(-1))));
101+
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
102+
103+
e = expectThrows(IllegalArgumentException.class, () -> process(ts, createRounding(Duration.ofHours(-1))));
104+
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
105+
}
106+
107+
private static long toMillis(String timestamp) {
108+
return Instant.parse(timestamp).toEpochMilli();
109+
}
110+
111+
}

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateTruncTests.java‎

Lines changed: 8 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,24 @@
1010
import com.carrotsearch.randomizedtesting.annotations.Name;
1111
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
1212

13-
import org.elasticsearch.common.Rounding;
1413
import org.elasticsearch.xpack.esql.core.expression.Expression;
1514
import org.elasticsearch.xpack.esql.core.tree.Source;
1615
import org.elasticsearch.xpack.esql.core.type.DataType;
1716
import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase;
1817
import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
18+
import org.hamcrest.Matchers;
1919

2020
import java.time.Duration;
2121
import java.time.Instant;
2222
import java.time.Period;
2323
import java.util.List;
2424
import java.util.function.Supplier;
2525

26-
import static org.elasticsearch.xpack.esql.expression.function.scalar.date.DateTrunc.createRounding;
27-
import static org.elasticsearch.xpack.esql.expression.function.scalar.date.DateTrunc.process;
28-
import static org.hamcrest.Matchers.containsString;
2926
import static org.hamcrest.Matchers.equalTo;
3027

28+
/**
29+
* Parameterized testing for {@link DateTrunc}. See also {@link DateTruncRoundingTests} for non-parametrized tests.
30+
*/
3131
public class DateTruncTests extends AbstractScalarFunctionTestCase {
3232

3333
public DateTruncTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseSupplier) {
@@ -61,95 +61,15 @@ public static Iterable<Object[]> parameters() {
6161
});
6262
}
6363

64-
public void testCreateRoundingDuration() {
65-
Rounding.Prepared rounding;
66-
67-
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> createRounding(Duration.ofHours(0)));
68-
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
69-
70-
e = expectThrows(IllegalArgumentException.class, () -> createRounding(Duration.ofHours(-10)));
71-
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
72-
73-
rounding = createRounding(Duration.ofHours(1));
74-
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.HOUR_OF_DAY), 0d);
75-
76-
rounding = createRounding(Duration.ofHours(10));
77-
assertEquals(10, rounding.roundingSize(Rounding.DateTimeUnit.HOUR_OF_DAY), 0d);
78-
79-
rounding = createRounding(Duration.ofMinutes(1));
80-
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.MINUTES_OF_HOUR), 0d);
81-
82-
rounding = createRounding(Duration.ofMinutes(100));
83-
assertEquals(100, rounding.roundingSize(Rounding.DateTimeUnit.MINUTES_OF_HOUR), 0d);
84-
85-
rounding = createRounding(Duration.ofSeconds(1));
86-
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.SECOND_OF_MINUTE), 0d);
87-
88-
rounding = createRounding(Duration.ofSeconds(120));
89-
assertEquals(120, rounding.roundingSize(Rounding.DateTimeUnit.SECOND_OF_MINUTE), 0d);
90-
91-
rounding = createRounding(Duration.ofSeconds(60).plusMinutes(5).plusHours(1));
92-
assertEquals(1 + 5 + 60, rounding.roundingSize(Rounding.DateTimeUnit.MINUTES_OF_HOUR), 0d);
93-
}
94-
95-
public void testCreateRoundingPeriod() {
96-
Rounding.Prepared rounding;
97-
98-
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> createRounding(Period.ofMonths(0)));
99-
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
100-
101-
e = expectThrows(IllegalArgumentException.class, () -> createRounding(Period.ofYears(-10)));
102-
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
103-
104-
e = expectThrows(IllegalArgumentException.class, () -> createRounding(Period.of(0, 1, 1)));
105-
assertThat(e.getMessage(), containsString("Time interval with multiple periods is not supported"));
106-
107-
rounding = createRounding(Period.ofDays(1));
108-
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.DAY_OF_MONTH), 0d);
109-
110-
rounding = createRounding(Period.ofDays(4));
111-
assertEquals(4, rounding.roundingSize(Rounding.DateTimeUnit.DAY_OF_MONTH), 0d);
112-
113-
rounding = createRounding(Period.ofDays(7));
114-
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.WEEK_OF_WEEKYEAR), 0d);
115-
116-
rounding = createRounding(Period.ofMonths(1));
117-
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.MONTH_OF_YEAR), 0d);
118-
119-
rounding = createRounding(Period.ofMonths(3));
120-
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.QUARTER_OF_YEAR), 0d);
121-
122-
rounding = createRounding(Period.ofYears(1));
123-
assertEquals(1, rounding.roundingSize(Rounding.DateTimeUnit.YEAR_OF_CENTURY), 0d);
124-
125-
e = expectThrows(IllegalArgumentException.class, () -> createRounding(Period.ofYears(3)));
126-
assertThat(e.getMessage(), containsString("Time interval is not supported"));
127-
}
128-
129-
public void testCreateRoundingNullInterval() {
130-
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> createRounding(null));
131-
assertThat(e.getMessage(), containsString("Time interval is not supported"));
132-
}
133-
134-
public void testDateTruncFunction() {
135-
long ts = toMillis("2023-02-17T10:25:33.38Z");
136-
137-
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> process(ts, createRounding(Period.ofDays(-1))));
138-
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
139-
140-
e = expectThrows(IllegalArgumentException.class, () -> process(ts, createRounding(Duration.ofHours(-1))));
141-
assertThat(e.getMessage(), containsString("Zero or negative time interval is not supported"));
142-
}
143-
14464
private static TestCaseSupplier ofDatePeriod(Period period, long value, String expectedDate) {
14565
return new TestCaseSupplier(
14666
List.of(DataType.DATE_PERIOD, DataType.DATETIME),
14767
() -> new TestCaseSupplier.TestCase(
14868
List.of(
149-
new TestCaseSupplier.TypedData(period, DataType.DATE_PERIOD, "interval"),
69+
new TestCaseSupplier.TypedData(period, DataType.DATE_PERIOD, "interval").forceLiteral(),
15070
new TestCaseSupplier.TypedData(value, DataType.DATETIME, "date")
15171
),
152-
"DateTruncEvaluator[date=Attribute[channel=1], interval=Attribute[channel=0]]",
72+
Matchers.startsWith("DateTruncEvaluator[fieldVal=Attribute[channel=0], rounding=Rounding["),
15373
DataType.DATETIME,
15474
equalTo(toMillis(expectedDate))
15575
)
@@ -161,10 +81,10 @@ private static TestCaseSupplier ofDuration(Duration duration, long value, String
16181
List.of(DataType.TIME_DURATION, DataType.DATETIME),
16282
() -> new TestCaseSupplier.TestCase(
16383
List.of(
164-
new TestCaseSupplier.TypedData(duration, DataType.TIME_DURATION, "interval"),
84+
new TestCaseSupplier.TypedData(duration, DataType.TIME_DURATION, "interval").forceLiteral(),
16585
new TestCaseSupplier.TypedData(value, DataType.DATETIME, "date")
16686
),
167-
"DateTruncEvaluator[date=Attribute[channel=1], interval=Attribute[channel=0]]",
87+
Matchers.startsWith("DateTruncEvaluator[fieldVal=Attribute[channel=0], rounding=Rounding["),
16888
DataType.DATETIME,
16989
equalTo(toMillis(expectedDate))
17090
)

0 commit comments

Comments
 (0)