Skip to content

Add functional tests for visualize loader API#22595

Merged
timroes merged 9 commits intoelastic:masterfrom
timroes:visualize-loader-functional-test
Sep 4, 2018
Merged

Add functional tests for visualize loader API#22595
timroes merged 9 commits intoelastic:masterfrom
timroes:visualize-loader-functional-test

Conversation

@timroes
Copy link
Copy Markdown
Contributor

@timroes timroes commented Aug 31, 2018

This PR adds functional tests within the plugin functional tests for embedding visualizations via the visualize loader API.

This has two purposes:

  • Have the API tested from within a plugin, that might not have the Kibana "frame" (e.g. no time picker or filterbar available).
  • The plugin that's tested should be well documented to serve as a replacement for kibana_sample_plugin. That way we have an automated tested plugin, that demonstrates several usages of the visualize loader API and thus will always be up to date with the given Kibana version (not like the above plugin, that's most of the time out of date).
  • Have a better test coverage over embedding visualization in different combinations for upcoming refactorings.

Also a minor change was made in this PR, that makes the loader now set the required display: flex on the container the visualization is embedded into.

For QA: This PR should not change any behavior on Kibana.

Dev Docs

Visualize Loader

  • The visualize loader to embed visualizations will now set display: flex on the element you pass in, so that all visualizations are rendered correctly.
  • The embedVisualizationWithSavedObject method is now deprecated and it's not encouraged using it anymore. You should rather use embedVisualizationWithId and load a visualization by its id.

@timroes timroes added test WIP Work in progress Feature:Visualizations Generic visualization features (in case no more specific feature label is available) labels Aug 31, 2018
@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@timroes timroes added release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. v7.0.0 v6.5.0 and removed WIP Work in progress labels Sep 3, 2018
@timroes timroes requested review from markov00 and ppisljar September 3, 2018 13:43
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@ppisljar
Copy link
Copy Markdown
Contributor

ppisljar commented Sep 4, 2018

retest

Copy link
Copy Markdown
Contributor

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

thanks Tim! we'll need to add more tests around this, but this should make it quite easy to do so now.

<EuiPageContentBody>
{/*
The div you want to render into should have its dimension set, since the visualization will
take exactly the space of that element.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we mention 'or use flex layout' ?

// This has a query key, which holds the query part of an Elasticsearch query
// and a meta key allowing to set some meta values, most important for this API
// the `negate` option to negate the filter.
return loader.embedVisualizationWithId(domNode, id, {

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

Overall code looks good.
Tested the tests using slow 3g network settings and discovered a possible flaky test when waiting for visualization to complete rendering (see comment)
Approving after discuss this issue.

async function selectParams(id) {
await testSubjects.click('embeddingParamsSelect');
await find.clickByCssSelector(`option[value="${id}"]`);
await testSubjects.waitForDeleted('visLoadingIndicator');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This methods waits the default timeout, and if the network have high latency, it can be cause a timeout error and a "flaky test" like the following generated configuring slow 3g on dev tools:

fail: "embedding visualizations embed by id vis on timebased data without date histogram should correctly embed"
         │      Timeout: An operation did not complete before its timeout expired.

Our tests assumes that for sure the visLoadingIndicator will be removed after the whenFirstRenderComplete will complete, so here it's better to try this for X times at least or increase the default timeout to avoid having network timeout errors.

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@timroes timroes merged commit 6fa2b04 into elastic:master Sep 4, 2018
@timroes timroes deleted the visualize-loader-functional-test branch September 4, 2018 10:53
timroes added a commit to timroes/kibana that referenced this pull request Sep 4, 2018
* Initial visualize loader functional tests

* Extend plugin test README

* Add temporary tz work around

* Switch to Australia/North timezone

* Add filtering tests

* Add all tests

* Remove unneeded uiExports

* Improve explanation

* Add saved object test, add retry
timroes added a commit that referenced this pull request Sep 4, 2018
* Initial visualize loader functional tests

* Extend plugin test README

* Add temporary tz work around

* Switch to Australia/North timezone

* Add filtering tests

* Add all tests

* Remove unneeded uiExports

* Improve explanation

* Add saved object test, add retry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. test v6.5.0 v7.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants