Skip to content

collection: python_files=* / explicit collection #4476

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
blueyed opened this issue Nov 28, 2018 · 13 comments
Open

collection: python_files=* / explicit collection #4476

blueyed opened this issue Nov 28, 2018 · 13 comments
Labels
topic: collection related to the collection phase type: enhancement new feature or API change, should be merged into features branch

Comments

@blueyed
Copy link
Contributor

blueyed commented Nov 28, 2018

I think it would be nice if pytest =(echo "def test_foo: pass") would work, i.e. it should consider any explicitly passed in file arguments for collection. (Zsh creates a temporary file here).

Also using -o 'python_files=*' does not appear to help here.

Related/via #4452.

@blueyed blueyed added the topic: collection related to the collection phase label Nov 28, 2018
@RonnyPfannschmidt
Copy link
Member

im -1 on that one - if its not having a .py extension i don't want to see it collected as python module in core

@blueyed
Copy link
Contributor Author

blueyed commented Nov 28, 2018

Fair point.

What about a warning then if python_files does not match *.py / ends with .py?

@RonnyPfannschmidt
Copy link
Member

that sounds usefull

@RonnyPfannschmidt
Copy link
Member

sidenote - https://unix.stackexchange.com/questions/482459/specifying-the-file-extension-produced-by-zsh-process-substitution should set the temporary file thing right at least in part - it may need extra variables to make it pytohn-safe

@blueyed blueyed added the type: enhancement new feature or API change, should be merged into features branch label Apr 13, 2019
@anarcat
Copy link

anarcat commented Sep 23, 2019

sidenote - https://unix.stackexchange.com/questions/482459/specifying-the-file-extension-produced-by-zsh-process-substitution should set the temporary file thing right at least in part - it may need extra variables to make it pytohn-safe

this doesn't help for my use case which is a small script that I don't want to have to ship as an entire package (see also #3520)

@blueyed blueyed closed this as completed Nov 27, 2019
@anarcat
Copy link

anarcat commented Nov 27, 2019

why was this closed? is this abandoned, or fixed? thanks!

@blueyed
Copy link
Contributor Author

blueyed commented Nov 28, 2019

@anarcat a new issue / PR should be created based on #4476 (comment) probably. Please do so if you want to "own" / work on it.
This issue by itself appears to be a no-go for pytest itself.

@sarnold
Copy link

sarnold commented Dec 17, 2019

Seriously? I'm with @anarcat on this one, mainly to satisfy packaging QA tools that yell at you for shipping a "binary" with a .py extension. If i do that ^^ pytest not only doesn't find the file but refuses to even touch it. Is the answer really "tell the package QA tool to blow" and always keep the .py extension if you want pytest to check it? In this case it's not a "trivial"/small script but an actual daemon script.

@nicoddemus
Copy link
Member

im -1 on that one - if its not having a .py extension i don't want to see it collected as python module in core

IIUC the proposal here is for python_files=* to be honored by the python plugin and try to collect anything, even if they don't have the .py extension. I believe the chance for misuse here is minimal. Here's the relevant code:

def pytest_collect_file(path, parent):
ext = path.ext
if ext == ".py":
if not parent.session.isinitpath(path):
if not path_matches_patterns(
path, parent.config.getini("python_files") + ["__init__.py"]
):
return
ihook = parent.session.gethookproxy(path)
return ihook.pytest_pycollect_makemodule(path=path, parent=parent)

All it would need is to remove the initial check for ".py" extension; of course I know the issue here is not one of implementation complexity, but opportunities for misuse; I'm just curious to know what are those opportunities, given that in this proposal the user must explicitly configure pytest to ignore the file extension.

@blueyed
Copy link
Contributor Author

blueyed commented Dec 30, 2019

I agree with @nicoddemus here, re-opening for now.
Maybe @RonnyPfannschmidt and/or @asottile can explain/elaborate why this should not be allowed (through explicit configuration).

(@nicoddemus
The initial issue was for pytest foo.txt to work already, without changing python_files - i.e. any given file would be collected already, but I can see that the "python" plugin should only handle "python_files" really.)

@blueyed blueyed reopened this Dec 30, 2019
@RonnyPfannschmidt
Copy link
Member

at first glance we would have to reconfigure both the python import system and the assertion rewriter to do it correctly, if that's the complexity needed, then i don't want to see it in core a plugin will have to contain it

@nicoddemus
Copy link
Member

Agreed, if it is more complex than a first glance shows, then we should avoid adding it to the core.

But if a simple implementation can be shown to support it, I would be OK with adding it to the core.

@blueyed
Copy link
Contributor Author

blueyed commented Dec 30, 2019

The assert rewriting uses python_files (without validation), so it's not more complex than removing the check that @nicoddemus pointed out.
(As for having a separate config for the assert rewriting see blueyed#145 (which appears to not be wanted / is expected behavior for pytest itself though, and not really relevant here - just mentioning it since it shows what code is involved there)

A simple implementation to show support for it would probably overwrite the pytest_collect_file to not have the check then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: collection related to the collection phase type: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

No branches or pull requests

5 participants