[IM] Remove `axios` dependency in tests by sabarasaba · Pull Request #128171 · elastic/kibana

sabarasaba · 2022-03-21T15:22:42Z

Partially addresses #127966

Summary

In order to upgrade the axios dependency in kibana(see: #111655) we need to switch the tests from this app to use HttpSetup mock instead of axios + sinon server mock, which is technically incorrect but axios just happened to have methods signature similar to HttpSetup.

Note: There's a bunch of CIT's for component_templates that live in x-pack/plugins/index_management/public/application/components/component_templates/__jest__, ideally the setup_environment.ts and http_request.ts files could be refactored into just one. I've created an issue for dealing with that #128376

sabarasaba · 2022-03-23T13:59:10Z

x-pack/plugins/index_management/__jest__/client_integration/helpers/http_requests.ts

-    const status = error ? error.status || 400 : 200;
-    const body = error ? error.body : response;
-
-    server.respondWith('GET', `${API_BASE_PATH}/index_templates/:id`, [


With sinon fake server we could use wildcards for url params, that is no longer allowed with HttpSetup mock. So we need to directly specify the full url when mocking api endpoints. For this particular case would look something like:

const setLoadTemplateResponse = ( templateId: string, response?: HttpResponse, error?: ResponseError ) => mockResponse('GET', `${API_BASE_PATH}/index_templates/${templateId}`, response, error);

elasticmachine · 2022-03-23T16:40:33Z

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

alisonelizabeth · 2022-03-23T23:12:17Z

@elasticmachine merge upstream

kibanamachine · 2022-03-23T23:12:29Z

expected head sha didn’t match current head ref.

alisonelizabeth

Great work with this @sabarasaba! That was a lot of tests 😅 . I left a few comments. Let me know what you think.

alisonelizabeth · 2022-03-23T23:16:07Z

x-pack/plugins/index_management/__jest__/client_integration/helpers/http_requests.ts

-      JSON.stringify(body),
-    ]);
-  };
+const registerHttpRequestMockHelpers = (


Starting here to L56 seems to be the same code across the test refactoring PRs. Does it make sense to try to centralize it somewhere so it can be reused?

If you agree, I think this can be done as a separate PR

I've suggested also to put this somewhere in kbn-test (see this thread), but we ended up agreeing on keeping separate implementations per app for now. I'll create a new issue after on our side to update this later on after I finish updating all the tests.

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

alisonelizabeth · 2022-03-23T23:23:54Z

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

      const { find, exists, actions, component } = testBed;

      // Composable templates
+      httpRequestsMockHelpers.setLoadTemplateResponse(templates[0].name, templates[0]);


can you explain why this wasn't needed previously?

When an index flyout is opened a new api request for getting details for that index is called. With the sinon route for mocking the setLoadTemplateResponse we had a wildcard that would load the same payload for any templateId. So the top level mock we have set up at the beginning of the describe was working just fine. But since we now have to directly specify for each templateId, we need to specify each request.

👍 got it. Makes sense.

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

alisonelizabeth · 2022-03-23T23:31:04Z

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

+
+      component.update();
+
+      find('indexTableIndexNameLink').at(0).simulate('click');


can you explain why this wasn't needed before?

Since the testBed instance does't get re-initialized in a top levelbeforeEach, this test was wrongly depending on the testBed instance that is initialized in the previous describe. So we need to specifically declare it in its own describe scope.

...s/index_management/__jest__/client_integration/index_template_wizard/template_clone.test.tsx

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

sabarasaba · 2022-03-24T09:12:25Z

Thanks for the review @alisonelizabeth, I know it was a lot of things! I think i've addressed your feedback, let me know what you think :)

alisonelizabeth

Latest LGTM. Awesome work!

sabarasaba · 2022-03-24T12:34:49Z

@elasticmachine merge upstream

kibana-ci · 2022-03-24T13:59:36Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💛 Build #33379 was flaky feac4c0
💔 Build #33312 failed ea7e7c5
💚 Build #33047 succeeded 4bdcb26
💔 Build #33030 failed 455a328
💔 Build #32637 failed ba71517
💔 Build #32620 failed 8afc145

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

cc @sabarasaba

(cherry picked from commit 00f58ad) # Conflicts: # x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/home/home.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/home/index_templates_tab.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/home/indices_tab.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/home/indices_tab.test.ts # x-pack/plugins/index_management/__jest__/client_integration/index_template_wizard/template_clone.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/index_template_wizard/template_create.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/index_template_wizard/template_edit.helpers.ts # x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/helpers/component_template_create.helpers.ts # x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/helpers/component_template_details.helpers.ts # x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/helpers/component_template_edit.helpers.ts

* [IM] Remove `axios` dependency in tests (#128171) (cherry picked from commit 00f58ad) # Conflicts: # x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/home/home.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/home/index_templates_tab.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/home/indices_tab.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/home/indices_tab.test.ts # x-pack/plugins/index_management/__jest__/client_integration/index_template_wizard/template_clone.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/index_template_wizard/template_create.helpers.ts # x-pack/plugins/index_management/__jest__/client_integration/index_template_wizard/template_edit.helpers.ts # x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/helpers/component_template_create.helpers.ts # x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/helpers/component_template_details.helpers.ts # x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/helpers/component_template_edit.helpers.ts * Fix linter issues * Fix broken tests

wip: remove axios and start setup refactoring

809c581

sabarasaba self-assigned this Mar 21, 2022

sabarasaba mentioned this pull request Mar 21, 2022

Get rid of axios dependency in the platform deployment management tests. #127966

Closed

7 tasks

sabarasaba added 5 commits March 22, 2022 10:53

fix more tests

ec80ba5

Fix index templates tab tests

dd405c1

Fix last issues with ds tests

ae9c3c9

Start refactoring index_template_wizard

8afc145

wip working on template create tests

ba71517

azasypkin mentioned this pull request Mar 22, 2022

Upgrade axios dependency (0.21.1 → 0.27.2). #111655

Merged

sabarasaba and others added 4 commits March 23, 2022 09:49

commit using @elastic.co

eee9c7c

Finish fixing and refactoring all the tests

500a75e

Finish refactoring rest of all tests

e8a291c

Finx linter issues

455a328

sabarasaba commented Mar 23, 2022

View reviewed changes

Remove unused imports

4bdcb26

sabarasaba marked this pull request as ready for review March 23, 2022 16:05

sabarasaba requested a review from a team as a code owner March 23, 2022 16:05

sabarasaba requested a review from alisonelizabeth March 23, 2022 16:05

Merge branch 'main' into im/remove_axios

ea7e7c5

alisonelizabeth reviewed Mar 23, 2022

View reviewed changes

Address CR changes

feac4c0

alisonelizabeth approved these changes Mar 24, 2022

View reviewed changes

Merge branch 'main' into im/remove_axios

73d3108

sabarasaba merged commit 00f58ad into elastic:main Mar 24, 2022

sabarasaba removed the auto-backport Deprecated - use backport:version if exact versions are needed label Mar 24, 2022

sabarasaba mentioned this pull request Mar 28, 2022

[7.17] [IM] Remove axios dependency in tests (#128171) #128586

Merged

sabarasaba mentioned this pull request Apr 13, 2022

[IndexManagement - IngestPipelines] Remove leftover axios dependencies #130089

Merged


		component.update();

		find('indexTableIndexNameLink').at(0).simulate('click');

Conversation

sabarasaba commented Mar 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Mar 23, 2022

Uh oh!

alisonelizabeth commented Mar 23, 2022

Uh oh!

kibanamachine commented Mar 23, 2022

Uh oh!

alisonelizabeth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sabarasaba Mar 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sabarasaba commented Mar 24, 2022

Uh oh!

alisonelizabeth left a comment

Choose a reason for hiding this comment

Uh oh!

sabarasaba commented Mar 24, 2022

Uh oh!

kibana-ci commented Mar 24, 2022

💚 Build Succeeded

Metrics [docs]

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sabarasaba commented Mar 21, 2022 •

edited

Loading

sabarasaba Mar 24, 2022 •

edited

Loading