Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import org.elasticsearch.test.rest.yaml.section.ClientYamlTestSection;
import org.elasticsearch.test.rest.yaml.section.DoSection;
import org.elasticsearch.test.rest.yaml.section.ExecutableSection;
import org.elasticsearch.test.rest.yaml.section.IsFalseAssertion;
import org.elasticsearch.xcontent.XContentLocation;
import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase;
import org.junit.After;
import org.junit.Before;
Expand Down Expand Up @@ -51,18 +53,25 @@ private void assertRequestBreakerEmpty() throws Exception {
EsqlSpecTestCase.assertRequestBreakerEmpty();
}

public static Iterable<Object[]> updateEsqlQueryDoSections(Iterable<Object[]> parameters, Function<DoSection, ExecutableSection> modify)
throws Exception {
public static Iterable<Object[]> partialResultsCheckingParameters() throws Exception {
return updateExecutableSections(createParameters(), AbstractEsqlClientYamlIT::insertPartialResultsAssertion);
}

public static Iterable<Object[]> updateExecutableSections(
Iterable<Object[]> parameters,
Function<List<ExecutableSection>, List<ExecutableSection>> updateFunction
) {
List<Object[]> result = new ArrayList<>();
for (Object[] orig : parameters) {
assert orig.length == 1;
ClientYamlTestCandidate candidate = (ClientYamlTestCandidate) orig[0];
try {
var testSection = candidate.getTestSection();
ClientYamlTestSection modified = new ClientYamlTestSection(
candidate.getTestSection().getLocation(),
candidate.getTestSection().getName(),
candidate.getTestSection().getPrerequisiteSection(),
candidate.getTestSection().getExecutableSections().stream().map(e -> modifyExecutableSection(e, modify)).toList()
testSection.getLocation(),
testSection.getName(),
testSection.getPrerequisiteSection(),
updateFunction.apply(testSection.getExecutableSections())
);
result.add(new Object[] { new ClientYamlTestCandidate(candidate.getRestTestSuite(), modified) });
} catch (IllegalArgumentException e) {
Expand All @@ -72,6 +81,33 @@ public static Iterable<Object[]> updateEsqlQueryDoSections(Iterable<Object[]> pa
return result;
}

public static Iterable<Object[]> updateEsqlQueryDoSections(
Iterable<Object[]> parameters,
Function<DoSection, ExecutableSection> modify
) {
return updateExecutableSections(parameters, sections -> sections.stream().map(e -> modifyExecutableSection(e, modify)).toList());
}

private static List<ExecutableSection> insertPartialResultsAssertion(List<ExecutableSection> sections) {
var newSections = new ArrayList<ExecutableSection>(sections.size());
for (var section : sections) {
var insertIsPartial = false;
if (section instanceof DoSection doSection) {
var apiCallSection = doSection.getApiCallSection();
if (apiCallSection.getApi().equals("esql.query")) {
// If `allow_partial_results` is explicitly set to true, partial results are allowed.
// If it's missing, no partial results are expected, even if the parameter's default is true.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit of magic, I think it's a pretty reasonable sacrifice.

insertIsPartial = "true".equals(apiCallSection.getParams().get("allow_partial_results")) == false;
}
}
newSections.add(section);
if (insertIsPartial) {
newSections.add(new IsFalseAssertion(XContentLocation.UNKNOWN, "is_partial"));
}
}
return newSections.size() == sections.size() ? sections : newSections;
}

private static ExecutableSection modifyExecutableSection(ExecutableSection e, Function<DoSection, ExecutableSection> modify) {
if (false == (e instanceof DoSection)) {
return e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public EsqlClientYamlAsyncIT(final ClientYamlTestCandidate testCandidate) {

@ParametersFactory
public static Iterable<Object[]> parameters() throws Exception {
return updateEsqlQueryDoSections(createParameters(), doSection -> {
return updateEsqlQueryDoSections(partialResultsCheckingParameters(), doSection -> {
ApiCallSection copy = doSection.getApiCallSection().copyWithNewApi("esql.async_query");
for (Map<String, Object> body : copy.getBodies()) {
body.put("wait_for_completion_timeout", "30m");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public EsqlClientYamlAsyncSubmitAndFetchIT(final ClientYamlTestCandidate testCan

@ParametersFactory
public static Iterable<Object[]> parameters() throws Exception {
return updateEsqlQueryDoSections(createParameters(), DoEsqlAsync::new);
return updateEsqlQueryDoSections(partialResultsCheckingParameters(), DoEsqlAsync::new);
}

private static class DoEsqlAsync implements ExecutableSection {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ public EsqlClientYamlIT(final ClientYamlTestCandidate testCandidate) {

@ParametersFactory
public static Iterable<Object[]> parameters() throws Exception {
return createParameters();
return partialResultsCheckingParameters();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,9 @@ protected final void doTest(String query) throws Throwable {
Map<?, ?> prevTooks = supportsTook() ? tooks() : null;
Map<String, Object> answer = runEsql(builder.query(query), testCase.assertWarnings(deduplicateExactWarnings()));

var isPartial = answer.get("is_partial");
assertTrue("unexpected partial results", isPartial == null || (boolean) isPartial == false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be easier to read with assertThat(..., anyOf(is(false), nullValue())). Dunno. Your call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, better, thanks.


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we also extract the failures and throw them?

Copy link
Contributor Author

@bpintea bpintea Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I've extracted _clusters contents into the assertion message (we don't have more otherwise).

var expectedColumnsWithValues = loadCsvSpecValues(testCase.expectedResults);

var metadata = answer.get("columns");
Expand Down