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

[Time to Visualize] Adds functional tests for linking/unlinking panel from embeddable library #89612

Merged
merged 9 commits into from
Feb 8, 2021

Conversation

cqliu1
Copy link
Contributor

@cqliu1 cqliu1 commented Jan 28, 2021

Summary

Related to #80584.

This adds functional tests for unlinking a panel from the embeddable library and save a panel to the embeddable library.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@cqliu1 cqliu1 added chore Feature:Dashboard Dashboard related features Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:small Small Level of Effort v8.0.0 impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. v7.12.0 labels Jan 28, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@cqliu1 cqliu1 force-pushed the dashboard/embeddable-library-tests branch from 3ecee63 to 181dae0 Compare January 28, 2021 18:19
@cqliu1 cqliu1 added the release_note:skip Skip the PR/issue when compiling release notes label Jan 28, 2021
@cqliu1 cqliu1 requested a review from a team January 28, 2021 18:21
@cqliu1 cqliu1 added the review label Jan 28, 2021
@cqliu1 cqliu1 marked this pull request as draft January 28, 2021 18:39
@cqliu1 cqliu1 added WIP Work in progress and removed review labels Jan 28, 2021
@cqliu1 cqliu1 force-pushed the dashboard/embeddable-library-tests branch from 181dae0 to f3a6dc6 Compare February 1, 2021 20:39
@cqliu1
Copy link
Contributor Author

cqliu1 commented Feb 1, 2021

@elasticmachine merge upstream

@cqliu1 cqliu1 marked this pull request as ready for review February 2, 2021 16:46
@cqliu1 cqliu1 added review and removed WIP Work in progress labels Feb 2, 2021
@cqliu1 cqliu1 force-pushed the dashboard/embeddable-library-tests branch from 5baca1e to 85f5bdd Compare February 2, 2021 17:33
@cqliu1 cqliu1 requested a review from a team as a code owner February 2, 2021 17:33

const originalPanel = await testSubjects.find('embeddablePanelHeading-documentexample');
await dashboardPanelActions.unlinkFromLibary(originalPanel);
await testSubjects.existOrFail('unlinkPanelSuccess');
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better instead of asserting a success message gets displayed to also go to visualize listing page and ensure saved object is listed when its in the library and not listed when not in library?

Copy link
Contributor

Choose a reason for hiding this comment

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

Even though unlinking won't actually remove any entries from the library, this is still important to test at least once. You could start by creating a panel by value, adding it via the 'save to library' action, then double checking that it shows up in the visualize listing page.

});

