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: Rework integration-only csv testing #108313

Merged
merged 2 commits into from
May 6, 2024

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented May 6, 2024

This reworks the integration-test-only csv testing for metadata to use the required_feature: syntax instead of the -IT_tests_only extension. This is a little more flexible and way nicer on the eyes.

This reworks the integration-test-only csv testing for `metadata` to use
the `required_feature:` syntax instead of the `-IT_tests_only`
extension. This is a little more flexible and way nicer on the eyes.
@nik9000 nik9000 added >test Issues or PRs that are addressing/adding tests :Analytics/ES|QL AKA ESQL v8.14.1 v8.15.0 labels May 6, 2024
@nik9000 nik9000 requested a review from bpintea May 6, 2024 13:49
@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 May 6, 2024
@nik9000 nik9000 mentioned this pull request May 6, 2024
21 tasks
*/
assumeTrue("Test " + testName + " is not enabled", isEnabled(testName, Version.CURRENT));
assumeFalse("metadata fields aren't supported", testCase.requiredFeatures.contains(EsqlFeatures.METADATA_FIELDS.id()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're introducing METADATA_FIELDS now in 8.15, wouldn't the tests updated above no longer be run against 8.13 and 8.14 bwc runs? Is this intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah! Sorry, that's a sad side effect of dropping that version thingy - which we're supposed to be doing too. I figured I'd backport to 8.14 and ignore 8.13 because we're not likely to cut any more of those.

Copy link
Contributor

@bpintea bpintea left a comment

Choose a reason for hiding this comment

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

Lgtm

@nik9000 nik9000 added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport-and-merge labels May 6, 2024
@elasticsearchmachine elasticsearchmachine merged commit 089fd7d into elastic:main May 6, 2024
15 checks passed
@nik9000 nik9000 deleted the esql_metadata_feature branch May 6, 2024 15:07
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.14 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 108313

nik9000 added a commit to nik9000/elasticsearch that referenced this pull request May 6, 2024
This reworks the integration-test-only csv testing for `metadata` to use
the `required_feature:` syntax instead of the `-IT_tests_only`
extension. This is a little more flexible and way nicer on the eyes.
@nik9000
Copy link
Member Author

nik9000 commented May 6, 2024

Backport is #108326

nik9000 added a commit that referenced this pull request May 6, 2024
This reworks the integration-test-only csv testing for `metadata` to use
the `required_feature:` syntax instead of the `-IT_tests_only`
extension. This is a little more flexible and way nicer on the eyes.
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-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) 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.

3 participants