Skip to content

Commit d48c119

Browse files
ivanceasmalyshev
ivancea
authored and
smalyshev
committed
Fix AvgTests error on -0.0 avg (elastic#113272)
Fixes elastic#113225 Fixes elastic#114175
1 parent 609b90e commit d48c119

File tree

6 files changed

+276
-360
lines changed

6 files changed

+276
-360
lines changed

‎muted-tests.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,6 @@ tests:
164164
- class: org.elasticsearch.packaging.test.WindowsServiceTests
165165
method: test80JavaOptsInEnvVar
166166
issue: https://github.com/elastic/elasticsearch/issues/113219
167-
- class: org.elasticsearch.xpack.esql.expression.function.aggregate.AvgTests
168-
method: "testFold {TestCase=<double> #2}"
169-
issue: https://github.com/elastic/elasticsearch/issues/113225
170167
- class: org.elasticsearch.packaging.test.WindowsServiceTests
171168
method: test81JavaOptsInJvmOptions
172169
issue: https://github.com/elastic/elasticsearch/issues/113313
@@ -236,9 +233,6 @@ tests:
236233
- class: org.elasticsearch.xpack.inference.InferenceCrudIT
237234
method: testGet
238235
issue: https://github.com/elastic/elasticsearch/issues/114135
239-
- class: org.elasticsearch.xpack.esql.expression.function.aggregate.AvgTests
240-
method: "testFold {TestCase=<double> #7}"
241-
issue: https://github.com/elastic/elasticsearch/issues/114175
242236
- class: org.elasticsearch.xpack.ilm.ExplainLifecycleIT
243237
method: testStepInfoPreservedOnAutoRetry
244238
issue: https://github.com/elastic/elasticsearch/issues/114220

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

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -163,15 +163,7 @@ private void aggregateSingleMode(Expression expression) {
163163
result = extractResultFromAggregator(aggregator, PlannerUtils.toElementType(testCase.expectedType()));
164164
}
165165

166-
assertThat(result, not(equalTo(Double.NaN)));
167-
assert testCase.getMatcher().matches(Double.POSITIVE_INFINITY) == false;
168-
assertThat(result, not(equalTo(Double.POSITIVE_INFINITY)));
169-
assert testCase.getMatcher().matches(Double.NEGATIVE_INFINITY) == false;
170-
assertThat(result, not(equalTo(Double.NEGATIVE_INFINITY)));
171-
assertThat(result, testCase.getMatcher());
172-
if (testCase.getExpectedWarnings() != null) {
173-
assertWarnings(testCase.getExpectedWarnings());
174-
}
166+
assertTestCaseResultAndWarnings(result);
175167
}
176168

177169
private void aggregateGroupingSingleMode(Expression expression) {
@@ -263,15 +255,7 @@ private void aggregateWithIntermediates(Expression expression) {
263255
result = extractResultFromAggregator(aggregator, PlannerUtils.toElementType(testCase.expectedType()));
264256
}
265257

266-
assertThat(result, not(equalTo(Double.NaN)));
267-
assert testCase.getMatcher().matches(Double.POSITIVE_INFINITY) == false;
268-
assertThat(result, not(equalTo(Double.POSITIVE_INFINITY)));
269-
assert testCase.getMatcher().matches(Double.NEGATIVE_INFINITY) == false;
270-
assertThat(result, not(equalTo(Double.NEGATIVE_INFINITY)));
271-
assertThat(result, testCase.getMatcher());
272-
if (testCase.getExpectedWarnings() != null) {
273-
assertWarnings(testCase.getExpectedWarnings());
274-
}
258+
assertTestCaseResultAndWarnings(result);
275259
}
276260

277261
private void evaluate(Expression evaluableExpression) {
@@ -288,15 +272,7 @@ private void evaluate(Expression evaluableExpression) {
288272
if (testCase.expectedType() == DataType.UNSIGNED_LONG && result != null) {
289273
result = NumericUtils.unsignedLongAsBigInteger((Long) result);
290274
}
291-
assertThat(result, not(equalTo(Double.NaN)));
292-
assert testCase.getMatcher().matches(Double.POSITIVE_INFINITY) == false;
293-
assertThat(result, not(equalTo(Double.POSITIVE_INFINITY)));
294-
assert testCase.getMatcher().matches(Double.NEGATIVE_INFINITY) == false;
295-
assertThat(result, not(equalTo(Double.NEGATIVE_INFINITY)));
296-
assertThat(result, testCase.getMatcher());
297-
if (testCase.getExpectedWarnings() != null) {
298-
assertWarnings(testCase.getExpectedWarnings());
299-
}
275+
assertTestCaseResultAndWarnings(result);
300276
}
301277

302278
private void resolveExpression(Expression expression, Consumer<Expression> onAggregator, Consumer<Expression> onEvaluableExpression) {

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,10 @@
9999
import static org.hamcrest.Matchers.either;
100100
import static org.hamcrest.Matchers.endsWith;
101101
import static org.hamcrest.Matchers.equalTo;
102+
import static org.hamcrest.Matchers.hasItem;
102103
import static org.hamcrest.Matchers.hasSize;
103104
import static org.hamcrest.Matchers.instanceOf;
105+
import static org.hamcrest.Matchers.not;
104106
import static org.hamcrest.Matchers.nullValue;
105107

106108
/**
@@ -754,6 +756,34 @@ public static void testFunctionInfo() {
754756
assertEquals(returnFromSignature, returnTypes);
755757
}
756758

759+
/**
760+
* Asserts the result of a test case matches the expected result and warnings.
761+
* <p>
762+
* The {@code result} parameter should be an object as returned by {@link #toJavaObjectUnsignedLongAware}.
763+
* </p>
764+
*/
765+
@SuppressWarnings("unchecked")
766+
protected final void assertTestCaseResultAndWarnings(Object result) {
767+
if (result instanceof Iterable<?>) {
768+
var collectionResult = (Iterable<Object>) result;
769+
assertThat(collectionResult, not(hasItem(Double.NaN)));
770+
assertThat(collectionResult, not(hasItem(Double.POSITIVE_INFINITY)));
771+
assertThat(collectionResult, not(hasItem(Double.NEGATIVE_INFINITY)));
772+
}
773+
774+
assert testCase.getMatcher().matches(Double.NaN) == false;
775+
assertThat(result, not(equalTo(Double.NaN)));
776+
assert testCase.getMatcher().matches(Double.POSITIVE_INFINITY) == false;
777+
assertThat(result, not(equalTo(Double.POSITIVE_INFINITY)));
778+
assert testCase.getMatcher().matches(Double.NEGATIVE_INFINITY) == false;
779+
assertThat(result, not(equalTo(Double.NEGATIVE_INFINITY)));
780+
assertThat(result, testCase.getMatcher());
781+
782+
if (testCase.getExpectedWarnings() != null) {
783+
assertWarnings(testCase.getExpectedWarnings());
784+
}
785+
}
786+
757787
protected final void assertTypeResolutionFailure(Expression expression) {
758788
assertTrue("expected unresolved", expression.typeResolved().unresolved());
759789
assertThat(expression.typeResolved().message(), equalTo(testCase.getExpectedTypeError()));

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

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import static org.hamcrest.Matchers.either;
4242
import static org.hamcrest.Matchers.equalTo;
4343
import static org.hamcrest.Matchers.is;
44-
import static org.hamcrest.Matchers.not;
4544
import static org.hamcrest.Matchers.nullValue;
4645
import static org.hamcrest.Matchers.sameInstance;
4746

@@ -127,15 +126,7 @@ public final void testEvaluate() {
127126
result = toJavaObjectUnsignedLongAware(block, 0);
128127
}
129128
}
130-
assertThat(result, not(equalTo(Double.NaN)));
131-
assert testCase.getMatcher().matches(Double.POSITIVE_INFINITY) == false;
132-
assertThat(result, not(equalTo(Double.POSITIVE_INFINITY)));
133-
assert testCase.getMatcher().matches(Double.NEGATIVE_INFINITY) == false;
134-
assertThat(result, not(equalTo(Double.NEGATIVE_INFINITY)));
135-
assertThat(result, testCase.getMatcher());
136-
if (testCase.getExpectedWarnings() != null) {
137-
assertWarnings(testCase.getExpectedWarnings());
138-
}
129+
assertTestCaseResultAndWarnings(result);
139130
}
140131

141132
/**

0 commit comments

Comments
 (0)