-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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 Jest-specific linting rules #5057
Conversation
Adding this caught a couple of important things: - The sound integration tests have a `.only` in them - The project-saver-hoc tests had two tests with the same description, causing possible confusion if that test broke
@@ -1,4 +1,4 @@ | |||
jasmine.DEFAULT_TIMEOUT_INTERVAL = 30000; // eslint-disable-line no-undef | |||
jest.setTimeout(30000); // eslint-disable-line no-undef |
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.
Confirmed that these are equivalent via https://jestjs.io/docs/en/jest-object.html#jestsettimeouttimeout and jestjs/jest#652
@@ -55,7 +55,7 @@ describe('Slider Prompt Container', () => { | |||
const componentProps = wrapper.find(SliderPromptComponent).props(); | |||
componentProps.onChangeMin({target: {value: '1.0'}}); | |||
componentProps.onOk(); | |||
expect(onOk).toBeCalledWith(1, 100, false); | |||
expect(onOk).toHaveBeenCalledWith(1, 100, false); |
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.
Would not be opposed to disabling this rule if we think it's annoying, it's just part of the recommended set. These changes were made just by using --fix
, so pretty easy to deal with.
@@ -423,7 +423,7 @@ describe('projectSaverHOC', () => { | |||
expect(mockedOnRemixing).toHaveBeenCalledWith(true); | |||
}); | |||
|
|||
test('when starting to remix, onRemixing should be called with param true', () => { | |||
test('when starting to remix, onRemixing should be called with param false', () => { |
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.
The test above this one has the same name, so the linter caught this copy-paste error
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.
LGTM #past-tense
This is follow up from #5049, so that kind of mistake will cause linting to fail.
Adding this caught a couple of important things:
.only
in them on the version of the code I had at the time