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 version test failure on non-SNAPSHOT builds #107138

Conversation

alex-spies
Copy link
Contributor

Fix #107106

Test the snapshot ESQL version separately and take the current build into account.

Test the snapshot ESQL version separately and take the current build
into account.
@alex-spies alex-spies added >test Issues or PRs that are addressing/adding tests :Analytics/ES|QL AKA ESQL v8.14.0 labels Apr 5, 2024
@alex-spies alex-spies requested a review from astefan April 5, 2024 08:59
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Apr 5, 2024
@elasticsearchmachine
Copy link
Collaborator

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

for (EsqlVersion version : EsqlVersion.values()) {
if (version == EsqlVersion.SNAPSHOT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the way to "skip" a test if some condition is not met is to use assumeTrue.

assumeTrue("Should not test snapshot version here", version != EsqlVersion.SNAPSHOT)

Copy link
Contributor

Choose a reason for hiding this comment

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

Please, ignore my comment. With this approach the test will always be skipped.

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM

@alex-spies
Copy link
Contributor Author

Thanks @astefan !

@alex-spies alex-spies merged commit ad77d32 into elastic:main Apr 5, 2024
14 checks passed
@alex-spies alex-spies deleted the skip-snapshot-version-check-on-non-snapshot-builds branch April 5, 2024 10:00
@alex-spies
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.13

Questions ?

Please refer to the Backport tool documentation

alex-spies added a commit to alex-spies/elasticsearch that referenced this pull request Apr 11, 2024
Test the snapshot ESQL version separately and take the current build
into account.

(cherry picked from commit ad77d32)
alex-spies added a commit that referenced this pull request Apr 11, 2024
)

Test the snapshot ESQL version separately and take the current build
into account.

(cherry picked from commit ad77d32)
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.

[CI] EsqlQueryRequestTests testKnownVersionIsValid failing
3 participants