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

[Archive Migration] dashboard/current/kibana #126293

Merged
merged 23 commits into from
Mar 9, 2022

Conversation

LeeDr
Copy link

@LeeDr LeeDr commented Feb 23, 2022

Summary

This PR replaces test/functional/fixtures/es_archiver/dashboard/current/kibana with test/functional/fixtures/kbn_archiver/dashboard/current/kibana which impacts quite a few tests.

A couple of interesting notes about this PR and the original es_archive.

  1. The original es_archive included some visualizations which referenced index patterns which were purposely removed for some tests. To create the kbn_archive, I load the es_archive and then save the kbn_archive. But this new kbn_archive won't load because loading saved objects checks references and 2 are missing. So I had to add 2 index patterns with the ids of the missing references, and then create another kbn_archive with only those 2 index patterns so some tests could unload them. I tried to only unload those in the tests which relied on them being missing. I also added a comment in those places.
  2. I want to backport these kbn_archive migration PRs back to 7.17 so I originally created it from that version. But it turns out that there was another change to this es_archive between 7.17 and 8.0. So my last commit of the kbn_archive was created from 8.0 instead, and I'll have to modify the 7.17 backport PR to use the right one.
  3. Cleaning - while loading an es_archive to the .kibana index would wipe out all existing Saved Objects, kibanaServer.importExport.load doesn't do that. So to eliminate interference from prior tests I added the kibanaServer.savedObjects.clean({ types: ['search', 'index-pattern', 'visualization', 'dashboard'], }); before the load, and in the after method (I had to add an after method to some tests). There really are a few other types that can. and perhaps should, be cleaned such as lens and maps. Maybe we need a cleanAll method to simplify this.

@LeeDr LeeDr changed the title Kbn archive dashboard [Archive Migration] dashboard/current/kibana Feb 23, 2022
@LeeDr LeeDr added Feature:Dashboard Dashboard related features v7.17.2 v8.0.2 v8.1.0 v8.2.0 Feature:Functional Testing test_ui_functional test_xpack_functional Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes labels Mar 3, 2022
@LeeDr
Copy link
Author

LeeDr commented Mar 4, 2022

@elasticmachine merge upstream

@LeeDr LeeDr marked this pull request as ready for review March 4, 2022 17:09
spalger
spalger previously requested changes Mar 4, 2022
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

I would greatly prefer that we don't try to define a "standard list" of types to cleanup in the kbn_client, or that we have a kbnArchive that is just for unloading. I'd like to propose that instead we create a "data scenarios" service for dashboard which has these defaults for the dashboard tests. I've started a PR showing what I mean here: LeeDr#4

@spalger spalger dismissed their stale review March 4, 2022 19:35

Discussed with @LeeDr and we should stick with what we have for now

@LeeDr
Copy link
Author

LeeDr commented Mar 7, 2022

@elasticmachine merge upstream

Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Looked through the code changes and everything here makes sense. I especially appreciated your explanations of the cleanStandardList function, and the explanations of the two removed index patterns.

Thank you for doing this! Changes LGTM

Lee Drengenberg and others added 2 commits March 8, 2022 10:49
@LeeDr
Copy link
Author

LeeDr commented Mar 8, 2022

@elasticmachine merge upstream

@LeeDr
Copy link
Author

LeeDr commented Mar 9, 2022

I'm not sure what happened on that last run?


You can still download the full log using the "Download" button above.
--
  | ... |  
  | $ node scripts/jest --config src/plugins/kibana_usage_collection/jest.integration.config.js | 18s
  | $ node scripts/jest --config x-pack/plugins/fleet/jest.integration.config.js | 15m 8s
  | $ node scripts/jest --config x-pack/plugins/reporting/jest.integration.config.js | 35s
  | # Received cancellation signal, interrupting
  | 🚨 Error: The command was interrupted by a signal

@LeeDr
Copy link
Author

LeeDr commented Mar 9, 2022

@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

@LeeDr LeeDr merged commit e932d83 into elastic:main Mar 9, 2022
kibanamachine pushed a commit that referenced this pull request Mar 9, 2022
* switch from es_archive to kbn_archive

* another test conversion

* add the kbn_archive

* remove unused esArchiver

* kbn_archive to full replace es_archiver/dashboard/current/kibana

* finish this test

* to fix this test we have to unload 2 index patterns

* had to re-make the kbn_archive from 8.0 instead of 7.17

* cleanup saved objects in before method

* remove unused esArchiver

* remove unused dashboard/current/kibana es_archiver files

* refactor clean to cleanStandardList

* a few more tests using the es_archive

* cleanup and uncomment smoketest

* Apply suggestions from code review

Co-authored-by: Spencer <[email protected]>

* update for code review

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Spencer <[email protected]>
(cherry picked from commit e932d83)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
8.1
7.17 Backport failed because of merge conflicts
8.0 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 126293

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Mar 9, 2022
* switch from es_archive to kbn_archive

* another test conversion

* add the kbn_archive

* remove unused esArchiver

* kbn_archive to full replace es_archiver/dashboard/current/kibana

* finish this test

* to fix this test we have to unload 2 index patterns

* had to re-make the kbn_archive from 8.0 instead of 7.17

* cleanup saved objects in before method

* remove unused esArchiver

* remove unused dashboard/current/kibana es_archiver files

* refactor clean to cleanStandardList

* a few more tests using the es_archive

* cleanup and uncomment smoketest

* Apply suggestions from code review

Co-authored-by: Spencer <[email protected]>

* update for code review

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Spencer <[email protected]>
(cherry picked from commit e932d83)

Co-authored-by: Lee Drengenberg <[email protected]>
LeeDr pushed a commit that referenced this pull request Apr 5, 2022
#128102)

* reimplement backport

* dashboard/create_and_add_embeddables passing locally

* dashboard/dashboard_back_button passing locally

* dashboard/dashboard_error_handling passes locally

* dashboard/dashboard_grid passing locally

* dashboard/dashboard_options passing locally

* dashboard/dashboard_saved_query passes locally

* dashboard/dashboard_snapshots fails to match locally but may on CI

* fix lint error
LeeDr pushed a commit that referenced this pull request Apr 5, 2022
#129508)

* reimplement backport

* dashboard/create_and_add_embeddables passing locally

* dashboard/dashboard_back_button passing locally

* dashboard/dashboard_error_handling passes locally

* dashboard/dashboard_grid passing locally

* dashboard/dashboard_options passing locally

* dashboard/dashboard_saved_query passes locally

* dashboard/dashboard_snapshots fails to match locally but may on CI

* fix lint error

* dashboard_unsaved_listing and data_shared_attributes passing locally

* edit_embeddable_redirects and edit_visualizations passing locally

* embed_mode and embeddable_data_grid passing locally

* embeddable_library.ts embeddable_rendering.ts empty_dashboard.ts full_screen_mode.ts passing locally

* more backported test changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Functional Testing release_note:skip Skip the PR/issue when compiling release notes test_ui_functional test_xpack_functional v8.1.0 v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants