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

Proposing a new flag, --list-of-files-or-dirs, to compliment the positional file_or_dir argument #11871

Closed
jbkkd opened this issue Jan 28, 2024 · 10 comments · Fixed by #12085
Closed
Labels
status: help wanted developers would like help from experts on this topic type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature

Comments

@jbkkd
Copy link

jbkkd commented Jan 28, 2024

What's the problem this feature will solve?

Passing a long list of test files/dirs into the pytest CLI can make reading the pytest command confusing.

Describe the solution you'd like

A new flag, --list-of-files-or-dirs, to compliment the positional file_or_dir argument.

The new flag will accept a single text file, with each line in the file representing a positional argument to pytest.

So, instead of calling pytest like this:
pytest test_file1.py test_file2.py test_file3.py ...
One would call:
pytest --list-of-files-or-dirs list_file.txt
With list_file.txt containing:

test_file1.py
test_file2.py
test_file3.py
...

Alternative Solutions

You can currently just call pytest with a list of all the files/dirs you want to test, but a long list passed through the CLI isn't such a clean interface.

Additional context

Happy to work take a stab at working on this if there's consensus that such a PR would be accepted.

@The-Compiler
Copy link
Member

Can you talk a bit more about your use-case? Why are you passing such a long list of files in the first place?

@jbkkd
Copy link
Author

jbkkd commented Jan 29, 2024

We're looking into dynamically choosing which tests to run for a given PR as part of our CI run.

The output of this test selection would be a list of tests from our test suite to run. The list is 0...N, and although this is discouraged, N theoretically could be hundreds of tests long if the PR is big / touches many files and functions.

@RonnyPfannschmidt
Copy link
Member

i would recommend using a collect_ignore hook

then instead of needing a new pytest feature and new pytest release, you can simply ignore/remove unwanted tests from collection either before collecting a file, or after collection completed

@jbkkd
Copy link
Author

jbkkd commented Jan 29, 2024

collect_ignore lives inside conftest.py, which we already have populated in our repo. We'd ideally want to avoid changing any of our existing files for this feature.

The new file I'm proposing is a standalone, brand new file generated for the sake of CI alone, and so would not have any effect on existing code.

Moreover, If we do go with this approach, we would need to first collect all our existing tests, and then deduct the list of tests we want to run, push that to collect_ignore, and then have pytest collect tests yet again. I can only imagine the amount of time this would add to our test suite, time that we're trying to shave here.

@bluetech
Copy link
Member

@jbkkd Your use case makes sense, but I'm not clear why you need a file for this instead of passing the arguments on the command line as usual. That is, instead of pytest --list-of-files-or-dirs list_file.txt do pytest $(cat list_file.txt).


There is a convention of using @file for this, e.g. gcc:

Read command-line options from file. The options read are inserted in place of the original @file option. If file does not exist, or cannot be read, then the option will be treated literally, and not removed.

Options in file are separated by whitespace. A whitespace character may be included in an option by surrounding the entire option in either single or double quotes. Any character (including a backslash) may be included by prefixing the character to be included with a backslash. The file may itself contain additional @file options; any such options will be processed recursively.

The use case I'm familiar with is command line max length issues on Windows.

@nicoddemus
Copy link
Member

I would like to add that the cat trick is shell specific, and you have to resort to other solutions when writing multi-platform "scripts" (for example the script part of a GitHub action).

I think there are legitimate uses for this feature (in some form or another), plus it is simple to implement, we should consider adding it to pytest.

@RonnyPfannschmidt
Copy link
Member

https://docs.python.org/3/library/argparse.html#fromfile-prefix-chars

It's possible that it constitutes a breaking change, just very unlikely

@bluetech bluetech added the type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature label Jan 31, 2024
@bluetech bluetech added the status: help wanted developers would like help from experts on this topic label Mar 5, 2024
@bluetech
Copy link
Member

bluetech commented Mar 5, 2024

I think using fromfile_prefix_chars='@' would be nice, PR welcome.

@levsa
Copy link
Contributor

levsa commented Mar 5, 2024

Just want to point out that it would be nice if the contents of the file could be the output of pytest --collect-only -q, e.g. one line of the input file could be e.g. file_path.py::TestClass::test_name[param_set0].

@RonnyPfannschmidt
Copy link
Member

thats how the fromfile_prefix_chars feature of argparse works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted developers would like help from experts on this topic type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants