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: Fix mixed cluster tests #107680

Merged
merged 3 commits into from
Apr 22, 2024
Merged

Conversation

alex-spies
Copy link
Contributor

Fix #107618.

Do not send version as it was not an allowed parameter back then.
@alex-spies alex-spies added >test Issues or PRs that are addressing/adding tests auto-backport Automatically create backport pull requests when merged :Analytics/ES|QL AKA ESQL v8.14.0 v8.15.0 labels Apr 22, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Apr 22, 2024
Comment on lines 1439 to 1444
String versionString = null;
Set<EsqlVersion> versions = EsqlSpecTestCase.availableVersions();
if (versions.isEmpty() == false) {
// Use the latest released version or SNAPSHOT, if available.
versionString = versions.stream().max(Comparator.comparingInt(EsqlVersion::id)).toString();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were sending a version in the JSON body on bwc tests; the version parameter is only available on 8.13.3+, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

C.f. corresponding entry in build.gradle:

// Versions on and after 8.13.3 will get a `version` parameter
def versionUnsupported = bwcVersion -> {
return bwcVersion.before(Version.fromString("8.13.3"));
}

@@ -1552,7 +1552,7 @@ s2point1:d | s_mv:i | languages:i
2.1 | 3 | null
;

evalOverridingKey
evalOverridingKey#[skip:-8.13.1,reason:fixed in 8.13.2]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in #107131, which was backported to 8.13.2 but not earlier.

Copy link
Contributor

@luigidellaquila luigidellaquila left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@alex-spies alex-spies merged commit a19410f into elastic:main Apr 22, 2024
14 checks passed
@alex-spies alex-spies deleted the fix-mixed-cluster-tests branch April 22, 2024 12:08
alex-spies added a commit to alex-spies/elasticsearch that referenced this pull request Apr 22, 2024
* Fix a test where pre-8.13 nodes in mixed cluster tests were sent a language version (added in 8.13.3).
* Skip a test for a fix introduced in 8.13.2 on older clusters.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.14

elasticsearchmachine pushed a commit that referenced this pull request Apr 22, 2024
* Fix a test where pre-8.13 nodes in mixed cluster tests were sent a language version (added in 8.13.3).
* Skip a test for a fix introduced in 8.13.2 on older clusters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v8.14.0 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] FieldExtractorIT BWC test failing with NPE
3 participants