-
Notifications
You must be signed in to change notification settings - Fork 40
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
prefer-in-document: .toHaveLength(1)
vs .toBeInTheDocument()
#171
Comments
Actually For example, given the following test: test('should only have 1 option', () => {
render(<MySelectThatRenderOneOrMoreOptions />);
// Retrieve all rendered options
const allOptions = within(
screen.getByRole('combobox'),
).getAllByRole('option');
// Make sure there is only 1
expect(allOptions).toHaveLength(1);
expect(allOptions).toHaveValue('value');
}); Here the test semantically represents counting the number of rendered elements and making sure there is one and exactly one element. After running the auto-fixer (and renaming variables accordingly): test('should only have 1 option', () => {
render(<MySelectThatRenderOneOrMoreOptions />);
// Retrieve all rendered options
const option = within(
screen.getByRole('combobox'),
).getByRole('option');
// Make sure there is only 1
expect(option).toBeInTheDocument();
expect(option).toHaveValue('value');
}); In this case, the test is structured in a way that makes it about the first element itself. Granted, it would fail if there were more than 1 element (and RTL would let us know we might want to use I think |
What about the flip case here, when a |
I think |
@nstepien yes, makes sense to me. Just one last subtly I wanted to ask about, and that is how best to handle queries which are expected to return exactly one vs queries which are expected to return one or more matches - e.g. As we have been discussing, this makes sense // no violation here, no fix required
expect(screen.queryAllByRole('option')).toHaveLength(1); but what about the case in which // original
expect(screen.queryByRole('option')).toHaveLength(1); In this second case, |
I think it depends,
I'd say the rule should warn about it, and provide 2 suggestions, wdyt? |
I like your suggestion @nstepien 👍 I'll have a crack at implementing that |
Based on the discussion in this thread, I've put together this truth table of selector type vs assertion style:
The resolution of the
both of which are stable states. |
Got hit by this again today 😔 |
Any update on this? Got positive on |
@SevenOutman PRs are welcome; reading over the comments I think its just wrong for this rule to be reporting on I'd say that should be our first focus, and then we can explore if it would make sense to expand the rule again to cover cases where we can know for sure that you should be using |
Sure. I've created a PR #273 for it. :) |
eslint-plugin-jest-dom
version: 3.8.1node
version: 16.1.0npm
version: 7.11.1Relevant code or config
What you did:
I want to ensure there is only one
option
.What happened:
The rule wants this to be
Reproduction repository:
Problem description:
It's not the same thing! I cannot assert that there is only one
option
now.Suggested solution:
prefer-in-document
should allow.toHaveLength(1)
. Or there should be an option at least.The text was updated successfully, but these errors were encountered: