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

Add a new test of the current behavior for ingest pipeline names and types #93483

Merged
merged 3 commits into from
Feb 6, 2023

Conversation

joegallo
Copy link
Contributor

@joegallo joegallo commented Feb 2, 2023

Related to #80763

This test is pretty much a crystallization of my earlier comment there (#80763 (comment)) into a yml test. (edit: yml didn't work out, now it's a java test.)

Note: none of this is meant to say the current behavior is desirable, it's just capturing that this is what the current behavior happens to be.

I'm doing this as a separate PR so that the diff on my next PR will show how the behavior has changed, and make an even stronger case for why that PR is a good solution to the problem of the above-linked issue.

@joegallo joegallo added >test Issues or PRs that are addressing/adding tests :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP Team:Data Management Meta label for data/management team v8.7.0 labels Feb 2, 2023
@joegallo joegallo requested a review from masseyke February 2, 2023 20:25
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@joegallo
Copy link
Contributor Author

joegallo commented Feb 2, 2023

Ahhhhh, well, that's unfortunate. My test passes in isolation, but the yml tests all bang on the same cluster, and the other ingest yml tests pollute the global cluster stats processor count, so this test can't check call counts the way I want it to. ☹️

I guess I'll rewrite this as an IT or something then, but it won't be quite as pretty.

@joegallo
Copy link
Contributor Author

joegallo commented Feb 3, 2023

Alrighty, now it's a java test. If you look closely at the bytes of the test, you can see how annoyed I am about this. 😄

Copy link
Member

@masseyke masseyke left a comment

Choose a reason for hiding this comment

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

I'm not a huge fan of asserting incorrect behavior in tests, but 👍 since the goal is to immediately fix the bugs and change this test.

@joegallo joegallo merged commit a0ecb84 into elastic:main Feb 6, 2023
@joegallo joegallo deleted the ingest-stats-new-test branch February 6, 2023 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants