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

[Ingest Pipelines] Fix functional tests #159336

Merged

Conversation

ElenaStoeva
Copy link
Contributor

Fixes #157511

Summary

This PR fixes the functional tests for Ingest Pipelines. Apparently, previously the test environment didn't have any automatically generated ingest pipelines and the tests were expecting an empty screen, but now there are three auto-generated managed pipelines, which is why the tests were failing.

Screenshot 2023-06-08 at 17 22 14

Checklist

@ElenaStoeva ElenaStoeva added Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes Feature:Ingest Node Pipelines Ingest node pipelines management labels Jun 8, 2023
@ElenaStoeva ElenaStoeva requested a review from a team June 8, 2023 16:23
@ElenaStoeva ElenaStoeva self-assigned this Jun 8, 2023
@ElenaStoeva ElenaStoeva changed the title Ingest pipelines fix functional test [Ingest Pipelines] Fix functional tests Jun 8, 2023
@ElenaStoeva
Copy link
Contributor Author

@elasticmachine merge upstream

@ElenaStoeva ElenaStoeva marked this pull request as ready for review June 9, 2023 10:33
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

@sabarasaba
Copy link
Member

Thanks for having a look at this @ElenaStoeva!

I believe this pipelines are suppoused to be cleaned up by whoever created them, so its ok for our tests to expect to have the empty state in the UI.

For the behavioral_analytics-events-final_pipeline I see there were some conversations about it that @yuliacech started.

And for the other two perhaps a test is not cleaning up after themselves? 🤔

@ElenaStoeva
Copy link
Contributor Author

Hey @sabarasaba, thanks for looking into this and for sending me the conversation about the behavioral_analytics-events-final_pipeline. It was mentioned in this discussion that a fix has been merged a month ago that would add the pipeline only when there are analytics collections, but it seems the pipeline is still created for some reason, so I'll ping the team to ask why this is the case.

Regarding the logs-default-pipeline, I see that this is coming from Elasticsearch and it has been added with elastic/elasticsearch#95971.

For the logs@json-message pipeline, this one has also been added 2 weeks ago with elastic/elasticsearch#96083.

Finally, I noticed that these 3 pipelines are regenerated even after I delete them in the test environment. So it seems the problem is not that the test user is not able to delete them, but that they always get recreated.

Overall, I wonder if we should change the tests so that they match the new scenario with the autogenerated pipelines and so they don't fail in the future if another such pipeline is added to Elasticsearch. What do you think?

@alisonelizabeth
Copy link
Contributor

Thanks for looking into this @ElenaStoeva! My suggestion would be to make this test more future-proof.

We can create our own "test" pipeline as part of the test setup and confirm it exists in UI. Then, we should make sure to delete on cleanup. That way, the test isn't dependent on any changes that might be made outside of our control.

@ElenaStoeva
Copy link
Contributor Author

Thanks for looking into this @ElenaStoeva! My suggestion would be to make this test more future-proof.

We can create our own "test" pipeline as part of the test setup and confirm it exists in UI. Then, we should make sure to delete on cleanup. That way, the test isn't dependent on any changes that might be made outside of our control.

Thanks for the suggestion @alisonelizabeth! I think this is a great idea for making the test future-proof.

@ElenaStoeva
Copy link
Contributor Author

@elasticmachine merge upstream

@ElenaStoeva
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 410 414 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 491 495 +4
total +6

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @ElenaStoeva

@alisonelizabeth
Copy link
Contributor

Thanks for updating the test @ElenaStoeva! This looks good. Have you had a chance to run it through the flaky test runner yet? Let me know if you need help here.

@ElenaStoeva
Copy link
Contributor Author

Thanks for updating the test @ElenaStoeva! This looks good. Have you had a chance to run it through the flaky test runner yet? Let me know if you need help here.

Thanks for looking into this @alisonelizabeth! I just ran the flaky test runner and it passed: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2417

Copy link
Contributor

@alisonelizabeth alisonelizabeth 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 @ElenaStoeva!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Ingest Node Pipelines Ingest node pipelines management release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing ES Promotion: FTR Configs #31 / Ingest pipelines app Ingest Pipelines Loads the app
6 participants