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.

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: Send language version in spec tests #107268

Merged
merged 2 commits into from
Apr 11, 2024

Conversation

alex-spies
Copy link
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.
@@ -96,6 +100,8 @@ protected EsqlSpecTestCase(String fileName, String groupName, String testName, I
this.lineNumber = lineNumber;
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
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
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
Collaborator

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

@alex-spies alex-spies added auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) and removed auto-merge 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
15 checks passed
@alex-spies alex-spies deleted the esql-version-in-esqlspecit branch April 11, 2024 08:49
nielsbauman pushed a commit to nielsbauman/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.
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.

None yet

3 participants