-
Notifications
You must be signed in to change notification settings - Fork 0
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
Ui tests #4
Ui tests #4
Changes from 4 commits
da4881b
8beb29b
2a17c11
922ddef
232ba40
0754257
a7b0acd
5f28efe
8a678a0
47d6cda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import { coreMock } from 'src/core/public/mocks'; | ||
import { renderApp } from './applications'; | ||
|
||
describe('renderApp', () => { | ||
it('mounts and unmounts UI', () => { | ||
const params = coreMock.createAppMountParamters(); | ||
const core = coreMock.createStart(); | ||
|
||
const unmount = renderApp(core, params, {}); | ||
expect(params.element.querySelector('.setup-guide')).not.toBeNull(); | ||
unmount(); | ||
expect(params.element.innerHTML).toEqual(''); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { mount } from 'enzyme'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I use mount over shallow in most tests for two reasons...
I this also generally lets us mock less and test bigger unit chunks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My wariness of using That being said, I accept that is a pre-optimization we don't have to worry about yet, that can easily wait until post-MVP, and you're right that useContext is far more annoying to do in I am slightly more concerned about point 1 - again, this is from past experience with writing tests with 3rd party libs (or even 1st party deps that we'd written ourselves) - but I tend to find that For example, if we wrote something like Sorry, all that was a lot of words :) TL;DR: I'm definitely not against moving forward with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I 100% agree with you. I usually default to shallow, and if shallow won't work, I use mount. One way I've handled the context thing in components is export two versions of a component, the outer component, which wraps an inner component to access context and pass it through as a simple property to the inner component. Then I unit test the InnerComponent.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was pre-hooks, redux higher-order component days, things are probably different now though. |
||
|
||
import { EngineOverviewHeader } from '../engine_overview_header'; | ||
import { KibanaContext } from '../../..'; | ||
|
||
describe('EngineOverviewHeader', () => { | ||
let enterpriseSearchUrl; | ||
|
||
afterEach(() => { | ||
enterpriseSearchUrl = undefined; | ||
}); | ||
|
||
const render = () => { | ||
return mount( | ||
<KibanaContext.Provider | ||
value={{ | ||
http: {}, | ||
enterpriseSearchUrl, | ||
setBreadcrumbs: jest.fn(), | ||
}} | ||
> | ||
<EngineOverviewHeader /> | ||
</KibanaContext.Provider> | ||
); | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to pull this out to a reusable helper function if possible. Something like this: const mountWithContext = (component, context) => {
const defaultContext = {
http: jest.fn(),
setBreadcrumbs: jest.fn(),
enterpriseSearchUrl: 'http://localhost:3002',
};
return mount(
<KibanaContext.Provider value={{ ...defaultContext, ...context }}>
{component}
</KibanaContext.Provider>
);
}; example usage: const wrapper = mountWithContext(<EngineOverviewHeader />, { enterpriseSearchUrl: '' }); I'd strongly prefer My thought is that this could get exported from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I created a |
||
|
||
describe('when enterpriseSearchUrl is set', () => { | ||
let wrapper; | ||
|
||
beforeEach(() => { | ||
enterpriseSearchUrl = 'http://localhost:3002'; | ||
wrapper = render(); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to see us refactor everything within context to be a passed param rather than let strings. As an example: const render = context => {
return mount(
<KibanaContext.Provider
value={{
http: {},
enterpriseSearchUrl: '',
setBreadcrumbs: jest.fn(),
...context,
}}
>
<EngineOverviewHeader />
</KibanaContext.Provider>
);
};
describe('when enterpriseSearchUrl is set', () => {
beforeEach(() => {
wrapper = render({ enterpriseSearchUrl: 'http://localhost:3002' });
});
});
describe('when enterpriseSearchUrl is not set', () => {
beforeEach(() => {
wrapper = render({ enterpriseSearchUrl: undefined });
});
}); Let me know how that feels to you? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Final form here: https://github.com/constancecchen/kibana/pull/4/files#r415998493 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feels good a7b0acd |
||
|
||
describe('the Launch App Search button', () => { | ||
const subject = () => wrapper.find('EuiButton[data-test-subj="launch_button"]'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a very minor thing, but I would actually slightly prefer that we remain as library-agnostic as possible and instead just do As an alternative note - There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, I'm seeing the same thing. I think it's because EuiButton is passing I'm fairly certain this wouldn't be an issue with shallow, so I'm good with leaving it for now and come back to it later when we've investigated using shallow more thoroughly. If we could change to camelCasing though that would be dope 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dope. 5f28efe |
||
|
||
it('should not be disabled', () => { | ||
expect(subject().props().isDisabled).toBeFalsy(); | ||
}); | ||
|
||
it('should use the enterpriseSearchUrl as the base path for its href', () => { | ||
expect(subject().props().href).toBe('http://localhost:3002/as'); | ||
}); | ||
}); | ||
}); | ||
|
||
describe('when enterpriseSearchUrl is not set', () => { | ||
JasonStoltz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let wrapper; | ||
|
||
beforeEach(() => { | ||
enterpriseSearchUrl = undefined; | ||
wrapper = render(); | ||
}); | ||
|
||
describe('the Launch App Search button', () => { | ||
const subject = () => wrapper.find('EuiButton[data-test-subj="launch_button"]'); | ||
|
||
it('should be disabled', () => { | ||
expect(subject().props().isDisabled).toBe(true); | ||
}); | ||
|
||
it('should not have an href', () => { | ||
expect(subject().props().href).toBeUndefined(); | ||
}); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,151 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import { appSearchBreadcrumbs, enterpriseSearchBreadcrumbs } from '../kibana_breadcrumbs'; | ||
|
||
jest.mock('../react_router_helpers', () => ({ | ||
letBrowserHandleEvent: () => false, | ||
})); | ||
|
||
describe('appSearchBreadcrumbs', () => { | ||
const historyMock = { | ||
createHref: jest.fn(), | ||
push: jest.fn(), | ||
}; | ||
|
||
const breadCrumbs = [ | ||
{ | ||
text: 'Page 1', | ||
path: '/page1', | ||
}, | ||
{ | ||
text: 'Page 2', | ||
path: '/page2', | ||
}, | ||
]; | ||
|
||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
const subject = () => appSearchBreadcrumbs(historyMock)(breadCrumbs); | ||
|
||
it('Builds a chain of breadcrumbs with Enterprise Search and App Search at the root', () => { | ||
JasonStoltz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
expect(subject()).toEqual([ | ||
{ | ||
href: undefined, | ||
onClick: expect.any(Function), | ||
text: 'Enterprise Search', | ||
}, | ||
{ | ||
href: undefined, | ||
onClick: expect.any(Function), | ||
text: 'App Search', | ||
}, | ||
{ | ||
href: undefined, | ||
onClick: expect.any(Function), | ||
text: 'Page 1', | ||
}, | ||
{ | ||
href: undefined, | ||
onClick: expect.any(Function), | ||
text: 'Page 2', | ||
}, | ||
]); | ||
}); | ||
|
||
describe('links', () => { | ||
const eventMock = { | ||
preventDefault: jest.fn(), | ||
}; | ||
|
||
it('has a link to Enterprise Search Home page first', () => { | ||
subject()[0].onClick(eventMock); | ||
expect(historyMock.push).toHaveBeenCalledWith('/'); | ||
}); | ||
|
||
it('has a link to App Search second', () => { | ||
subject()[1].onClick(eventMock); | ||
expect(historyMock.push).toHaveBeenCalledWith('/app_search'); | ||
}); | ||
|
||
it('has a link to our custom page third', () => { | ||
subject()[2].onClick(eventMock); | ||
expect(historyMock.push).toHaveBeenCalledWith('/page1'); | ||
}); | ||
|
||
it('has a link to our second custom page last', () => { | ||
subject()[3].onClick(eventMock); | ||
expect(historyMock.push).toHaveBeenCalledWith('/page2'); | ||
}); | ||
}); | ||
}); | ||
|
||
describe('enterpriseSearchBreadcrumbs', () => { | ||
const historyMock = { | ||
createHref: jest.fn(), | ||
push: jest.fn(), | ||
}; | ||
|
||
const breadCrumbs = [ | ||
{ | ||
text: 'Page 1', | ||
path: '/page1', | ||
}, | ||
{ | ||
text: 'Page 2', | ||
path: '/page2', | ||
}, | ||
]; | ||
|
||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
const subject = () => enterpriseSearchBreadcrumbs(historyMock)(breadCrumbs); | ||
|
||
it('Builds a chain of breadcrumbs with Enterprise Search at the root', () => { | ||
expect(subject()).toEqual([ | ||
{ | ||
href: undefined, | ||
onClick: expect.any(Function), | ||
text: 'Enterprise Search', | ||
}, | ||
{ | ||
href: undefined, | ||
onClick: expect.any(Function), | ||
text: 'Page 1', | ||
}, | ||
{ | ||
href: undefined, | ||
onClick: expect.any(Function), | ||
text: 'Page 2', | ||
}, | ||
]); | ||
}); | ||
|
||
describe('links', () => { | ||
const eventMock = { | ||
preventDefault: jest.fn(), | ||
}; | ||
|
||
it('has a link to Enterprise Search Home page first', () => { | ||
subject()[0].onClick(eventMock); | ||
expect(historyMock.push).toHaveBeenCalledWith('/'); | ||
}); | ||
|
||
it('has a link to our custom page third', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. replace with: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated here: a7b0acd |
||
subject()[1].onClick(eventMock); | ||
expect(historyMock.push).toHaveBeenCalledWith('/page1'); | ||
}); | ||
|
||
it('has a link to our second custom page last', () => { | ||
subject()[2].onClick(eventMock); | ||
expect(historyMock.push).toHaveBeenCalledWith('/page2'); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { SetAppSearchBreadcrumbs } from '../kibana_breadcrumbs'; | ||
import { mount } from 'enzyme'; | ||
|
||
jest.mock('./generate_breadcrumbs', () => ({ | ||
appSearchBreadcrumbs: jest.fn(), | ||
})); | ||
import { appSearchBreadcrumbs } from './generate_breadcrumbs'; | ||
|
||
jest.mock('react-router-dom', () => ({ | ||
useHistory: () => ({ | ||
createHref: jest.fn(), | ||
push: jest.fn(), | ||
location: { | ||
pathname: '/current-path', | ||
}, | ||
}), | ||
})); | ||
|
||
import { KibanaContext } from '../..'; | ||
JasonStoltz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
describe('SetAppSearchBreadcrumbs', () => { | ||
const setBreadcrumbs = jest.fn(); | ||
const builtBreadcrumbs = []; | ||
const appSearchBreadCrumbsInnerCall = jest.fn().mockReturnValue(builtBreadcrumbs); | ||
const appSearchBreadCrumbsOuterCall = jest.fn().mockReturnValue(appSearchBreadCrumbsInnerCall); | ||
appSearchBreadcrumbs.mockImplementation(appSearchBreadCrumbsOuterCall); | ||
|
||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
const render = props => { | ||
return mount( | ||
<KibanaContext.Provider | ||
value={{ | ||
http: {}, | ||
enterpriseSearchUrl: 'http://localhost:3002', | ||
setBreadcrumbs, | ||
}} | ||
> | ||
<SetAppSearchBreadcrumbs {...props} /> | ||
</KibanaContext.Provider> | ||
); | ||
}; | ||
|
||
describe('when isRoot is false', () => { | ||
const subject = () => render({ text: 'Page 1', isRoot: false }); | ||
|
||
it('calls appSearchBreadcrumbs to build breadcrumbs, then registers them with Kibana', () => { | ||
subject(); | ||
|
||
// calls appSearchBreadcrumbs to build breadcrumbs with the target page and current location | ||
expect(appSearchBreadCrumbsInnerCall).toHaveBeenCalledWith([ | ||
{ text: 'Page 1', path: '/current-path' }, | ||
]); | ||
|
||
// then registers them with Kibana | ||
expect(setBreadcrumbs).toHaveBeenCalledWith(builtBreadcrumbs); | ||
}); | ||
}); | ||
|
||
describe('when isRoot is true', () => { | ||
JasonStoltz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const subject = () => render({ text: 'Page 1', isRoot: true }); | ||
|
||
it('calls appSearchBreadcrumbs to build breadcrumbs with an empty breadcrumb, then registers them with Kibana', () => { | ||
subject(); | ||
|
||
// uses an empty bredcrumb | ||
expect(appSearchBreadCrumbsInnerCall).toHaveBeenCalledWith([]); | ||
|
||
// then registers them with Kibana | ||
expect(setBreadcrumbs).toHaveBeenCalledWith(builtBreadcrumbs); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be renamed
index.test.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, and should also be moved to
public/applications/index.test.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a7b0acd