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] Remove axios dependency in tests #128467

Merged
merged 10 commits into from
Mar 30, 2022

Conversation

sabarasaba
Copy link
Member

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 pipeline_editor that live in x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/__jest__, ideally the setup_environment.ts and http_request.ts files could be refactored into just one. I'll create an issue for dealing with that

@sabarasaba sabarasaba 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 auto-backport Deprecated - use backport:version if exact versions are needed v8.2.0 v7.17.2 labels Mar 24, 2022
@sabarasaba sabarasaba self-assigned this Mar 24, 2022
@sabarasaba
Copy link
Member Author

@elasticmachine merge upstream

@sabarasaba sabarasaba requested a review from yuliacech March 28, 2022 08:06
@sabarasaba sabarasaba marked this pull request as ready for review March 28, 2022 08:06
@sabarasaba sabarasaba requested a review from a team as a code owner March 28, 2022 08:06
@elasticmachine
Copy link
Contributor

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

'/api/ingest_pipelines/simulate'
expect(httpSetup.post).toHaveBeenLastCalledWith(
`${API_BASE_PATH}/simulate`,
expect.anything()
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I don't think that would test for 2 different calls to the simulate API. You might want to use toHaveBeenCalledTimes or toHaveBeenNthCalledWith

Copy link
Member Author

Choose a reason for hiding this comment

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

nice catch!, ill patch it up!

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thanks a lot for updating these tests, @sabarasaba! Changes LGTM 👍
There is quite some boilerplate for mock setup but I've seen your discussion about keeping that implementation in each plugin separately for now.

@sabarasaba
Copy link
Member Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @sabarasaba

@sabarasaba sabarasaba merged commit d869a7f into elastic:main Mar 30, 2022
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.3 The branch "8.3" is invalid or doesn't exist

Manual backport

To create the backport manually run:

node scripts/backport --pr 128467

Questions ?

Please refer to the Backport tool documentation

sabarasaba added a commit to sabarasaba/kibana that referenced this pull request Mar 30, 2022
* Refactor main cits

* commit using @elastic.co

* Finish refactoring pipeline_editor cits

* Carefully access prop

* Fix hardcoded props

* Fix ts issues

* Add back missing attr

* Address CR changes

(cherry picked from commit d869a7f)

# Conflicts:
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/helpers/http_requests.ts
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/helpers/pipelines_clone.helpers.ts
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/helpers/pipelines_create.helpers.ts
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/helpers/pipelines_create_from_csv.helpers.ts
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/helpers/pipelines_edit.helpers.ts
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/ingest_pipelines_create_from_csv.test.tsx
@sabarasaba sabarasaba added backport:skip This commit does not require backporting and removed auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 30, 2022
sabarasaba added a commit that referenced this pull request Mar 30, 2022
)

* Refactor main cits

* commit using @elastic.co

* Finish refactoring pipeline_editor cits

* Carefully access prop

* Fix hardcoded props

* Fix ts issues

* Add back missing attr

* Address CR changes

(cherry picked from commit d869a7f)

# Conflicts:
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/helpers/http_requests.ts
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/helpers/pipelines_clone.helpers.ts
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/helpers/pipelines_create.helpers.ts
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/helpers/pipelines_create_from_csv.helpers.ts
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/helpers/pipelines_edit.helpers.ts
#	x-pack/plugins/ingest_pipelines/__jest__/client_integration/ingest_pipelines_create_from_csv.test.tsx
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.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants