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

Ignore CSV tests that require field caps change #119940

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

ioanatia
Copy link
Contributor

#119918

semantic_text fields are now reported as text by the field caps which results in failures for CSV tests for non snapshot builds.

This is a quirk of CSV tests that I will need to fix, single-node and multi-node tests should still run the semantic text tests.
From what I tested locally the single and multi node tests should be succesful.

Since this seems to be more of a test problem and not an indicative of a prod bug, the goal is to enable the CSV test suite for now.

@ioanatia ioanatia 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.18.0 labels Jan 10, 2025
@ioanatia ioanatia requested a review from alex-spies January 10, 2025 13:35
@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.0.0 labels Jan 10, 2025
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Contributor

@tteofili tteofili left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@alex-spies alex-spies 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 for looking into it quickly!

@ioanatia ioanatia merged commit 4f5ad9d into elastic:main Jan 10, 2025
16 checks passed
@ioanatia ioanatia deleted the csv_tests branch January 10, 2025 14:40
@ioanatia ioanatia added auto-backport Automatically create backport pull requests when merged and removed auto-backport Automatically create backport pull requests when merged v8.18.0 labels Jan 10, 2025
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

The backport operation could not be completed due to the following error:

There are no branches to backport to. Aborting.

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

@ioanatia
Copy link
Contributor Author

ioanatia commented Jan 10, 2025

tests are not muted on 8.x - but I still need to backport the CsvTests change 🤦‍♀️

EDIT: actually #119792 needs to be backported first

Mikep86 pushed a commit to Mikep86/elasticsearch that referenced this pull request Jan 10, 2025
@Mikep86
Copy link
Contributor

Mikep86 commented Jan 10, 2025

This change was backported in #119962

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 backport pending Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants