From 4670b5be4a0993550035c3bebb2dca4623a13fad Mon Sep 17 00:00:00 2001 From: gmarouli Date: Thu, 19 Jun 2025 17:30:32 +0300 Subject: [PATCH 1/3] Add an error margin when comparing floats. --- muted-tests.yml | 9 ---- .../matchers/source/DynamicFieldMatcher.java | 48 ++++++++++++------- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index 6729ceaf6ac3d..9a4791a691fa3 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -541,15 +541,6 @@ tests: - class: org.elasticsearch.xpack.esql.qa.multi_node.EsqlSpecIT method: test {knn-function.KnnSearchWithKOption SYNC} issue: https://github.com/elastic/elasticsearch/issues/129512 -- class: org.elasticsearch.xpack.logsdb.qa.StandardVersusStandardReindexedIntoLogsDbChallengeRestIT - method: testMatchAllQuery - issue: https://github.com/elastic/elasticsearch/issues/129527 -- class: org.elasticsearch.xpack.logsdb.qa.BulkChallengeRestIT - method: testMatchAllQuery - issue: https://github.com/elastic/elasticsearch/issues/129528 -- class: org.elasticsearch.xpack.logsdb.qa.StoredSourceLogsDbVersusReindexedLogsDbChallengeRestIT - method: testMatchAllQuery - issue: https://github.com/elastic/elasticsearch/issues/129529 - class: org.elasticsearch.index.engine.ThreadPoolMergeExecutorServiceTests method: testIORateIsAdjustedForAllRunningMergeTasks issue: https://github.com/elastic/elasticsearch/issues/129531 diff --git a/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java b/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java index 1da503a9b792f..1f367b8b605f3 100644 --- a/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java +++ b/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java @@ -17,9 +17,8 @@ import java.util.List; import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.function.Function; -import java.util.stream.Collectors; +import java.util.function.Supplier; import static org.elasticsearch.datageneration.matchers.Messages.formatErrorMessage; import static org.elasticsearch.datageneration.matchers.Messages.prettyPrintCollections; @@ -62,31 +61,46 @@ public MatchResult match(List actual, List expected) { var normalizedActual = normalizeDoubles(actual); var normalizedExpected = normalizeDoubles(expected); + Supplier noMatchSupplier = () -> MatchResult.noMatch( + formatErrorMessage( + actualMappings, + actualSettings, + expectedMappings, + expectedSettings, + "Values of dynamically mapped field containing double values don't match after normalization, normalized " + + prettyPrintCollections(normalizedActual, normalizedExpected) + ) + ); - return normalizedActual.equals(normalizedExpected) - ? MatchResult.match() - : MatchResult.noMatch( - formatErrorMessage( - actualMappings, - actualSettings, - expectedMappings, - expectedSettings, - "Values of dynamically mapped field containing double values don't match after normalization, normalized " - + prettyPrintCollections(normalizedActual, normalizedExpected) - ) - ); + if (normalizedActual.size() != normalizedExpected.size()) { + return noMatchSupplier.get(); + } + + for (int i = 0; i < normalizedActual.size(); i++) { + if (floatsEquals(normalizedActual.get(i), normalizedExpected.get(i))) { + return noMatchSupplier.get(); + } + } + + return MatchResult.match(); } return matchWithGenericMatcher(actual, expected); } - private static Set normalizeDoubles(List values) { + private static List normalizeDoubles(List values) { if (values == null) { - return Set.of(); + return List.of(); } Function toFloat = (o) -> o instanceof Number n ? n.floatValue() : Float.parseFloat((String) o); - return values.stream().filter(Objects::nonNull).map(toFloat).collect(Collectors.toSet()); + + // We skip nulls because they trip the pretty print collections. + return values.stream().filter(Objects::nonNull).map(toFloat).toList(); + } + + private static boolean floatsEquals(Float actual, Float expected) { + return Math.abs(actual - expected) > 1e-8; } private MatchResult matchWithGenericMatcher(List actualValues, List expectedValues) { From f790ff500024d110865aeaf64efd5a55958f54bc Mon Sep 17 00:00:00 2001 From: gmarouli Date: Fri, 20 Jun 2025 11:48:13 +0300 Subject: [PATCH 2/3] Align implementation with method name --- .../matchers/source/DynamicFieldMatcher.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java b/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java index 1f367b8b605f3..ef3764bc01fcd 100644 --- a/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java +++ b/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java @@ -24,6 +24,7 @@ import static org.elasticsearch.datageneration.matchers.Messages.prettyPrintCollections; class DynamicFieldMatcher { + private static final double FLOAT_ERROR_MARGIN = 1e-8; private final XContentBuilder actualMappings; private final Settings.Builder actualSettings; private final XContentBuilder expectedMappings; @@ -77,7 +78,7 @@ public MatchResult match(List actual, List expected) { } for (int i = 0; i < normalizedActual.size(); i++) { - if (floatsEquals(normalizedActual.get(i), normalizedExpected.get(i))) { + if (floatEquals(normalizedActual.get(i), normalizedExpected.get(i)) == false) { return noMatchSupplier.get(); } } @@ -99,8 +100,8 @@ private static List normalizeDoubles(List values) { return values.stream().filter(Objects::nonNull).map(toFloat).toList(); } - private static boolean floatsEquals(Float actual, Float expected) { - return Math.abs(actual - expected) > 1e-8; + private static boolean floatEquals(Float actual, Float expected) { + return Math.abs(actual - expected) < FLOAT_ERROR_MARGIN; } private MatchResult matchWithGenericMatcher(List actualValues, List expectedValues) { From 238e422b8650b3ed01118d44b69b76e82c648404 Mon Sep 17 00:00:00 2001 From: gmarouli Date: Mon, 23 Jun 2025 16:52:58 +0300 Subject: [PATCH 3/3] Add comment to explain the normalisation choices --- .../datageneration/matchers/source/DynamicFieldMatcher.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java b/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java index ef3764bc01fcd..5ac8055a85fe5 100644 --- a/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java +++ b/test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/DynamicFieldMatcher.java @@ -89,6 +89,11 @@ public MatchResult match(List actual, List expected) { return matchWithGenericMatcher(actual, expected); } + /** + * We make the normalisation of double values stricter than {@link SourceTransforms#normalizeValues} to facilitate the equality of the + * values within a margin of error. Synthetic source does support duplicate values and preserves the order, but it loses some accuracy, + * this is why the margin of error is very important. In the future, we can make {@link SourceTransforms#normalizeValues} also stricter. + */ private static List normalizeDoubles(List values) { if (values == null) { return List.of();