Skip to content

ESQL: Finish migrating error testing#143322

Merged
nik9000 merged 3 commits intoelastic:mainfrom
nik9000:esql_remove_errors_for_cases
Mar 2, 2026
Merged

ESQL: Finish migrating error testing#143322
nik9000 merged 3 commits intoelastic:mainfrom
nik9000:esql_remove_errors_for_cases

Conversation

@nik9000
Copy link
Copy Markdown
Member

@nik9000 nik9000 commented Feb 28, 2026

A long time ago (#119678) we started migrating type error testing from many test cases to a single test. This was supposed to allow us to significantly simplify our testing infrastructure. But we never finished it. Well, I've finally done it!

This:

  • Replaces errorsForCasesWithoutExamples and subclasses of ErrorsForCasesWithoutExamplesTestCase
  • Replaces TestCaseSupplier.TestCase.typeError in TopTests with a new TopErrorTests

A long time ago (elastic#119678) we started migrating type error testing from
many test cases to a single test. This was supposed to allow us to
significantly simplify our testing infrastructure. But we never finished
it. Well, I've finally done it!

This:
* Replaces `errorsForCasesWithoutExamples` and subclasses of
  `ErrorsForCasesWithoutExamplesTestCase`
* Replaces `TestCaseSupplier.TestCase.typeError` in `TopTests` with a
  new `TopErrorTests`
@nik9000 nik9000 requested a review from ivancea February 28, 2026 02:39
@nik9000 nik9000 added >test Issues or PRs that are addressing/adding tests :Analytics/ES|QL AKA ESQL v9.4.0 labels Feb 28, 2026
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Feb 28, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

.and(isType(limitField(), dt -> dt == DataType.INTEGER, sourceText(), SECOND, "integer"))
.and(isNotNull(orderField(), sourceText(), THIRD))
.and(isString(orderField(), sourceText(), THIRD));
.and(isType(orderField(), dt -> dt == DataType.KEYWORD, sourceText(), THIRD, "keyword"));
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This wants a constant string and those are always keyword.

Copy link
Copy Markdown
Contributor

@ivancea ivancea left a comment

Choose a reason for hiding this comment

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

Thanks Nik!

null
);
assertTrue(top.typeResolved().unresolved());
assertThat(top.typeResolved().message(), equalTo("Limit must be greater than 0 in [], found [0]"));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! 🙏
Those should have been PostAnalysisVerificationAware afaik. We should migrate them eventually

* For two args, this assumes they are both spatial.
* For three args, we assume two spatial and one additional numerical argument, treated differently.
*/
protected static String typeErrorMessage(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At some point, I start wondering if this is less code than just hardcoding every combination... Just a thought, feel free to ignore it

@nik9000 nik9000 merged commit cb06312 into elastic:main Mar 2, 2026
35 checks passed
szybia added a commit to szybia/elasticsearch that referenced this pull request Mar 2, 2026
…locations

* upstream/main: (94 commits)
  Mute org.elasticsearch.xpack.esql.qa.mixed.EsqlClientYamlIT test {p0=esql/40_tsdb/TS Command grouping on text field} elastic#142544
  Mute org.elasticsearch.index.store.StoreDirectoryMetricsIT testDirectoryMetrics elastic#143419
  Mute org.elasticsearch.xpack.esql.qa.multi_node.GenerativeIT test elastic#143023
  TS_INFO information retrieval command (elastic#142721)
  ESQL: External source parallel execution and distribution (elastic#143349)
  Mute org.elasticsearch.index.mapper.blockloader.FlattenedFieldRootBlockLoaderTests testBlockLoaderForFieldInObject {preference=Params[syntheticSource=false, preference=DOC_VALUES]} elastic#143414
  Mute org.elasticsearch.index.mapper.blockloader.FlattenedFieldRootBlockLoaderTests testBlockLoaderForFieldInObject {preference=Params[syntheticSource=false, preference=NONE]} elastic#143413
  Mute org.elasticsearch.index.mapper.blockloader.FlattenedFieldRootBlockLoaderTests testBlockLoaderForFieldInObject {preference=Params[syntheticSource=false, preference=STORED]} elastic#143412
  Removing ingest random sampling (elastic#143289)
  Mute org.elasticsearch.xpack.esql.qa.single_node.GenerativeIT test elastic#143023
  [Transform] Clean up internal tests (elastic#143246)
  Skip time series field type merge for non-TS agg queries (elastic#143262)
  Enable zero-copy SIMD vector scoring on searchable snapshots (frozen tier) (elastic#141718)
  Mute org.elasticsearch.xpack.search.CrossClusterAsyncSearchIT testCancelViaExpirationOnRemoteResultsWithMinimizeRoundtrips elastic#143407
  Fix MemorySegmentUtilsTests (elastic#143391)
  Unmute testWorkflowsRestrictionAllowsAccess (elastic#143308)
  Cancel async query on expiry (elastic#143016)
  ESQL: Finish migrating error testing (elastic#143322)
  Reduce LuceneOperator.Status memory consumption with large QueryDSL queries (elastic#143175)
  ESQL: Generative testing with full text functions (elastic#142961)
  ...
tballison pushed a commit to tballison/elasticsearch that referenced this pull request Mar 3, 2026
A long time ago (elastic#119678) we started migrating type error testing from
many test cases to a single test. This was supposed to allow us to
significantly simplify our testing infrastructure. But we never finished
it. Well, I've finally done it!

This:
* Replaces `errorsForCasesWithoutExamples` and subclasses of
  `ErrorsForCasesWithoutExamplesTestCase`
* Replaces `TestCaseSupplier.TestCase.typeError` in `TopTests` with a
  new `TopErrorTests`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants