Skip to content

Conversation

@cdce8p
Copy link

@cdce8p cdce8p commented Feb 19, 2022

At the moment, files are filtered from pytestArgs before test discovery is run. This is unexpected for a user and could lead to issues:

  1. Not all tests will be found
    Given a file tree and pytestArgs
tests/
├── dir
│   └── test_b.py
├── helpers
│   └── test_c.py
└── test_a.py
    "python.testing.pytestArgs": [
        "tests/dir",
        "tests/helpers/test_c.py",
    ],

tests/helpers/test_c.py would be filtered and only tests inside of tests/dir would be discovered.

  1. pytest discovery failing completely
    In large projects it's possible that different test subfolders require different dependencies via conftest.py. Not all of them might be installed. If the desire was to only discover tests in tests/dir/test_b.py, having that argument filtered would mean that discovery is run from root and thus all conftest.py files are loaded. If the test is inside a subfolder, it's possible to work around it by removing the filename. However that doesn't work for tests inside the root test directory.

Closes: #18562

@kimadeline
Copy link

kimadeline commented Feb 22, 2022

Hi @cdce8p, thank you for your contribution!

Could you open a bug against the repo with the same description as what you've put here (title could be something along the lines of "Pytest test discovery failing when pytestArgs contains filepaths"), and update the news entry to your PR?

Thanks!

@cdce8p
Copy link
Author

cdce8p commented Feb 23, 2022

Could you open a bug against the repo with the same description as what you've put here (title could be something along the lines of "Pytest test discovery failing when pytestArgs contains filepaths"), and update the news entry to your PR?

Done 🙂 One other thing, I wasn't sure if a test would makes sense here. I couldn't find an existing one and all the PR is doing is to remove a filter.

@kimadeline
Copy link

It should be fine to have no tests, we're planning to rewrite pytest support soon (#17242).

@kimadeline kimadeline added the skip tests Updates to tests unnecessary label Feb 23, 2022
Copy link

@kimadeline kimadeline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! 🥳 Sadly this PR barely did not make the cutoff for the next release (I was away last week), I will merge it as soon as we got our RC ready, and your changes will ship with the next release.

@kimadeline kimadeline merged commit 69731c4 into microsoft:main Mar 1, 2022
@cdce8p cdce8p deleted the pytest-discovery branch March 1, 2022 16:45
wesm pushed a commit to posit-dev/positron that referenced this pull request Mar 28, 2024
* Fix pytest file discovery

* Rename news file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip tests Updates to tests unnecessary

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pytest test discovery failing when pytestArgs contains filepaths

2 participants