Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Esql warnings in tests #98699

Merged
merged 4 commits into from
Aug 22, 2023
Merged

Esql warnings in tests #98699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
079dc9a
Stack of pow tests, and returning NaNs
not-napoleon Aug 21, 2023
3e34c28
Warn on integer overflow test
not-napoleon Aug 21, 2023
246db71
Warn on long overflow
not-napoleon Aug 21, 2023
7a3ad04
Return nulls instead of NaNs from pow
not-napoleon Aug 22, 2023
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
37 changes: 37 additions & 0 deletions .../test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.time.Duration;
import java.time.Period;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ExecutionException;
Expand Down Expand Up @@ -83,12 +84,33 @@ public static class TestCase {
*/
private Matcher<Object> matcher;

/**
* Warnings this test is expected to produce
*/
private String[] expectedWarnings;

public TestCase(List<TypedData> data, String evaluatorToString, DataType expectedType, Matcher<Object> matcher) {
this.source = Source.EMPTY;
this.data = data;
this.evaluatorToString = evaluatorToString;
this.expectedType = expectedType;
this.matcher = matcher;
this.expectedWarnings = null;
}

public TestCase(
Copy link
Member

Choose a reason for hiding this comment

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

This maybe should be private just so it's normal to build it without the warnings and add, I guess.

Copy link
Member

Choose a reason for hiding this comment

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

Like, I imagine it'd be nice if there was one "normal" way to do it.

List<TypedData> data,
String evaluatorToString,
DataType expectedType,
Matcher<Object> matcher,
String... expectedWarnings
) {
this.source = Source.EMPTY;
this.data = data;
this.evaluatorToString = evaluatorToString;
this.expectedType = expectedType;
this.matcher = matcher;
this.expectedWarnings = expectedWarnings;
}

public Source getSource() {
Expand All @@ -114,6 +136,15 @@ public List<Object> getDataValues() {
public Matcher<Object> getMatcher() {
return matcher;
}

public TestCase withWarning(String warning) {
if (expectedWarnings != null) {
String[] newWarngings = Arrays.copyOf(this.expectedWarnings, this.expectedWarnings.length + 1);
newWarngings[this.expectedWarnings.length] = warning;
return new TestCase(this.data, this.evaluatorToString, this.expectedType, this.matcher, newWarngings);
}
return new TestCase(this.data, this.evaluatorToString, this.expectedType, this.matcher, warning);
}
}

/**
Expand Down Expand Up @@ -234,6 +265,9 @@ public final void testSimple() {
// TODO should we convert unsigned_long into BigDecimal so it's easier to assert?
Object result = toJavaObject(evaluator(expression).get().eval(row(testCase.getDataValues())), 0);
assertThat(result, testCase.getMatcher());
if (testCase.expectedWarnings != null) {
assertWarnings(testCase.expectedWarnings);
}
}

public final void testSimpleWithNulls() {
Expand Down Expand Up @@ -295,6 +329,9 @@ public final void testSimpleConstantFolding() {
assertThat(e.dataType(), equalTo(testCase.expectedType));
assertTrue(e.foldable());
assertThat(e.fold(), testCase.getMatcher());
if (testCase.expectedWarnings != null) {
assertWarnings(testCase.expectedWarnings);
}
}

public void testSerializationOfSimple() {
Expand Down
13 changes: 8 additions & 5 deletions ...rc/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/Log10Tests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.type.DataTypes;
import org.hamcrest.Matcher;

import java.util.List;
import java.util.function.Supplier;
Expand All @@ -38,13 +37,17 @@ public static Iterable<Object[]> parameters() {
DataTypes.DOUBLE,
equalTo(Math.log10(arg))
);
}), new TestCaseSupplier("Log10(negative)", () -> {
double arg = randomIntBetween(Integer.MIN_VALUE, -1); // it's inclusive
return new TestCase(
List.of(new TypedData(arg, DataTypes.DOUBLE, "arg")),
"Log10DoubleEvaluator[val=Attribute[channel=0]]",
DataTypes.DOUBLE,
equalTo(Double.NaN)
);
})));
}

private Matcher<Object> resultsMatcher(List<TypedData> typedData) {
return equalTo(Math.log10((Double) typedData.get(0).data()));
}

@Override
protected Expression build(Source source, List<Expression> args) {
return new Log10(source, args.get(0));
Expand Down
Loading