Skip to content

Commit

Permalink
Fixup reduceRandom tests (#65263)
Browse files Browse the repository at this point in the history
In aa1ea96 I made all
`testReduceRandom` tests for aggs mimick production more precisely.
More precisely, they pick the correct "lead" result when performing
partial reduction. This is great, but, sadly, some tests assumed that we
always reduced against the "first" aggregator. This fixes those tests.

Closes #65163
  • Loading branch information
nik9000 committed Nov 20, 2020
1 parent 956ae93 commit 56605e4
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ public class InternalTopHitsTests extends InternalAggregationTestCase<InternalTo
@Override
protected InternalTopHits createTestInstance(String name, Map<String, Object> metadata) {
if (randomBoolean()) {
return createTestInstanceSortedByFields(name, metadata, ESTestCase::randomFloat,
return createTestInstanceSortedByFields(name, between(1, 40), metadata, ESTestCase::randomFloat,
randomSortFields(), InternalTopHitsTests::randomOfType);
}
return createTestInstanceSortedScore(name, metadata, ESTestCase::randomFloat);
return createTestInstanceSortedScore(name, between(1, 40), metadata, ESTestCase::randomFloat);
}

@Override
Expand All @@ -87,6 +87,7 @@ protected List<InternalTopHits> randomResultsToReduce(String name, int size) {
usedScores.add(score);
return score;
};
int requestedSize = between(1, 40);
Supplier<InternalTopHits> supplier;
if (randomBoolean()) {
SortField[] sortFields = randomSortFields();
Expand All @@ -96,16 +97,16 @@ protected List<InternalTopHits> randomResultsToReduce(String name, int size) {
usedSortFieldValues.add(value);
return value;
};
supplier = () -> createTestInstanceSortedByFields(name, null, scoreSupplier, sortFields, sortFieldValueSuppier);
supplier = () -> createTestInstanceSortedByFields(name, requestedSize, null, scoreSupplier, sortFields, sortFieldValueSuppier);
} else {
supplier = () -> createTestInstanceSortedScore(name, null, scoreSupplier);
supplier = () -> createTestInstanceSortedScore(name, requestedSize, null, scoreSupplier);
}
return Stream.generate(supplier).limit(size).collect(toList());
}

private InternalTopHits createTestInstanceSortedByFields(String name, Map<String, Object> metadata,
private InternalTopHits createTestInstanceSortedByFields(String name, int requestedSize, Map<String, Object> metadata,
Supplier<Float> scoreSupplier, SortField[] sortFields, Function<SortField.Type, Object> sortFieldValueSupplier) {
return createTestInstance(name, metadata, scoreSupplier,
return createTestInstance(name, metadata, scoreSupplier, requestedSize,
(docId, score) -> {
Object[] fields = new Object[sortFields.length];
for (int f = 0; f < sortFields.length; f++) {
Expand All @@ -118,15 +119,20 @@ private InternalTopHits createTestInstanceSortedByFields(String name, Map<String
sortFieldsComparator(sortFields));
}

private InternalTopHits createTestInstanceSortedScore(String name, Map<String, Object> metadata, Supplier<Float> scoreSupplier) {
return createTestInstance(name, metadata, scoreSupplier, ScoreDoc::new, TopDocs::new, scoreComparator());
private InternalTopHits createTestInstanceSortedScore(
String name,
int requestedSize,
Map<String, Object> metadata,
Supplier<Float> scoreSupplier
) {
return createTestInstance(name, metadata, scoreSupplier, requestedSize, ScoreDoc::new, TopDocs::new, scoreComparator());
}

private InternalTopHits createTestInstance(String name, Map<String, Object> metadata, Supplier<Float> scoreSupplier,
private InternalTopHits createTestInstance(String name, Map<String, Object> metadata, Supplier<Float> scoreSupplier,
int requestedSize,
BiFunction<Integer, Float, ScoreDoc> docBuilder,
BiFunction<TotalHits, ScoreDoc[], TopDocs> topDocsBuilder, Comparator<ScoreDoc> comparator) {
int from = 0;
int requestedSize = between(1, 40);
int actualSize = between(0, requestedSize);

float maxScore = Float.NEGATIVE_INFINITY;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,6 @@ protected T createUnmappedInstance(String name, Map<String, Object> metadata) {
}

@Override

protected final Class<T> categoryClass() {
return (Class<T>) InternalAggregation.class;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,27 @@ protected void assertFromXContent(InternalTopMetrics aggregation, ParsedAggregat
}
}

@Override
protected List<InternalTopMetrics> randomResultsToReduce(String name, int count) {
InternalTopMetrics prototype = createTestInstance();
return randomList(
count,
count,
() -> new InternalTopMetrics(
prototype.getName(),
prototype.getSortOrder(),
prototype.getMetricNames(),
prototype.getSize(),
randomTopMetrics(
InternalAggregationTestCase::randomNumericDocValueFormat,
between(0, prototype.getSize()),
prototype.getMetricNames().size()
),
prototype.getMetadata()
)
);
}

@Override
protected void assertReduced(InternalTopMetrics reduced, List<InternalTopMetrics> inputs) {
InternalTopMetrics first = inputs.get(0);
Expand Down

0 comments on commit 56605e4

Please sign in to comment.