ESQL: Make function tests with timezone or locale use random configurations #138107
Conversation
… ones that use it
# Conflicts: # x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateFormatTests.java
# Conflicts: # x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java
# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/grouping/TBucket.java
…earch into esql-datetrunc-timezone # Conflicts: # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateTruncTests.java
# Conflicts: # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/grouping/BucketTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/grouping/TBucketTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/AbstractConfigurationFunctionTestCase.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateTruncTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/FoldNullTests.java
ivancea
added
>test
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
craigtaverner
left a comment
craigtaverner
left a comment
There was a problem hiding this comment.
LGTM. Some minor comments/questions.
| DataType.KEYWORD, | ||
| equalTo(null) | ||
| ).withStaticConfiguration() | ||
| ) |
There was a problem hiding this comment.
Should we have a withConfiguration here?
There was a problem hiding this comment.
Perhaps meaningless for null values?
There was a problem hiding this comment.
Exactly. A null will be null in whatever timezone, so better keep it randomized
| DataType.KEYWORD, | ||
| equalTo(null) | ||
| ).withStaticConfiguration() | ||
| ) |
There was a problem hiding this comment.
Should we have a withConfiguration here?
| return suppliers.stream() | ||
| .map(supplier -> new TestCaseSupplier(supplier.name(), supplier.types(), () -> mapper.apply(supplier.get()))) | ||
| .toList(); | ||
| .collect(Collectors.toCollection(ArrayList::new)); |
There was a problem hiding this comment.
Does this not do the same thing?
There was a problem hiding this comment.
.toList() returned list is immutable, and I wanted to update it after the mapping in some tests (Keep adding test cases).
The (current) usecase for this method was to update the config (or anything really) in TestCaseSuppliers provided by other utilities. Specifically the unary() and similar methods we have that magically generate them. I didn't want to add a weird callback/extra configuration to each of those functions.
test/framework/src/main/java/org/elasticsearch/test/ReadableMatchers.java
Outdated
Show resolved
Hide resolved
4 participants
Closes #107997
Since configurations were fully randomized, some tests requiring specific locales or timezones were changed to use a static config.
In this PR, those tests were updated to still use a random configuration, but override the single parameter they need to be static as part of the test case.