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

[Snapshot & Restore] Remove axios dependency in tests #128614

Merged
merged 9 commits into from
Apr 4, 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.

@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:Snapshot and Restore Elasticsearch snapshots and repositories UI auto-backport Deprecated - use backport:version if exact versions are needed v8.2.0 labels Mar 28, 2022
@sabarasaba sabarasaba self-assigned this Mar 28, 2022
initialEntries: ['/add_policy'],
componentRoutePath: '/add_policy',
},
doMountAsync: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

This was previously mounting the component on the wrong route, but since we had a wildcard api mock with sinon things were working anyway.

@@ -805,8 +812,12 @@ describe('<SnapshotRestoreHome />', () => {

test('should display a message when snapshot in progress ', async () => {
const { find, actions } = testBed;
const updatedSnapshot = { ...snapshot1, state: 'IN_PROGRESS' };
httpRequestsMockHelpers.setGetSnapshotResponse(updatedSnapshot);
const updatedSnapshot = { ...snapshot2, state: 'IN_PROGRESS' };
Copy link
Member Author

Choose a reason for hiding this comment

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

Was previously using the wrong snapshot mock but since the api call had a wildcard it was working fine.

@sabarasaba sabarasaba marked this pull request as ready for review March 29, 2022 14:01
@sabarasaba sabarasaba requested a review from a team as a code owner March 29, 2022 14:01
@elasticmachine
Copy link
Contributor

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

@sabarasaba sabarasaba added v8.3.0 and removed v8.2.0 labels Mar 29, 2022
@sabarasaba
Copy link
Member Author

@elasticmachine merge upstream

@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
Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Great work @sabarasaba! LGTM.

export const WithAppDependencies = (Comp: any) => (props: any) =>
(
export const WithAppDependencies = (Comp: any, httpSetup?: HttpSetup) => (props: any) => {
// We need to optionally setup the httpService since some cit helpers (such as snapshot_list.helpers)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 thanks for adding this comment!

@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 fa74030 into elastic:main Apr 4, 2022
sabarasaba added a commit to sabarasaba/kibana that referenced this pull request Apr 4, 2022
* wip: start refactoring test helpers

* commit using @elastic.co

* Fix more tests

* Finish off refactoring all cits

* Add docs

* Fix linter issues

* Fix ts issues

(cherry picked from commit fa74030)

# Conflicts:
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/home.helpers.ts
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/policy_add.helpers.ts
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/policy_edit.helpers.ts
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/repository_add.helpers.ts
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/repository_edit.helpers.ts
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/restore_snapshot.helpers.ts
sabarasaba added a commit that referenced this pull request Apr 4, 2022
…29343)

* wip: start refactoring test helpers

* commit using @elastic.co

* Fix more tests

* Finish off refactoring all cits

* Add docs

* Fix linter issues

* Fix ts issues

(cherry picked from commit fa74030)

# Conflicts:
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/home.helpers.ts
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/policy_add.helpers.ts
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/policy_edit.helpers.ts
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/repository_add.helpers.ts
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/repository_edit.helpers.ts
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/restore_snapshot.helpers.ts
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:Snapshot and Restore Elasticsearch snapshots and repositories UI 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