Merged
Conversation
Contributor
|
Should we go straight to 16.2.0? |
Contributor
|
@kjbekkelund I think it makes sense to complete our upgrade to 16.0.0 and then once that's done immediately bump Kibana, X-Pack, and EUI to 16.2.0. Does that sound good to you? |
Contributor
|
Works for me 👍 |
* Update to Enzyme 3.x. (#12) * Add findTestSubject test helper. Fix KuiCodeEditor tests. * Fix KuiContextMenu tests. * Fix KuiConfirmModal tests. * Fix tests for KuiPager. * Update KuiConfirmModal tests to use findTestSubject helper. * Publish test helpers from ui_framework. Fix DashboardCloneModal tests.
* Fix DashboardPanelContainer tests. * Update KuiColorPicker tests. * Fix ControlsTab tests. * Rename vis to input_control_vis. Update and fix InputControlVis tests. * Fix KuiListingTable tests. * Fix KuiTextInput tests. * Update DashboardGrid test snapshots.
Contributor
Author
|
jenkins, test this |
* Remove old enzyme setup file. * Add .test to YesNo and AddDeleteButtons test files.
stacey-gammon
approved these changes
Dec 8, 2017
| import React from 'react'; | ||
| import sinon from 'sinon'; | ||
| import { mount, shallow } from 'enzyme'; | ||
| import { findTestSubject } from 'ui_framework/test'; |
kimjoar
approved these changes
Dec 11, 2017
Contributor
kimjoar
left a comment
kimjoar
left a comment
There was a problem hiding this comment.
LGTM.
Added my thoughts on findTestSubject, but if you don't agree I'm fine with merging as-is.
| @@ -0,0 +1,10 @@ | |||
| // Extract the DOM node which matches a specific test subject selector. | |||
| export const findTestSubject = (mountedComponent, testSubjectSelector, isDOMNode = true) => { | |||
Contributor
There was a problem hiding this comment.
Big +1 for extracting this function.
Hm, maybe add some docs or examples that explains isDOMNode? Not sure what if it from reading this function.
Contributor
There was a problem hiding this comment.
Thinking a bit more on this. Personally I also find boolean args difficult to read. I think I'd personally just move the getDomNode outside this function. I feel it's easier to read findTestSubject(myComponent, 'foo').getDOMNode(). We also just needed getDOMNode 3 times before, so it doesn't feel too bad to do that (but maybe we need it more now because of changes in enzyme?)
Contributor
There was a problem hiding this comment.
I really like your point about boolean args -- I have the same difficulty with them. I also like your suggestion. We'll try it out.
nreese
added a commit
to nreese/kibana
that referenced
this pull request
Dec 11, 2017
* get kibana to start with react 16.0.0 * ensure width and height for dashboard grid * Update to Enzyme 3.x. (#12) * Fix tests broken by Enzyme upgrade (#13) * Update to Enzyme 3.x. (#12) * Add findTestSubject test helper. Fix KuiCodeEditor tests. * Fix KuiContextMenu tests. * Fix KuiConfirmModal tests. * Fix tests for KuiPager. * Update KuiConfirmModal tests to use findTestSubject helper. * Publish test helpers from ui_framework. Fix DashboardCloneModal tests. * Fix more tests (#14) * Fix DashboardPanelContainer tests. * Update KuiColorPicker tests. * Fix ControlsTab tests. * Rename vis to input_control_vis. Update and fix InputControlVis tests. * Fix KuiListingTable tests. * Fix KuiTextInput tests. * Update DashboardGrid test snapshots. * Fix test file names (#15) * Remove old enzyme setup file. * Add .test to YesNo and AddDeleteButtons test files. * needed window.cancelAnimationFrame to accompany requestAnimationFrame * remove getDOMNode function call from findTestSubject
nreese
added a commit
that referenced
this pull request
Dec 11, 2017
* get kibana to start with react 16.0.0 * ensure width and height for dashboard grid * Update to Enzyme 3.x. (#12) * Fix tests broken by Enzyme upgrade (#13) * Update to Enzyme 3.x. (#12) * Add findTestSubject test helper. Fix KuiCodeEditor tests. * Fix KuiContextMenu tests. * Fix KuiConfirmModal tests. * Fix tests for KuiPager. * Update KuiConfirmModal tests to use findTestSubject helper. * Publish test helpers from ui_framework. Fix DashboardCloneModal tests. * Fix more tests (#14) * Fix DashboardPanelContainer tests. * Update KuiColorPicker tests. * Fix ControlsTab tests. * Rename vis to input_control_vis. Update and fix InputControlVis tests. * Fix KuiListingTable tests. * Fix KuiTextInput tests. * Update DashboardGrid test snapshots. * Fix test file names (#15) * Remove old enzyme setup file. * Add .test to YesNo and AddDeleteButtons test files. * needed window.cancelAnimationFrame to accompany requestAnimationFrame * remove getDOMNode function call from findTestSubject
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
@cjcenizal and I paired up to upgraded to react 16.0.0 so kibana is running the same version as EUI