Skip to content

ESQL: Send language version in spec tests#107268

Merged
alex-spies merged 2 commits intoelastic:mainfrom
alex-spies:esql-version-in-esqlspecit
Apr 11, 2024
Merged

ESQL: Send language version in spec tests#107268
alex-spies merged 2 commits intoelastic:mainfrom
alex-spies:esql-version-in-esqlspecit

Conversation

@alex-spies
Copy link
Copy Markdown
Contributor

@alex-spies alex-spies commented Apr 9, 2024

Relates #104890

For now this just send a random version; later, we will want to specify applicable versions in the csv tests themselves.

For now this just sends a random version; later, we will want to specify
applicable versions in the csv tests themselves.
this.testCase = testCase;
this.mode = mode;
// TODO: Read applicable versions from csv-spec files/make it part of testCase.
this.versions = Build.current().isSnapshot() ? Set.of(EsqlVersion.values()) : Set.of(EsqlVersion.releasedAscending());
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Once we introduce the first change that requires a new lang version, I think our csv tests will look roughly like this:

mvMedian
// Until version 2024.04.01, inclusive
versions:-=2024.04.01
row i = [1,2,3,4] | eval mv_median(i) | drop i;

mv_median(i):i
2
;

mvMedianNew
// From the next version
versions:2024.04.01+
row i = [1,2,3,4] | eval mv_median(i) | drop i;

mv_median(i):d
2.5
;

The old mvMedian test will be tested with a random version up to 2024.04.01, while mvMedianNew will be tested with a random later version, including snapshot.

I think we'll also want a mechanism to specify the version to run the spec/csv tests at; when making a breaking change (like having mv_median return a different data type), it's important to run as many tests against snapshot as possible to see what breaks.

@alex-spies
Copy link
Copy Markdown
Contributor Author

buildkite run elasticsearch-ci/bwc-snapshots

@alex-spies alex-spies added >test Issues or PRs that are addressing/adding tests :Analytics/ES|QL AKA ESQL labels Apr 10, 2024
@alex-spies alex-spies marked this pull request as ready for review April 10, 2024 14:04
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Apr 10, 2024
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

@alex-spies alex-spies added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) and removed auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels Apr 11, 2024
@alex-spies alex-spies merged commit 8704189 into elastic:main Apr 11, 2024
@alex-spies alex-spies deleted the esql-version-in-esqlspecit branch April 11, 2024 08:49
craigtaverner pushed a commit to craigtaverner/elasticsearch that referenced this pull request Apr 11, 2024
For now this just sends a random version; later, we will want to specify
applicable versions in the csv tests themselves.
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 v8.14.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants