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

[visualization_editor] tabify visData before render #14068

Merged
merged 1 commit into from
Oct 3, 2017

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Sep 20, 2017

This fixes an error that sometimes is causing the tests to fail (it's a race) https://kibana-ci.elastic.co/job/elastic+kibana+6.x+multijob-intake/287/console

The error this prevents is here: http://github.com/elastic/kibana/blob/5c9ba5f73cc613e18689403819ee0a58f887b40c/src/core_plugins/table_vis/public/table_vis_controller.js#L32-L33

This line is executing after the test fails because the test completes after renderFunction is called and doesn't wait for rendering to complete (despite the test title).

@ppisljar can you take a look at how we can ensure that the vis tests are done rendering and cleaned up before moving on to the next tests?

@ppisljar
Copy link
Member

if response should be tabified or not depends on the actual visualization the editor will render.
visualization defines responseHandler (which could be none, tabify or basic (does the point series/hierarchical stuff)

as the tests here are using table visualization tabify will work correctly.
maybe a better result would be to markdown, as it does not require any data, yet we would still be able to test visualize_editor

Copy link
Member

@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.

LGTM

@spalger
Copy link
Contributor Author

spalger commented Sep 20, 2017

how we can ensure that the vis tests are done rendering and cleaned up before moving on to the next tests?

Have any ideas @ppisljar?

@ppisljar
Copy link
Member

@spalger wanna discuss this live ?

generally, rendercomplete will get called after rendering is done ... looking at the table code that should be the case ... so i am wondering if something else is going on ? do we get a renderComplete, but then render is called again or sth like this ?

@spalger spalger merged commit cdf7f89 into elastic:master Oct 3, 2017
spalger added a commit that referenced this pull request Oct 3, 2017
spalger added a commit that referenced this pull request Oct 3, 2017
@spalger
Copy link
Contributor Author

spalger commented Oct 3, 2017

6.0: 1be4e00
6.x/6.1: cc7e5a5

@spalger spalger deleted the fix/visualization_editor-tests branch October 3, 2017 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants