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

bugfix: Stricter check for "tests" and "docs" directories in sp-repo-review #527

Merged

Conversation

ukmo-ccbunney
Copy link
Contributor

Summary

The sp-repo-review checks for the existence of the tests (PY005) and docs (PY004) directories are too lenient and often return false positives.

Details

The existence of a file or directory in the repo that contains the word "docs" or "tests" will result in a pass for these tests.

For example, the existence of the .readthedocs file causes the PY004 Has docs folder test to pass, simply because it contains the work docs. Similarly, an innocuously named file such as fastest.txt would result in a false pass of PY005 Has tests folder.

This PR modifies the PY004 and PY005 checks to:

  • Compare against string tests or docs, rather than testing if the filename contains that string.
  • Require that the path is a directory
  • test is no longer a match for PY005: should be tests as per Scientific Python: package-structure; this required an update to the internal tests.

@henryiii
Copy link
Collaborator

Making this require a folder is great, and I think that solves your initial issue. I'm a little worried about too many false negatives; old packages that use /test or packages that use tests-integration, for example, do have tests, so this check probably should pass.

@ukmo-ccbunney
Copy link
Contributor Author

Making this require a folder is great, and I think that solves your initial issue. I'm a little worried about too many false negatives; old packages that use /test or packages that use tests-integration, for example, do have tests, so this check probably should pass.

Hi @henryiii
OK - that makes sense, although I made this change taking into account the very specific guidance in the "packaging" section of the Scientific Python guide:

You should have a /tests folder PY005 (recommended) and/or a src//tests folder.

How do you feel about a slightly more lenient check for a folder that starts with the word test?

@henryiii
Copy link
Collaborator

I think that's fine.

Also added some extra unit tests for the Py004 and Py005 checks.
@ukmo-ccbunney
Copy link
Contributor Author

I think that's fine.

Great - I've pushed up a new commit that does this.
It also adds some extra unit tests for the PY004 (has docs) and PY005 (has tests) checks.

@henryiii henryiii merged commit 95e0bb0 into scientific-python:main Dec 13, 2024
19 checks passed
@henryiii
Copy link
Collaborator

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants