Skip to content
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

Handle dead code in tests folders of the repo #15803

Open
webknjaz opened this issue Jan 31, 2025 · 1 comment
Open

Handle dead code in tests folders of the repo #15803

webknjaz opened this issue Jan 31, 2025 · 1 comment

Comments

@webknjaz
Copy link
Member

Problem statement

While reviewing #15772, the discussions led to re-discovering the observation that not all lines in the tests folders are hit in CI. This effectively means that there's dead code in there.

End goal / acceptance criteria

Guidance

There are several reasons for lines not being marked as covered, and different situations may need different approaches.

  • A function or a fixture that is never used anywhere. Solution: delete it.
    • Sometimes, a test function is copied and not renamed. When this happens, the first copy has all of its lines uncovered because it's shadowed by the new function. Solution: rename one of the functions, so both are actually executed.
  • A Python module that is never used anywhere. Solution: delete it.
  • Code that is only hit when testing goes wrong, this is often under a branch in the control flow. Solution: append a # pragma: no cover to the line. Be granular, add a code comment justifying the exclusion as these lines will stop influencing the coverage metric forever, being never taken into account.
  • Part of tests is run conditionally and is supposed to remain in the repo. Solution: add the corresponding invocation in CI. Alternative (undesired): make the test runner omit coverage collection in .coveragerc.

Locating the affected files

The test files with dead code can be found by browsing these web pages:

Justification / reading materials

When there's dead test code, it goes stale after some while. It becomes broken while contributing to a false sense of test existence. We don't want this.

N.B. I posted more on how coverage is set up in general, here: #15772 (comment).

@AlanCoding
Copy link
Member

Thanks, from quick looks at the coverage link

  • factories/ was a half-baked idea at the start and shortcomings in the results here should be legitimate. Any amount of that we could delete should be great.
  • docs/ is probably another case of missing coverage collection, because this is used to generate and push the schema. The point of this material is the github action itself. It's not tests in the conventional sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants