Skip to content

Some more false negative for testing-library/prefer-screen-queries #391

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

Open
julienw opened this issue May 31, 2021 · 3 comments
Open

Some more false negative for testing-library/prefer-screen-queries #391

julienw opened this issue May 31, 2021 · 3 comments
Labels
enhancement New feature or request pinned Pinned for different reasons. Issues with this label won't be flagged as stale by stalebot

Comments

@julienw
Copy link
Contributor

julienw commented May 31, 2021

Hey @Belco90
I just upgraded to the new version that fixed #367, and I am reporting that it indeed reported new cases...but not all of them!

For example these cases aren't reported still: https://github.com/firefox-devtools/profiler/blob/1c5dfed055f864413b3a62e43ca2d27b074d6108/src/test/components/CallNodeContextMenu.test.js#L107-L112

I'm pretty sure this is because of the spread operator. And indeed I reproduced by slightly adjusting one of the smaller testcases:

import * as React from 'react';
import { render } from 'firefox-profiler/test/fixtures/testing-library';

function setup() {
  const result = render(<div />);
  return { ...result };
}

it('detected', async () => {
  const { getByText } = await setup();

  expect(getByText('foo')).toBeInTheDocument(); // detected
});

it('undetected', () => {
  const { getByText } = setup();

  expect(getByText('foo')).toBeInTheDocument(); // undetected
});

Hope this helps :-)

@Belco90
Copy link
Member

Belco90 commented May 31, 2021

Hey @julienw! Thanks for reporting more feedback about prefer-screen-queries rule. It definitely seems the rule is not ready for the spread operator.

@Belco90 Belco90 added the enhancement New feature or request label May 31, 2021
@julienw
Copy link
Contributor Author

julienw commented Jul 3, 2021

If I understand properly the current implementation for the rule, it tries to follow where variables were declared, so that when there's a call with eg getByText the rule can decide if the variable is legit. My understanding is that the rule minimizes the false positives. I would like to suggest that it could minimize false negatives instead: the reason is that it's easy to silence false positives, but false negatives are forever lost.

Therefore, why can't we report all getByText calls that are "standalone" to start with (that is they're not method calls) ? This would also follow the spirit of not fixing #143.
Possible false positives is if a project has getByText and friends functions that aren't from testing-library.

@stale
Copy link

stale bot commented Sep 1, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Sep 1, 2021
@Belco90 Belco90 added pinned Pinned for different reasons. Issues with this label won't be flagged as stale by stalebot and removed wontfix This will not be worked on labels Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pinned Pinned for different reasons. Issues with this label won't be flagged as stale by stalebot
Projects
None yet
Development

No branches or pull requests

2 participants