Skip to content

Commit 5e3174b

Browse files
[8.19] Fixes flaky ST_CENTROID_AGG tests (#114892) (#128037) (#128039)
* Fixes flaky ST_CENTROID_AGG tests (#114892) Even with Kahan summation, we were occasionally getting floating point differences at the 14th decimal point, well beyond anything a GIS use case would care about. * Unmute after cherry-pick
1 parent 26af178 commit 5e3174b

File tree

2 files changed

+50
-12
lines changed

2 files changed

+50
-12
lines changed

muted-tests.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,6 @@ tests:
6161
- class: org.elasticsearch.xpack.ml.integration.MlJobIT
6262
method: testMultiIndexDelete
6363
issue: https://github.com/elastic/elasticsearch/issues/112381
64-
- class: org.elasticsearch.xpack.esql.expression.function.aggregate.SpatialCentroidTests
65-
method: "testAggregateIntermediate {TestCase=<geo_point> #2}"
66-
issue: https://github.com/elastic/elasticsearch/issues/112461
67-
- class: org.elasticsearch.xpack.esql.expression.function.aggregate.SpatialCentroidTests
68-
method: testAggregateIntermediate {TestCase=<geo_point>}
69-
issue: https://github.com/elastic/elasticsearch/issues/112463
7064
- class: org.elasticsearch.xpack.inference.external.http.RequestBasedTaskRunnerTests
7165
method: testLoopOneAtATime
7266
issue: https://github.com/elastic/elasticsearch/issues/112471

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/SpatialCentroidTests.java

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@
2222
import org.elasticsearch.xpack.esql.expression.function.FunctionName;
2323
import org.elasticsearch.xpack.esql.expression.function.MultiRowTestCaseSupplier;
2424
import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
25+
import org.hamcrest.BaseMatcher;
26+
import org.hamcrest.Description;
27+
import org.hamcrest.Matcher;
2528

26-
import java.nio.ByteOrder;
2729
import java.util.List;
2830
import java.util.function.Supplier;
2931
import java.util.stream.Stream;
3032

31-
import static org.hamcrest.Matchers.equalTo;
33+
import static org.hamcrest.Matchers.closeTo;
3234

3335
@FunctionName("st_centroid_agg")
3436
public class SpatialCentroidTests extends AbstractAggregationTestCase {
@@ -74,16 +76,58 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
7476
count++;
7577
}
7678

77-
var expected = new BytesRef(
78-
WellKnownBinary.toWKB(new Point(xSum.value() / count, ySum.value() / count), ByteOrder.LITTLE_ENDIAN)
79-
);
79+
var expectedX = xSum.value() / count;
80+
var expectedY = ySum.value() / count;
8081

8182
return new TestCaseSupplier.TestCase(
8283
List.of(fieldTypedData),
8384
"SpatialCentroid[field=Attribute[channel=0]]",
8485
fieldTypedData.type(),
85-
equalTo(expected)
86+
centroidMatches(expectedX, expectedY, 1e-14)
8687
);
8788
});
8889
}
90+
91+
@SuppressWarnings("SameParameterValue")
92+
private static Matcher<BytesRef> centroidMatches(double x, double y, double error) {
93+
return new TestCentroidMatcher(x, y, error);
94+
}
95+
96+
private static class TestCentroidMatcher extends BaseMatcher<BytesRef> {
97+
private final double x;
98+
private final double y;
99+
private final Matcher<Double> mx;
100+
private final Matcher<Double> my;
101+
102+
private TestCentroidMatcher(double x, double y, double error) {
103+
this.x = x;
104+
this.y = y;
105+
this.mx = closeTo(x, error);
106+
this.my = closeTo(y, error);
107+
}
108+
109+
@Override
110+
public boolean matches(Object item) {
111+
if (item instanceof BytesRef wkb) {
112+
var point = (Point) WellKnownBinary.fromWKB(GeometryValidator.NOOP, false, wkb.bytes, wkb.offset, wkb.length);
113+
return mx.matches(point.getX()) && my.matches(point.getY());
114+
}
115+
return false;
116+
}
117+
118+
@Override
119+
public void describeMismatch(Object item, Description description) {
120+
if (item instanceof BytesRef wkb) {
121+
var point = (Point) WellKnownBinary.fromWKB(GeometryValidator.NOOP, false, wkb.bytes, wkb.offset, wkb.length);
122+
description.appendText("was ").appendValue(point);
123+
} else {
124+
description.appendText("was ").appendValue(item);
125+
}
126+
}
127+
128+
@Override
129+
public void describeTo(Description description) {
130+
description.appendValue(" POINT (" + x + " " + y + ")");
131+
}
132+
}
89133
}

0 commit comments

Comments
 (0)