More frontend tests #56
Conversation
| let wrapper; | ||
|
|
||
| beforeEach(() => { | ||
| props = { |
There was a problem hiding this comment.
there is an issue with react-dropzone that is generating an error:
react-dropzone/react-dropzone#554
| it('should render to right', () => { | ||
| wrapper.setProps({ menuOffsetDirection: 'right' }); | ||
| expect(wrapper.find('.menu').prop('style')).toHaveProperty('left', -30); | ||
| expect(wrapper.find('.menu').prop('style')).toHaveProperty('right', undefined); |
There was a problem hiding this comment.
these two expect expressions could be merged into one like in this example:
expect(wrapper.find('.video-stream img').prop('style')).toEqual({width: 600,height:450})
There was a problem hiding this comment.
As we discussed, this would not be a good idea in this case.
There was a problem hiding this comment.
There is a recommendation to use .toBeUndefined() when checking for undefined.
https://jestjs.io/docs/en/expect#tobeundefined
| it('should not propagate click', () => { | ||
| wrapper.simulate('click', mockClickEvent); | ||
| expect(mockClickEvent.stopPropagation).toBeCalled(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
We agreed on the following:
-Use .toHaveLength() to check for element existence, because it also checks if it exists only once.
-In the future, use .hasClass() where applicable. It's not worth the effort to rewrite anything for this now.
-Use .prop('disabled') to check for disabled status, instead of .find([disabled])
-Use toBe() when not comparing objects.
-Use .prop('key') instead of .props().key.
| it('should call onSubmitHandler when form is submitted', () => { | ||
| wrapper.simulate('submit', mockClickEvent); | ||
| expect(props.onSubmitHandler).toHaveBeenCalled(); | ||
| expect(wrapper.state().submitted).toEqual(true); |
There was a problem hiding this comment.
I propose to use state() as we agreed in the case of props().
state('key') instead of state().key
| import * as React from 'react'; | ||
| import { shallow } from 'enzyme'; | ||
| import { UnmountClosed } from 'react-collapse'; |
There was a problem hiding this comment.
This import should be a few lines below in the component list.
There was a problem hiding this comment.
I normally put external imports at the top, then our components under // components. react-collapse is an external import.
| wrapper.find('.label').simulate('click', mockClickEvent); | ||
| expect(wrapper.find('.visible')).toHaveLength(1); | ||
| wrapper.find('.label').simulate('click', mockClickEvent); | ||
| expect(wrapper.find('.visible').exists()).toEqual(false); |
There was a problem hiding this comment.
What is the reason here for having 2 different patterns to check for the existence of an element with this class?
There was a problem hiding this comment.
It's not, the first checks that it's open, the the second checks that it's closed
|
Everything looks good. |
No description provided.