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

Use same pytest --rootdir logic as test discovery #17898

Merged
merged 3 commits into from
Nov 8, 2021

Conversation

bhrutledge
Copy link

@bhrutledge bhrutledge commented Nov 2, 2021

This is a quick submission via github.dev, so please pardon the lack of local testing and adherence to the contributing guidelines.

For #9553, specifically re: the second issue described in #9553 (comment).

FYI @karthiknadig, who touched this code in #17509.

@bhrutledge bhrutledge changed the title Use same --rootdir logic as test discovery Use same --rootdir logic as test discovery Nov 2, 2021
@bhrutledge bhrutledge changed the title Use same --rootdir logic as test discovery Use same pytest --rootdir logic as test discovery Nov 2, 2021
@@ -89,7 +89,7 @@ export class PytestRunner implements ITestsRunner {
// if user has provided `--rootdir` then use that, otherwise add `cwd`
if (testArgs.filter((a) => a.startsWith('--rootdir')).length === 0) {
// Make sure root dir is set so pytest can find the relative paths
testArgs.splice(0, 0, '--rootdir', options.workspaceFolder.fsPath);
testArgs.splice(0, 0, '--rootdir', options.cwd);
Copy link
Author

Choose a reason for hiding this comment

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

Based on the logic below, this should use python.testing.cwd if it's set, otherwise the --rootdir option from python.testing.pytestArgs.

return this.runner.runTests(
testRun,
{
workspaceFolder: workspace.uri,
cwd:
settings.testing.cwd && settings.testing.cwd.length > 0
? settings.testing.cwd
: workspace.uri.fsPath,
token,
args: settings.testing.pytestArgs,
},
this.idToRawData,

@karthiknadig
Copy link
Member

@bhrutledge Can you provide a news file under news/2 Fixes with name 9553.md with content something like (so you get credit for this fix):

Partial fix for using the same directory as discovery while running.
(thanks [Brian Rutledge](https://github.com/bhrutledge))

@ghost
Copy link

ghost commented Nov 2, 2021

CLA assistant check
All CLA requirements met.

@karthiknadig karthiknadig self-requested a review November 2, 2021 21:06
@karthiknadig karthiknadig assigned karthiknadig and unassigned karrtikr Nov 2, 2021
@karthiknadig
Copy link
Member

@bhrutledge Please sign the CLA so we can review and merge this.

@bhrutledge
Copy link
Author

bhrutledge commented Nov 4, 2021

Please sign the CLA so we can review and merge this.

@karthiknadig Sorry for the delay on this; I haven't signed a CLA before, and was waiting on some advice.

In the meantime, I've installed the VSIX locally and verified that it sets --rootdir appropriately in my example repo. IE, given "python.testing.cwd": "repo/rootdir", running a single test file yields:

> ~/Dev/vscode-pytest-workspace/venv/bin/python -m pytest --rootdir ~/Dev/vscode-pytest-workspace/repo/rootdir --override-ini junit_family=xunit1 --junit-xml=/var/folders/bw/ttgk1hcs0k989q2yzmxkzn8c0000gp/T/tmp-12273mm0YANFsJ0JF.xml ./package/test_module.py

Aside: I didn't find any documentation on how to use a VS Code extension while it's being developed, without manually installing the VSIX. I'm thinking of something equivalent to the "editable installs" provided via pip and setuptools. Is that possible? Can somebody point me in the right direction?

Speaking of testing: I note that a check failed because this PR doesn't include TypeScript tests. However, it's not clear to me where to write such a test, and I note that #17509 didn't include tests. Any suggestions?

@bhrutledge
Copy link
Author

@karthiknadig ICYMI, I've signed the CLA.

@karthiknadig karthiknadig added the skip tests Updates to tests unnecessary label Nov 4, 2021
@karthiknadig
Copy link
Member

@bhrutledge here are some instructions on how to build and run the extension during development.

Build environments and compiling: https://github.com/microsoft/vscode-python/wiki/Coding
Running the code under debugger: https://github.com/microsoft/vscode-python/wiki/Testing#debugging

There aren't any tests for this. I added this code as an intermediate step till we get to point of updating the pytest adapter. I think the test adapter that we currently use tries to do things that it should probably let pytest handle itself. We are currently in the process of re-doing the test adapters starting with the unittest adapter first, and pytest will be next. So most of this code will go away when we get to that point.

@bhrutledge
Copy link
Author

here are some instructions on how to build and run the extension during development.

Thanks @karthiknadig. I had read over those, but they didn't answer my question; I've opened #17931 for assistance.

We are currently in the process of re-doing the test adapters

I'm interested in following this. Can you share some relevant links?

For this PR, is there anything else you need from me?

@bhrutledge
Copy link
Author

@karthiknadig Can you take another look at this? Looks like some checks are still pending; don't know why.

@bhrutledge
Copy link
Author

This failing test doesn't seem related to this change: https://github.com/microsoft/vscode-python/runs/4143672875?check_suite_focus=true

@karthiknadig
Copy link
Member

That is due to pipenv ending support for 2.7.

@karthiknadig karthiknadig merged commit d5f0dc5 into microsoft:main Nov 8, 2021
wesm pushed a commit to posit-dev/positron that referenced this pull request Mar 28, 2024
…-python#17898)

* Use same --rootdir logic as test discovery

* Add news entry
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.

3 participants