Skip to content

Use testing-library ESLint plugin #9870

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.

Sign up for GitHub

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

Merged
aduth merged 1 commit into from
Jan 8, 2024
Merged

Use testing-library ESLint plugin #9870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions .eslintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"plugin:react-hooks/recommended",
"plugin:react/jsx-runtime"
],
"plugins": ["@18f/eslint-plugin-identity"],
"plugins": ["@18f/eslint-plugin-identity", "testing-library"],
"env": {
"browser": true,
"commonjs": true
Expand All @@ -20,10 +20,6 @@
{
"selector": "AssignmentExpression[left.property.name='href'][right.type=/(Template)?Literal/]",
"message": "Do not assign window.location.href to a string or string template to avoid losing i18n parameters"
},
{
"selector": "ExpressionStatement[expression.callee.object.name='userEvent']",
"message": "Await the promised result of a userEvent interaction"
}
]
},
Expand All @@ -44,11 +40,17 @@
"devDependencies": true,
"packageDir": "."
}
]
],
"testing-library/await-async-events": "error",
"testing-library/await-async-queries": "error",
"testing-library/await-async-utils": "error",
"testing-library/no-await-sync-events": "error",
"testing-library/no-await-sync-queries": "error",
"testing-library/no-debugging-utils": "error"
}
},
{
// Turn off react linting rules for most packages/files
// Turn off react linting rules for most packages/files
"files": [
"spec/**",
"app/javascript/packs/**",
Expand All @@ -63,8 +65,8 @@
"app/javascript/packages/validated-field/**",
"app/javascript/packages/verify-flow/**",
// In progress: enabling these rules for all files in packages/document-capture
"app/javascript/packages/document-capture/context/**",
"app/javascript/packages/document-capture/higher-order/**",
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's gonna lint this file to detect trailing spaces? 😛

"app/javascript/packages/document-capture/context/**",
"app/javascript/packages/document-capture/higher-order/**",
"app/javascript/packages/document-capture/hooks/**",
// Comment out a file to enable react lint rules for that file only
"app/javascript/packages/document-capture/components/acuant-camera.tsx",
Expand Down Expand Up @@ -113,6 +115,6 @@
"react-hooks/exhaustive-deps": "off",
"react-hooks/rules-of-hooks": "off"
}
},
}
]
}
}
12 changes: 6 additions & 6 deletions app/javascript/packages/address-search/components/address-search.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('AddressSearch', () => {
const locationsURL = 'https://localhost:3000/locations/endpoint';

context('Page Heading and PO Search About Message', () => {
it('both render when handleLocationSelect is not null', async () => {
it('both render when handleLocationSelect is not null', () => {
const handleLocationsFound = sandbox.stub();
const onSelect = sinon.stub();
const { queryByText, queryByRole } = render(
Expand All @@ -25,8 +25,8 @@ describe('AddressSearch', () => {
</SWRConfig>,
);

const heading = await queryByText('in_person_proofing.headings.po_search.location');
const aboutMessage = await queryByText(
const heading = queryByText('in_person_proofing.headings.po_search.location');
const aboutMessage = queryByText(
'in_person_proofing.body.location.po_search.po_search_about',
);

Expand All @@ -37,7 +37,7 @@ describe('AddressSearch', () => {
).to.exist();
});

it('both do not render when handleLocationSelect is null', async () => {
it('both do not render when handleLocationSelect is null', () => {
const handleLocationsFound = sandbox.stub();
const onSelect = sinon.stub();
const { queryByText } = render(
Expand All @@ -53,8 +53,8 @@ describe('AddressSearch', () => {
</SWRConfig>,
);

const heading = await queryByText('in_person_proofing.headings.po_search.location');
const aboutMessage = await queryByText(
const heading = queryByText('in_person_proofing.headings.po_search.location');
const aboutMessage = queryByText(
'in_person_proofing.body.location.po_search.po_search_about',
);
expect(heading).to.be.empty;
Expand Down
22 changes: 10 additions & 12 deletions app/javascript/packages/address-search/components/full-address-search.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('FullAddressSearch', () => {
const usStatesTerritories = [['Delware', 'DE']];

context('Page Heading and PO Search About Message', () => {
it('both render when handleLocationSelect is not null', async () => {
it('both render when handleLocationSelect is not null', () => {
const handleLocationsFound = sandbox.stub();
const onSelect = sinon.stub();
const { queryByText, queryByRole } = render(
Expand All @@ -30,8 +30,8 @@ describe('FullAddressSearch', () => {
</SWRConfig>,
);

const heading = await queryByText('in_person_proofing.headings.po_search.location');
const aboutMessage = await queryByText(
const heading = queryByText('in_person_proofing.headings.po_search.location');
const aboutMessage = queryByText(
'in_person_proofing.body.location.po_search.po_search_about',
);

Expand All @@ -42,7 +42,7 @@ describe('FullAddressSearch', () => {
).to.exist();
});

it('both do not render when handleLocationSelect is null', async () => {
it('both do not render when handleLocationSelect is null', () => {
const handleLocationsFound = sandbox.stub();
const { queryByText } = render(
<SWRConfig value={{ provider: () => new Map() }}>
Expand All @@ -57,8 +57,8 @@ describe('FullAddressSearch', () => {
</SWRConfig>,
);

const heading = await queryByText('in_person_proofing.headings.po_search.location');
const aboutMessage = await queryByText(
const heading = queryByText('in_person_proofing.headings.po_search.location');
const aboutMessage = queryByText(
'in_person_proofing.body.location.po_search.po_search_about',
);
expect(heading).to.be.empty;
Expand All @@ -67,7 +67,7 @@ describe('FullAddressSearch', () => {
});

context('Address Search Label Text', () => {
it('does not render when handleLocationSelect is not null', async () => {
it('does not render when handleLocationSelect is not null', () => {
const handleLocationsFound = sandbox.stub();
const onSelect = sinon.stub();
const { queryByText } = render(
Expand All @@ -83,13 +83,11 @@ describe('FullAddressSearch', () => {
</SWRConfig>,
);

const searchLabel = await queryByText(
'in_person_proofing.headings.po_search.address_search_label',
);
const searchLabel = queryByText('in_person_proofing.headings.po_search.address_search_label');
expect(searchLabel).to.be.empty;
});

it('renders when handleLocationSelect is null', async () => {
it('renders when handleLocationSelect is null', () => {
const handleLocationsFound = sandbox.stub();
const { queryByText } = render(
<SWRConfig value={{ provider: () => new Map() }}>
Expand All @@ -104,7 +102,7 @@ describe('FullAddressSearch', () => {
</SWRConfig>,
);

const searchLabel = await queryByText(
const searchLabel = queryByText(
'in_person_proofing.body.location.po_search.address_search_label',
);
expect(searchLabel).to.exist();
Expand Down
14 changes: 11 additions & 3 deletions ...capture/components/in-person-location-full-address-entry-post-office-search-step.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,11 @@ const DEFAULT_PROPS = {
describe('InPersonLocationFullAddressEntryPostOfficeSearchStep', () => {
const usStatesTerritories: [string, string][] = [['Delware', 'DE']];
const locationsURL = 'https://localhost:3000/locations/endpoint';
const inPersonURL = '#in_person';
const wrapper: ComponentType = ({ children }) => (
<InPersonContext.Provider
value={{
inPersonURL,
locationsURL,
addressSearchURL: 'https://localhost:3000',
inPersonOutageMessageEnabled: false,
Expand Down Expand Up @@ -74,7 +76,10 @@ describe('InPersonLocationFullAddressEntryPostOfficeSearchStep', () => {
beforeEach(() => {
server.resetHandlers();
// todo: should we return USPS_RESPONSE here?
server.use(rest.post(locationsURL, (_req, res, ctx) => res(ctx.json([{ name: 'Baltimore' }]))));
server.use(
rest.post(locationsURL, (_req, res, ctx) => res(ctx.json([{ name: 'Baltimore' }]))),
rest.put(locationsURL, (_req, res, ctx) => res(ctx.json({ success: true }))),
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this like a missing stub?

Copy link
Copy Markdown
Contributor Author

@aduth aduth Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this like a missing stub?

Yes, with the previous code, we weren't awaiting findAllByText, so the thing it was "clicking" was a promise object, not the intended button element. Clicking the button issues a network request before redirecting. Once the correction was made to await the result to click the button, these changes were also necessary to stub the resulting network response.

);
});

it('renders the step', () => {
Expand Down Expand Up @@ -326,8 +331,11 @@ describe('InPersonLocationFullAddressEntryPostOfficeSearchStep', () => {
await findByLabelText('in_person_proofing.body.location.po_search.zipcode_label'),
);

await userEvent.click(findAllByText('in_person_proofing.body.location.location_button')[0]);
await userEvent.click(
(await findAllByText('in_person_proofing.body.location.location_button'))[0],
);

expect(await queryByText('simple_form.required.text')).to.be.null();
expect(queryByText('simple_form.required.text')).to.be.null();
expect(window.location.hash).to.equal(inPersonURL);
});
});
12 changes: 9 additions & 3 deletions .../packages/document-capture/components/in-person-location-post-office-search-step.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,11 @@ describe('InPersonLocationPostOfficeSearchStep', () => {
const usStatesTerritories: [string, string][] = [['Delware', 'DE']];
const locationsURL = 'https://localhost:3000/locations/endpoint';
const addressSearchURL = 'https://localhost:3000/addresses/endpoint';
const inPersonURL = '#in_person';
const wrapper: ComponentType = ({ children }) => (
<InPersonContext.Provider
value={{
inPersonURL,
locationsURL,
addressSearchURL,
inPersonOutageMessageEnabled: false,
Expand Down Expand Up @@ -353,7 +355,7 @@ describe('InPersonLocationPostOfficeSearchStep', () => {
await userEvent.click(
await findByText('in_person_proofing.body.location.po_search.search_button'),
);
const moreResults = await queryAllByText('in_person_proofing.body.location.location_button');
const moreResults = queryAllByText('in_person_proofing.body.location.location_button');

expect(moreResults).to.be.empty();
});
Expand All @@ -366,6 +368,7 @@ describe('InPersonLocationPostOfficeSearchStep', () => {
res(ctx.json(DEFAULT_RESPONSE), ctx.status(200)),
),
rest.post(locationsURL, (_req, res, ctx) => res(ctx.json([{ name: 'Baltimore' }]))),
rest.put(locationsURL, (_req, res, ctx) => res(ctx.json({ success: true }))),
);
});

Expand All @@ -387,9 +390,12 @@ describe('InPersonLocationPostOfficeSearchStep', () => {
await findByLabelText('in_person_proofing.body.location.po_search.address_search_label'),
);

await userEvent.click(findAllByText('in_person_proofing.body.location.location_button')[0]);
await userEvent.click(
(await findAllByText('in_person_proofing.body.location.location_button'))[0],
);

expect(await queryByText('in_person_proofing.body.location.inline_error')).to.be.null();
expect(queryByText('in_person_proofing.body.location.inline_error')).to.be.null();
expect(window.location.hash).to.equal(inPersonURL);
});
});
});
2 changes: 1 addition & 1 deletion app/javascript/packages/form-steps/form-steps.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ describe('FormSteps', () => {

await userEvent.click(getByText(t('forms.buttons.continue')));

await expect(findByText('Second Title')).to.be.fulfilled();
await findByText('Second Title');
await expect(checkFormHasExpectedErrors()).to.be.rejected();

window.history.back();
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
"eslint-plugin-prettier": "^5.0.0",
"eslint-plugin-react": "^7.31.8",
"eslint-plugin-react-hooks": "^4.6.0",
"eslint-plugin-testing-library": "^6.2.0",
"jsdom": "^22.1.0",
"mocha": "^10.0.0",
"mq-polyfill": "^1.1.8",
Expand Down
3 changes: 2 additions & 1 deletion spec/javascript/packages/document-capture/components/document-capture-spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,10 @@ describe('document-capture/components/document-capture', () => {
submitButton = getByText('forms.buttons.submit.default');
expect(isFormValid(submitButton.closest('form'))).to.be.true();

// eslint-disable-next-line require-await
Copy link
Copy Markdown
Contributor Author

@aduth aduth Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is somewhat strange and unexpected. Without it, it identifies the new Promise argument function as being async without and await call, but it's not an async function, and I'm not sure why it would suddenly start being flagged.

My guess is that some transitive dependency of what's changed in yarn.lock changed how the file is being parsed?

await new Promise((resolve) => {
onSubmit.callsFake(resolve);
// eslint-disable-next-line no-restricted-syntax
// eslint-disable-next-line testing-library/await-async-events
userEvent.click(submitButton);
});

Expand Down
8 changes: 4 additions & 4 deletions spec/javascript/packages/document-capture/components/review-issues-step-spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ describe('document-capture/components/review-issues-step', () => {
});

it('skip renders initially with warning page when failed image is submitted again', () => {
const { findByRole, getByRole, getByText } = render(
const { queryByRole, getByRole, getByText } = render(
<I18nContext.Provider
value={
new I18n({
Expand Down Expand Up @@ -446,9 +446,9 @@ describe('document-capture/components/review-issues-step', () => {
</I18nContext.Provider>,
);

expect(findByRole('button', { name: 'idv.failure.button.warning' })).not.to.exist;
expect(getByRole('heading', { name: 'doc_auth.headings.review_issues' })).to.be.ok;
expect(getByText('duplicate image')).to.be.ok;
expect(queryByRole('button', { name: 'idv.failure.button.warning' })).to.not.exist();
expect(getByRole('heading', { name: 'doc_auth.headings.review_issues' })).to.be.ok();
expect(getByText('duplicate image')).to.be.ok();
});

context('ial2 strict', () => {
Expand Down
Loading