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

Add --pytest argument to name-tests-test hook. #712

Closed
jenstroeger opened this issue Jan 6, 2022 · 5 comments
Closed

Add --pytest argument to name-tests-test hook. #712

jenstroeger opened this issue Jan 6, 2022 · 5 comments

Comments

@jenstroeger
Copy link

jenstroeger commented Jan 6, 2022

Considering that pytest’s test discovery uses “test_*.py or *_test.py” file names, I think it makes sense to make this hook a bit more flexible — e.g. by adding a dedicated arg.

Happy to provide PR.

@asottile
Copy link
Member

asottile commented Jan 6, 2022

the default mode is pytest mode -- allowing both is not desirable (pick one and stick with it for consistency)

@asottile asottile closed this as completed Jan 6, 2022
@jenstroeger
Copy link
Author

Looking at this line of code:

test_name_pattern = r'test.*\.py' if args.django else r'.*_test\.py'

File names like test_foo.py (an acceptable PyTest filename) fail for the hook:

> pre-commit run name-tests-test --all-files
Tests should end in _test.py.............................................Failed
- hook id: name-tests-test
- exit code: 1

tests/test_something.py does not match pattern ".*_test\.py"

unless I allow them with the --django option — although my project doesn’t use Django at all (thus expecting that option is a bit misleading).

So, if we were picky then the above line should change to

test_name_pattern = r'test.*\.py' if args.django else r'test_.*\.py|.*_test\.py'

which is actually correct considering PyTest’s documented file name patterns.

Furthermore, I’d consider changing Tests should end in _test.py message

name: tests should end in _test.py

because it’s incorrect when using the --django option.

@asottile
Copy link
Member

asottile commented Jan 6, 2022

yes of course I understand that, I am a pytest core maintainer

the tool chooses instead to enforce a convention rather than letting you loosely use multiple naming ideas -- but it gives you the freedom to choose which convention you prefer

if you don't want to pick a convention them simply don't use the tool

note also that the tool's existence predates pytest's change in convention (allowing both names)

while we're being pedantic, note also that pytest is not spelled "PyTest" but is stylized "pytest"

@jenstroeger
Copy link
Author

Thank you for the context!

note also that the tool's existence predates pytest's change in convention (allowing both names)

Alrighty, time to update the tool/hook then 😉

Alternatively, how about adding a --pattern option where folks can specify a custom file name patterns? The --django option could stay for back-compat. I can provide a PR, if that helps.

Furthermore, I’d consider changing Tests should end in _test.py message

name: tests should end in _test.py

because it’s incorrect when using the --django option.

Would you like a PR for that change?

@asottile
Copy link
Member

asottile commented Jan 6, 2022

I'm sorry but you're not listening, I'm not going to continue this discussion

@pre-commit pre-commit locked as resolved and limited conversation to collaborators Jan 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants