chore(ui-react): lint test files #3306
Conversation
|
| let updateControlledValue; | ||
| const ControlledSwitch = () => { | ||
| const [isChecked, setIsChecked] = React.useState(true); | ||
| const changeFunction = (e) => { | ||
| setIsChecked(e.target.checked); | ||
| }; | ||
| updateControlledValue = setIsChecked; | ||
|
|
||
| return ( | ||
| <SwitchField | ||
| label={label} | ||
| isChecked={isChecked} | ||
| onChange={changeFunction} | ||
| /> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
This test component was extraneous, the desired behavior can be achieved using the rerender function returned from calling render
| ); | ||
|
|
||
| const field = container.getElementsByTagName('input')[0]; | ||
| expect(field).toBeChecked(); | ||
| }); | ||
|
|
||
| it('should fire the onChange function with a checkbox change event', async () => { |
There was a problem hiding this comment.
Removed this test as it was really only testing the behavior of the SwitchFieldControlledExample contained inside, not the functionality of the SwitchField. To note, doublechecked that it had no impact on test coverage
| ); | ||
|
|
||
| if (breakpoint === 'base') { | ||
| // eslint-disable-next-line jest/no-conditional-expect |
There was a problem hiding this comment.
Disabled jest/no-conditional-expect here (and below on lines 70, 92, and 95) to avoid refactoring further, which most likely require moving away from the current approach of using the iterative it.each
| let matchMedia: MatchMediaMock; | ||
| let mediaQueries = getMediaQueries({ breakpoints }); | ||
|
|
||
| const testBreakpoints = (m) => { |
There was a problem hiding this comment.
Curious why linting didn't like sharing this function. I thought it was pretty clever ;)
There was a problem hiding this comment.
Eslint does not care abt cleverness 😭
There was a problem hiding this comment.
All jokes aside, maybe it kind of breaks jest idioms. Could see an argument for requiring the tests to be inside an it for readability?
| mediaQueries.reverse().forEach(testBreakpoints); | ||
| it.each(mediaQueries)( | ||
| `should return the $breakpoint breakpoint as expected`, | ||
| ({ query, breakpoint }) => { |
There was a problem hiding this comment.
Couldn't this be the shared function here?
There was a problem hiding this comment.
Tried that but it gives a linting error in the it blocks because eslint thinks there are no tests inside. We could just disable that lint rule but thought it was probably okay to be more verbose since it's a test file
Description of changes
Begin the process of linting all the test files in
@aws-amplify/ui-react:yarn react lint --fixto autofix issuesjestidiomaticit.eachBefore (with linting turned on for test files):
After (with linting turned on for test files):
Issue #, if available
NA
Description of how you validated changes
yarn react lint && yarn react testChecklist
yarn testpassessideEffectsfield updatedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.