after(async () => {
await security.testUser.restoreDefaults();
Copy link
Contributor

Choose a reason for hiding this comment

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

One concern I have about this test is that "document example" saved object is used in other maps tests. What happens if there is an error linking the saved object back to the library? Will down stream tests be effected? Seems like there is not a good way to clean up the "document example" saved object incase it gets left in a stranded state. How about creating a new map saved object in https://github.com/elastic/kibana/blob/master/x-pack/test/functional/es_archives/maps/kibana/data.json that is used only in this test to avoid an potential flaky tests if this test fails.

Copy link
Contributor

Choose a reason for hiding this comment

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

The 'unlink' feature doesn't delete any saved objects. It only affects the panel that it is applied against, and this panel looks to be created on a new dashboard so it shouldn't cause any downstream problems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nreese I've updated the maps tests to create a new map. Can you take a look when you get a chance?

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.

These tests are looking great, and everything is passing CI which is amazing. I left a note of one additional test that could be good to add.

@@ -91,6 +91,7 @@
/src/plugins/dashboard/ @elastic/kibana-presentation
/src/plugins/input_control_vis/ @elastic/kibana-presentation
/src/plugins/vis_type_markdown/ @elastic/kibana-presentation
/test/functional/apps/dashboard/ @elastic/kibana-presentation
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!


const originalPanel = await testSubjects.find('embeddablePanelHeading-documentexample');
await dashboardPanelActions.unlinkFromLibary(originalPanel);
await testSubjects.existOrFail('unlinkPanelSuccess');
Copy link
Contributor

Choose a reason for hiding this comment

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

Even though unlinking won't actually remove any entries from the library, this is still important to test at least once. You could start by creating a panel by value, adding it via the 'save to library' action, then double checking that it shows up in the visualize listing page.

@cqliu1 cqliu1 force-pushed the dashboard/embeddable-library-tests branch from c682ae9 to 2c0c63c Compare February 4, 2021 21:00
@spalger
Copy link
Contributor

spalger commented Feb 4, 2021

jenkins test this

@cqliu1 cqliu1 force-pushed the dashboard/embeddable-library-tests branch from 8bb01f7 to 3ca707b Compare February 5, 2021 22:34
@cqliu1 cqliu1 force-pushed the dashboard/embeddable-library-tests branch from 3ca707b to 14d4118 Compare February 8, 2021 18:40
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

ci stats were not enabled for this build

History

  • 💔 Build #104454 failed 3ca707b610ae9c29dfa5c1c1cac70acc1fceb377
  • 💔 Build #104071 failed 8bb01f758226e89fef30edab82da8a4d2acd4b98
  • 💔 Build #104054 failed 2c0c63cccbf8cc7325d5e1e3bfa0ea2e2e80acbf
  • 💔 Build #103998 failed 2c0c63cccbf8cc7325d5e1e3bfa0ea2e2e80acbf
  • 💔 Build #103581 failed c682ae9aa8443706ea010f9297add3cdde14ff89

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

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

LGTM
code review

@cqliu1 cqliu1 merged commit 3cf00d2 into elastic:master Feb 8, 2021
@cqliu1 cqliu1 deleted the dashboard/embeddable-library-tests branch February 8, 2021 23:21
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 9, 2021
…timeline-and-rollover-info

* 'master' of github.com:elastic/kibana: (47 commits)
  [Fleet] Use TS project references (elastic#87574)
  before/beforeEach clean up (elastic#90663)
  [Vega] user should be able to set a specific tilemap service using the mapStyle property (elastic#88440)
  [Security Solution][Case] ServiceNow SIR Connector (elastic#88655)
  [Search Sessions] Enable extend from management (elastic#90558)
  [ILM] Delete phase redesign (rework) (elastic#90291)
  [APM-UI][E2E] use withGithubStatus step (elastic#90651)
  Add folding in kb-monaco and update some viewers (elastic#90152)
  [Grok Debugger] Changed test to wait for grok debugger container to exist to fix test flakiness (elastic#90543)
  Strongly typed EUI theme for styled-components (elastic#90106)
  Fix vega renovate label (elastic#90591)
  [Uptime] Migrate to TypeScript project references (elastic#90510)
  [Monitoring] Migrate data source for legacy alerts to monitoring data directly (elastic#87377)
  [Upgrade Assistant] Add A11y Tests (elastic#90265)
  [Time to Visualize] Adds functional tests for linking/unlinking panel from embeddable library (elastic#89612)
  [dev-utils/ship-ci-stats] fail when CI stats is down (elastic#90678)
  chore(NA): remove write permissions on Bazel remote cache for PRs (elastic#90652)
  chore(NA): move bazel workspace status from bash script into nodejs executable (elastic#90560)
  Use default ES distribution for functional tests (elastic#88737)
  [Alerts] Jira: Disallow labels with spaces (elastic#90548)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/timeline/timeline.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/lib/absolute_timing_to_relative_timing.test.ts
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/lib/absolute_timing_to_relative_timing.ts
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 10, 2021
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 89612 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 11, 2021
cqliu1 added a commit that referenced this pull request Feb 11, 2021
… from embeddable library (#89612) (#91066)

# Conflicts:
#	.github/CODEOWNERS

Co-authored-by: Kibana Machine <[email protected]>
@cqliu1 cqliu1 restored the dashboard/embeddable-library-tests branch June 8, 2022 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Feature:Dashboard Dashboard related features impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants