-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add tests for Editable.getSettings() #3892
Conversation
blocks/editable/test/index.js
Outdated
} ); | ||
|
||
test( 'should be overriden', () => { | ||
const mock = jest.fn().mockImplementation( ( setting ) => 'mocked' ); // eslint-disable-line no-unused-vars |
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.
It is perfectly valid to omit setting
in function definition instead of disabling Eslint check.
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.
You can also pass callback to jest.fn
instead of using mockImplementation
. See https://facebook.github.io/jest/docs/en/mock-functions.html#mock-implementations.
const mock = jest.fn( () => 'mocked' );
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.
Yup, makes sense.
blocks/editable/test/index.js
Outdated
describe( 'Component', () => { | ||
describe( '.getSettings', () => { | ||
const value = [ 'Hi!' ]; | ||
const wrapper = shallow( <Editable value={ value } /> ); |
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 wrapper
is used only in one tests. It would be better to put it there, because the other tests uses a different wrapper.
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.
I left some minor comments, which aren't blockers. This looks good. Again, this tests a private method, which isn't ideal. Let's move on with this anyway :)
db8142e
to
e8e7eb1
Compare
e8e7eb1
to
b393dbe
Compare
Add tests for Editable.getSettings()