Initial unit tests for embedded_visualize_handler #31324
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lukeelmers
added
Feature:ExpressionLanguage
|
Pinging @elastic/kibana-app |
This comment has been minimized.
This comment has been minimized.
💚 Build Succeeded |
lukeelmers
added a commit
to lukeelmers/kibana
that referenced
this pull request
Mar 7, 2019
lukeelmers
added a commit
to lukeelmers/kibana
that referenced
this pull request
Mar 7, 2019
spalger
pushed a commit
that referenced
this pull request
Mar 7, 2019
Reverts #31324, failing on master
|
This PR started causing master to fail, so I reverted it in #32615 |
PhilippBaranovskiy
pushed a commit
to PhilippBaranovskiy/kibana
that referenced
this pull request
Mar 7, 2019
PhilippBaranovskiy
pushed a commit
to PhilippBaranovskiy/kibana
that referenced
this pull request
Mar 7, 2019
…2615) Reverts elastic#31324, failing on master
|
This was my bad, the last green CI build was awhile ago, and I should've re-run it as a precautionary measure. Opened #32657 with the same PR rebased on master. Will merge & backport that one instead once a green build comes back. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #30422
This is a first pass at unit tests for the
embedded_visualize_handler. For now I prioritized public methods & interfaces, and have also tried to cover some of the issues we've seen come up recently with the work on the new pipeline (e.g. #30405, #30830) -- but I'm open to suggestion on any other pieces we feel are particularly error-prone that would be worth spending time on now.Public stuff that hasn't been tested yet is focused around rendering, because that's where the two paths diverge (pipeline/visualize data loaders). Also for the legacy
visualizationLoader, it is very tricky to get things working as you have to try mocking theVisobject, especially without the help of the usualngMock.moduleandngMock.injectsetup.Since we are looking to move toward the expression pipeline as soon as things are stable, I hesitate to invest the time in getting tests working with
visualizationLoader-- I think that time is better spent focusing on tests for the pipeline/interpreter.Ran this suite both with and without the pipeline; everything should pass in both cases.