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

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

Closed
7 tasks done
azasypkin opened this issue Mar 17, 2022 · 5 comments
Closed
7 tasks done
Assignees
Labels
NeededFor:Operations NeededFor:Security Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more technical debt Improvement of the software architecture and operational architecture triage_needed

Comments

@azasypkin
Copy link
Member

azasypkin commented Mar 17, 2022

Summary

In #111655 (comment) we're trying to upgrade axios package, but are blocked because of the following axios usage in the tests (axios just happened to have methods signature similar to HttpSetup, but it's technically incorrect and quite fragile):

🚫 const httpSetup = axios.create({ adapter: axiosXhrAdapter }) as unknown as HttpSetup; 🚫

The path forward is to switch tests to use HttpSetup mock instead of axios + sinon server mock. There is an example and reference implementation in the Upgrade Assistant tests, see this pull request.

If you have any questions or don't have enough capacity to update the tests, please ping me and I'll try to help out.

Here is the list of the plugins that need to be updated (if I didn't miss anything):

IMPORTANT: we have to backport axios upgrade to 7.17.x and hence updated tests should be backported to the 7.17 branch as well.

@azasypkin azasypkin added technical debt Improvement of the software architecture and operational architecture Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more NeededFor:Security NeededFor:Operations labels Mar 17, 2022
@elasticmachine
Copy link
Contributor

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

@cjcenizal
Copy link
Contributor

@azasypkin Thank you for raising this! The team will be able to get to this next week.

@azasypkin
Copy link
Member Author

@azasypkin Thank you for raising this! The team will be able to get to this next week.

Awesome, thank you ❤️ !

@sabarasaba
Copy link
Member

This has all been done now and also backported to 7.17.

@azasypkin
Copy link
Member Author

Thanks a lot, @sabarasaba! It looks like all tests in Kibana are green with the latest axios version 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeededFor:Operations NeededFor:Security Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more technical debt Improvement of the software architecture and operational architecture triage_needed
Projects
None yet
Development

No branches or pull requests

4 participants