Skip to content

[Index management] Clean all tests after execution and navigate directly to the tab#232198

Merged
SoniaSanzV merged 6 commits intoelastic:mainfrom
SoniaSanzV:kbn_mngmt_test/index_management_tests_cleanup
Aug 29, 2025
Merged

[Index management] Clean all tests after execution and navigate directly to the tab#232198
SoniaSanzV merged 6 commits intoelastic:mainfrom
SoniaSanzV:kbn_mngmt_test/index_management_tests_cleanup

Conversation

@SoniaSanzV
Copy link
Contributor

@SoniaSanzV SoniaSanzV commented Aug 19, 2025

Summary

Space time project. I'm trying to improve the test for Kibana Management. In this PR, I did two things:

  • Clean up all created resources after execution. This suite contained a lot of resources that wasn't deleted after execution. That complicates local testing and leads to failures if you re-run the tests.
  • Navigates directly to the desired tab when setting up the tests. This is not an anti-patter, but it was time consuming. We were doing this:
      await pageObjects.common.navigateToApp('indexManagement');
      // Navigate to the data streams tab
      await pageObjects.indexManagement.changeTabs('data_streamsTab');
      await pageObjects.header.waitUntilLoadingHasFinished();

But the navigateToApp method already contains a path parameter, so doing it this way, we can get rid of unnecessaries waitUntilLoadingHasFinished calls.
With this simple change, the execution time of serverless tests locally dropped from 2 minutes to 1 minute.

  • Also, changes the data-test-subj for creating a component template from createPipelineButton to createComponentTemplateButton, that wasn't reflecting what the button really does.

Flay test runner stateful tests: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/9177
Flay test runner serverless tests: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/9182

@SoniaSanzV SoniaSanzV self-assigned this Aug 19, 2025
@SoniaSanzV SoniaSanzV requested a review from a team as a code owner August 19, 2025 09:44
@SoniaSanzV SoniaSanzV added Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes backport:all-open Backport to all branches that could still receive a release labels Aug 19, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-management (Team:Kibana Management)

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#9177

[✅] x-pack/platform/test/functional/apps/index_management/config.ts: 100/100 tests passed.

see run history

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#9182

[✅] x-pack/platform/test/serverless/functional/configs/search/config.group1.ts: 100/100 tests passed.

see run history

Copy link
Contributor

@ElenaStoeva ElenaStoeva left a comment

Choose a reason for hiding this comment

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

Great job @SoniaSanzV - it's nice to see the test running time reduction! Changes lgtm with one suggestion for an improvement.

} catch (e) {
log.debug('[Teardown error] Error deleting test policy');
throw e;
log.debug(`[Teardown error] Error deleting test index: ${e.message}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

We could separate these into two try-catch blocks so that if one fails the other resource is still deleted, and the log message would be more helpful (i.e. right now, if the policy deletion failed, the logs will say that the index deletion failed).

} catch (e) {
log.debug('[Teardown error] Error deleting test policy');
throw e;
log.debug(`[Teardown error] Error deleting test policy: ${e.message}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here

} catch (e) {
log.debug('[Teardown error] Error deleting test policy');
throw e;
log.debug(`[Teardown error] Error deleting test index: ${e.message}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here

} catch (e) {
log.debug('[Teardown error] Error deleting test policy');
throw e;
log.debug(`[Teardown error] Error deleting test index: ${e.message}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here

@SoniaSanzV
Copy link
Contributor Author

Great job @SoniaSanzV - it's nice to see the test running time reduction! Changes lgtm with one suggestion for an improvement.

Thank you for the review! I've addressed your suggestions in my last commit!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
indexManagement 686.9KB 686.9KB +18.0B

History

cc @SoniaSanzV

Copy link
Contributor

@ElenaStoeva ElenaStoeva 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 the last changes!

@SoniaSanzV SoniaSanzV merged commit 1abc780 into elastic:main Aug 29, 2025
12 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.18, 8.19, 9.0, 9.1

https://github.com/elastic/kibana/actions/runs/17319957952

@SoniaSanzV SoniaSanzV deleted the kbn_mngmt_test/index_management_tests_cleanup branch August 29, 2025 09:21
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 29, 2025
…tly to the tab (elastic#232198)

## Summary
Space time project. I'm trying to improve the test for Kibana
Management. In this PR, I did two things:
* Clean up all created resources after execution. This suite contained a
lot of resources that wasn't deleted after execution. That complicates
local testing and leads to failures if you re-run the tests.
* Navigates directly to the desired tab when setting up the tests. This
is not an anti-patter, but it was time consuming. We were doing this:
```
      await pageObjects.common.navigateToApp('indexManagement');
      // Navigate to the data streams tab
      await pageObjects.indexManagement.changeTabs('data_streamsTab');
      await pageObjects.header.waitUntilLoadingHasFinished();
```

But the `navigateToApp` method already contains a path parameter, so
doing it this way, we can get rid of unnecessaries
`waitUntilLoadingHasFinished` calls.
With this simple change, the execution time of serverless tests locally
dropped from 2 minutes to 1 minute.

* Also, changes the data-test-subj for creating a component template
from `createPipelineButton` to `createComponentTemplateButton`, that
wasn't reflecting what the button really does.

Flay test runner stateful tests:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/9177
Flay test runner serverless tests:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/9182

(cherry picked from commit 1abc780)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 29, 2025
…tly to the tab (elastic#232198)

## Summary
Space time project. I'm trying to improve the test for Kibana
Management. In this PR, I did two things:
* Clean up all created resources after execution. This suite contained a
lot of resources that wasn't deleted after execution. That complicates
local testing and leads to failures if you re-run the tests.
* Navigates directly to the desired tab when setting up the tests. This
is not an anti-patter, but it was time consuming. We were doing this:
```
      await pageObjects.common.navigateToApp('indexManagement');
      // Navigate to the data streams tab
      await pageObjects.indexManagement.changeTabs('data_streamsTab');
      await pageObjects.header.waitUntilLoadingHasFinished();
```

But the `navigateToApp` method already contains a path parameter, so
doing it this way, we can get rid of unnecessaries
`waitUntilLoadingHasFinished` calls.
With this simple change, the execution time of serverless tests locally
dropped from 2 minutes to 1 minute.

* Also, changes the data-test-subj for creating a component template
from `createPipelineButton` to `createComponentTemplateButton`, that
wasn't reflecting what the button really does.

Flay test runner stateful tests:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/9177
Flay test runner serverless tests:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/9182

(cherry picked from commit 1abc780)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
8.18 Backport failed because of merge conflicts
8.19
9.0 Backport failed because of merge conflicts

You might need to backport the following PRs to 9.0:
- [Docs] Adds security advisory note to release notes (#233347)
9.1

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 232198

Questions ?

Please refer to the Backport tool documentation

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

12 similar comments
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 232198 locally
cc: @SoniaSanzV

4 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 232198 locally
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 232198 locally
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 232198 locally
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 232198 locally
cc: @SoniaSanzV

@SoniaSanzV SoniaSanzV removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 2, 2025
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 3, 2025
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 232198 locally
cc: @SoniaSanzV

4 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 232198 locally
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 232198 locally
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 232198 locally
cc: @SoniaSanzV

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 232198 locally
cc: @SoniaSanzV

@SoniaSanzV SoniaSanzV added backport:skip This PR does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. backport:version Backport to applied version labels labels Oct 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages 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 t// v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants