Added Close Index Component Integration Test For Index Management #114020

cuff-links · 2021-10-05T21:46:41Z

Summary

This PR adds a component integration test that checks that we are able to click the close index button and check the last request to ensure that it was made to the close index API.

cuff-links · 2021-10-07T04:49:26Z

@elasticmachine merge upstream

…inks/kibana into Add_CIT_For_Close_Index_Operation

elasticmachine · 2021-10-08T00:51:58Z

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

cuff-links · 2021-10-12T01:19:37Z

@elasticmachine merge upstream

sabarasaba

Thanks for working on this @cuff-links! Changes LGTM, just had a suggestions for simplifying the button selector and maybe abstracting the clicking logic into a new action:

sabarasaba · 2021-10-12T08:11:43Z

x-pack/plugins/index_management/__jest__/client_integration/home/indices_tab.test.ts

+      const closeIndexButton = contextMenu
+        .find('button[data-test-subj="indexTableContextMenuButton"]')
+        .at(0);


I think we could simplify this a little bit by just using a selector like:

Suggested change

const closeIndexButton = contextMenu

.find('button[data-test-subj="indexTableContextMenuButton"]')

.at(0);

const closeIndexButton = find('indexContextMenu.indexTableContextMenuButton').at(0);

Also, what do you think about abstracting this into another action that allows us to click a context menu option like:

enum ContextMenuOptions { ToggleIndex = 0, RefreshIndex = 2, ClearIndexCache = 3, FlushIndex = 4, } const clickContextMenuOption = async (option: ContextMenuOption) => { const { find } = testBed; await act(async () => { find('indexContextMenu.indexTableContextMenuButton').at(option).simulate('click'); }); };

And then in the test we could just do:

await clickContextMenuOption(ContextMenuOptions.ToggleIndex);

yuliacech

Great work adding those tests, @cuff-links! I think the best way to solve for selecting the correct button is to add specific data-test-subj values in this file. For example for "close index" button:

items.push({
        "data-test-subj": 'closeIndexButton',
        name: i18n.translate('xpack.idxMgmt.indexActionsMenu.closeIndexLabel', {
          defaultMessage: 'Close {selectedIndexCount, plural, one {index} other {indices} }',
          values: { selectedIndexCount },
        }),
        onClick: () => {
          if (hasSystemIndex) {
            this.closePopover();
            this.setState({ renderConfirmModal: this.renderConfirmCloseModal });
            return;
          }
          this.closePopoverAndExecute(closeIndices);
        },
      });

That would make other tests easier to work with too. WDYT?

cuff-links · 2021-10-13T04:21:02Z

Great work adding those tests, @cuff-links! I think the best way to solve for selecting the correct button is to add specific data-test-subj values in this file. For example for "close index" button:

items.push({
        "data-test-subj": 'closeIndexButton',
        name: i18n.translate('xpack.idxMgmt.indexActionsMenu.closeIndexLabel', {
          defaultMessage: 'Close {selectedIndexCount, plural, one {index} other {indices} }',
          values: { selectedIndexCount },
        }),
        onClick: () => {
          if (hasSystemIndex) {
            this.closePopover();
            this.setState({ renderConfirmModal: this.renderConfirmCloseModal });
            return;
          }
          this.closePopoverAndExecute(closeIndices);
        },
      });

That would make other tests easier to work with too. WDYT?

I tried applying that data test subject but it did not get added to the element. The only time that the data test subject got added was if you change line 244. But that breaks several of the tests. I think perhaps handling it the way @sabarasaba mentioned may work well.

…p test.

sabarasaba

Latest LGTM @cuff-links! I left just one small nit 🚀

sabarasaba · 2021-10-13T07:34:43Z

x-pack/plugins/index_management/__jest__/client_integration/home/indices_tab.helpers.ts

+    const contextMenu = find('indexContextMenu');
+    contextMenu
+      .find('button[data-test-subj="indexTableContextMenuButton"]')
+      .at(option)
+      .simulate('click');


nit: I think we can still simplify this like:

Suggested change

const contextMenu = find('indexContextMenu');

contextMenu

.find('button[data-test-subj="indexTableContextMenuButton"]')

.at(option)

.simulate('click');

find('indexContextMenu.indexTableContextMenuButton').at(option).simulate('click');

yuliacech · 2021-10-13T15:00:34Z

I tried applying that data test subject but it did not get added to the element. The only time that the data test subject got added was if you change line 244. But that breaks several of the tests. I think perhaps handling it the way @sabarasaba mentioned may work well.

Hi @cuff-links, thanks for considering my suggestions!
Yes, the code on line 244 needs to be deleted as it overwrites any data-test-subj that are added initially. Which tests get broken when specific values are added? Maybe those tests will also benefit from using the specific value instead of relying on the order? For example, the current solution won't work when testing for opening an index, because a closed index has other actions allowed than a closed one. The tests could also break if any new actions are added, that would disturb the order. Maybe we could have a look at the code together over zoom?

yuliacech · 2021-10-13T19:08:25Z

Hi @cuff-links, I was thinking about your comment that some tests get broken with the value of data-test-subj changes and checked those. I think the code quality could benefit if these tests are easier to maintain. I opened a PR to help out and fix those. Let me know what you think!

…lose_Index_Operation

…he new data test subjects.

cuff-links · 2021-10-15T01:33:39Z

Unrelated test failure. Gonna have jenkins retest.

cuff-links · 2021-10-15T01:33:50Z

jenkins test this

cuff-links · 2021-10-15T05:30:03Z

@yuliacech Updated with your PR's changes. Think we are good to go.

…lose_Index_Operation

kibanamachine · 2021-10-16T00:19:32Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 5a00225
Storybooks Preview
Pipeline Steps (look for red circles / failed steps)
Interpreting CI Failures

Failed CI Steps

Jest Unit Tests

Test Failures

Kibana Pipeline / jest / Jest Tests.x-pack/plugins/index_management/__jest__/client_integration/home. index actions from detail panel should be able to flush index

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

Stack Trace

TypeError: indices.forEach is not a function
    at INDEX_MANAGEMENT_RELOAD_INDICES_SUCCESS (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/index_management/public/application/store/reducers/indices.js:45:15)
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/redux-actions/lib/handleAction.js:50:64
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/redux-actions/node_modules/reduce-reducers/lib/index.js:32:22
    at Array.reduce (<anonymous>)
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/redux-actions/node_modules/reduce-reducers/lib/index.js:31:21
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/redux-actions/lib/handleActions.js:42:12
    at combination (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/redux/lib/redux.js:536:29)
    at combination (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/redux/lib/redux.js:536:29)
    at dispatch (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/redux/lib/redux.js:296:22)
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/redux-thunk/lib/index.js:14:16
    at dispatch (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/redux/lib/redux.js:667:28)
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/index_management/public/application/store/actions/reload_indices.js:29:12

Metrics [docs]

✅ unchanged

History

💚 Build #161227 succeeded 0e41fdf
💔 Build #161185 failed 0e41fdf
💚 Build #160308 succeeded 7644081
💚 Build #159804 succeeded d6d9416

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

yuliacech

Thanks for updating this, @cuff-links! code changes LGTM 👍

… a refresh done after the close index call.

kibana-ci · 2021-12-10T10:34:14Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💔 Build #12659 failed c9ab398
💔 Build #12656 failed e80fc6a

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

kibanamachine · 2021-12-10T21:51:31Z

The following labels were identified as gaps in your version labels and will be added automatically:

v8.1.0

If any of these should not be on your pull request, please manually remove them.

…astic#114020) * Added close index test. * Fixed linting issues. * Fixed linting issues. * Abstracted out the index action option selection method and cleaned up test. * Merged Yulia's changes into this PR and updated the test to consume the new data test subjects. * Adjusted assertion to check for second to last request since there is a refresh done after the close index call. * Fixed linting issue. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-12-10T21:54:09Z

💔 Backport failed

Status	Branch	Result
✅	8.0
❌	7.15	Commit could not be cherrypicked due to conflicts

Successful backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 114020

…14020) (#121043) * Added close index test. * Fixed linting issues. * Fixed linting issues. * Abstracted out the index action option selection method and cleaned up test. * Merged Yulia's changes into this PR and updated the test to consume the new data test subjects. * Adjusted assertion to check for second to last request since there is a refresh done after the close index call. * Fixed linting issue. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: John Dorlus <silne.dorlus@elastic.co>

…astic#114020) * Added close index test. * Fixed linting issues. * Fixed linting issues. * Abstracted out the index action option selection method and cleaned up test. * Merged Yulia's changes into this PR and updated the test to consume the new data test subjects. * Adjusted assertion to check for second to last request since there is a refresh done after the close index call. * Fixed linting issue. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…14020) (#121722) * Added close index test. * Fixed linting issues. * Fixed linting issues. * Abstracted out the index action option selection method and cleaned up test. * Merged Yulia's changes into this PR and updated the test to consume the new data test subjects. * Adjusted assertion to check for second to last request since there is a refresh done after the close index call. * Fixed linting issue. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…astic#114020) * Added close index test. * Fixed linting issues. * Fixed linting issues. * Abstracted out the index action option selection method and cleaned up test. * Merged Yulia's changes into this PR and updated the test to consume the new data test subjects. * Adjusted assertion to check for second to last request since there is a refresh done after the close index call. * Fixed linting issue. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Added close index test.

bf09de0

cuff-links requested review from cjcenizal and removed request for cjcenizal October 5, 2021 21:46

cuff-links changed the title ~~Added Close Index CIT test~~ Added Close Index Component Integration Test For Index Management Oct 5, 2021

Fixed linting issues.

feb936b

kibanamachine and others added 3 commits October 7, 2021 00:49

Merge branch 'master' into Add_CIT_For_Close_Index_Operation

b0e0a5d

Fixed linting issues.

3cb39c7

Merge branch 'Add_CIT_For_Close_Index_Operation' of github.com:cuff-l…

a616bbd

…inks/kibana into Add_CIT_For_Close_Index_Operation

cuff-links marked this pull request as ready for review October 8, 2021 00:51

cuff-links requested a review from a team as a code owner October 8, 2021 00:51

cuff-links requested review from yuliacech, sabarasaba and sebelga October 9, 2021 22:23

Merge branch 'master' into Add_CIT_For_Close_Index_Operation

d6d9416

sabarasaba reviewed Oct 12, 2021

View reviewed changes

yuliacech suggested changes Oct 12, 2021

View reviewed changes

Abstracted out the index action option selection method and cleaned u…

7644081

…p test.

cuff-links requested a review from sabarasaba October 13, 2021 05:27

sabarasaba approved these changes Oct 13, 2021

View reviewed changes

yuliacech mentioned this pull request Oct 13, 2021

[Index Management] Added specific data-test-subj values to index context menu buttons #114900

Merged

cuff-links added 2 commits October 14, 2021 16:53

Merge branch 'master' of github.com:elastic/kibana into Add_CIT_For_C…

f6867d7

…lose_Index_Operation

Merged Yulia's changes into this PR and updated the test to consume t…

0e41fdf

…he new data test subjects.

cuff-links requested a review from yuliacech October 15, 2021 05:29

cuff-links added 2 commits October 15, 2021 15:15

Merge branch 'master' of github.com:elastic/kibana into Add_CIT_For_C…

342f23f

…lose_Index_Operation

Merge branch 'master' of github.com:elastic/kibana into Add_CIT_For_C…

5a00225

…lose_Index_Operation

yuliacech approved these changes Oct 18, 2021

View reviewed changes

John Dorlus and others added 3 commits December 9, 2021 22:22

Merge branch 'main' into Add_CIT_For_Close_Index_Operation

e80fc6a

Adjusted assertion to check for second to last request since there is…

c9ab398

… a refresh done after the close index call.

Fixed linting issue.

1758606

cuff-links merged commit 3129b98 into elastic:main Dec 10, 2021

kibanamachine added the v8.1.0 label Dec 10, 2021

kibanamachine mentioned this pull request Dec 10, 2021

[8.0] Added Close Index Component Integration Test For Index Management (#114020) #121043

Merged

cuff-links mentioned this pull request Dec 21, 2021

[7.17] Added Close Index Component Integration Test For Index Management (#114020) #121722

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Close Index Component Integration Test For Index Management #114020

Added Close Index Component Integration Test For Index Management #114020

cuff-links commented Oct 5, 2021

cuff-links commented Oct 7, 2021

elasticmachine commented Oct 8, 2021

cuff-links commented Oct 12, 2021

sabarasaba left a comment

sabarasaba Oct 12, 2021

yuliacech left a comment •

edited

Loading

cuff-links commented Oct 13, 2021

sabarasaba left a comment

sabarasaba Oct 13, 2021

yuliacech commented Oct 13, 2021

yuliacech commented Oct 13, 2021

cuff-links commented Oct 15, 2021

cuff-links commented Oct 15, 2021

cuff-links commented Oct 15, 2021

kibanamachine commented Oct 16, 2021 •

edited

Loading

Standard Out

Stack Trace

yuliacech left a comment

kibana-ci commented Dec 10, 2021

kibanamachine commented Dec 10, 2021

kibanamachine commented Dec 10, 2021

Added Close Index Component Integration Test For Index Management #114020

Added Close Index Component Integration Test For Index Management #114020

Conversation

cuff-links commented Oct 5, 2021

Summary

cuff-links commented Oct 7, 2021

elasticmachine commented Oct 8, 2021

cuff-links commented Oct 12, 2021

sabarasaba left a comment

Choose a reason for hiding this comment

sabarasaba Oct 12, 2021

Choose a reason for hiding this comment

yuliacech left a comment • edited Loading

Choose a reason for hiding this comment

cuff-links commented Oct 13, 2021

sabarasaba left a comment

Choose a reason for hiding this comment

sabarasaba Oct 13, 2021

Choose a reason for hiding this comment

yuliacech commented Oct 13, 2021

yuliacech commented Oct 13, 2021

cuff-links commented Oct 15, 2021

cuff-links commented Oct 15, 2021

cuff-links commented Oct 15, 2021

kibanamachine commented Oct 16, 2021 • edited Loading

💔 Build Failed

Failed CI Steps

Test Failures

Standard Out

Stack Trace

Metrics [docs]

History

yuliacech left a comment

Choose a reason for hiding this comment

kibana-ci commented Dec 10, 2021

💚 Build Succeeded

Metrics [docs]

History

kibanamachine commented Dec 10, 2021

kibanamachine commented Dec 10, 2021

💔 Backport failed

yuliacech left a comment •

edited

Loading

kibanamachine commented Oct 16, 2021 •

edited

Loading