-
Notifications
You must be signed in to change notification settings - Fork 484
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
Don't install tests #1710
Don't install tests #1710
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @scop .
Agree in principal, however please see the discussion in here where we're talking about moving some of the current test functionality into the main codebase to validate custom dictionaries:
https://github.com/codespell-project/codespell/pull/1669/files/654599a6cdcffa7a74bb8da8d223adbc4f23bdb6#diff-7b3ed02bc73dc06b7db906cf97aa91dec2b2eb21f2d92bc5caa761df5bbc168f
Are you interested in having a go at that?
Not really at this point, sorry. |
Most repos I work on do include tests in their distributions. However, I'm not sure it's necessary. Let's try it and we can easily revert if needed |
@larsoner Did you think of source distributions such as sdist (which certainly must include tests), or binary distributions such as wheel (which are supposed to exclude tests)? @larsoner It looks like #2523 broke this, tests are again included by wheel binary distributions: codespell-2.2.2-py3-none-any.whl contains tests/test_*.py files
|
I think it's fine to include them, so I don't mind having them in there / effectively reverting this. They are small and don't really do any harm AFAIK. And as above, many projects do include them, so maybe there are some advantages (e.g., being able to test that your particular installation passes all tests) |
They're not needed at runtime.