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

Testing: pytest should be running multiple nodes in one single process #19796

Closed
lachaib opened this issue Sep 7, 2022 · 6 comments
Closed
Assignees
Labels
area-testing bug Issue identified by VS Code Team member as probable bug

Comments

@lachaib
Copy link

lachaib commented Sep 7, 2022

Sorry if I missed an existing feature request with my searches I couldn't find one.

Hello,

When selecting multiple nodes to run tests, one would/could expect all of them to be run in one single process instantiation, for the following reasons:

  • performances: preparing some resources (data, loading modules on a big codebase...) may take some time and it is possible to have this time done once for all instead of once for each
  • reporting: one may want to get the coverage report generated by a selection of test nodes and coverage can be consolidated automatically with python-coverage instead of having to manually set coverage file option and then consolidate several files into one
  • isolation: it happens, despite efforts, that some tests don't behave the same when run together instead of one by one. To reproduce isolation issues that may be encountered later in CI, it'd be useful to be able to run in same conditions.

I've specified pytest in this issue although I think it may be valid for other runners, because I'm only using pytest and I know it's possible to specify several tests in command-line invocation.

Thinking about the case where people may expect parallel runs and consider current state as a feature, I think they should rather consider options like pytest-xdist instead.

I dug in the code, and, basically, it would mean that instead of having runTests do an await of delegated calls to runTest for each node, it may directly invoke pytest with all nodes in arguments.
Maybe even, if a parent and a child node are selected together only parent node could be passed to invocation, to avoid user mistakes leading to confusion.

In case this feature is accepted and needs external contribution, I may be able to code it

Thank you 🙂

@lachaib lachaib added the feature-request Request for new features or functionality label Sep 7, 2022
@github-actions github-actions bot added the triage-needed Needs assignment to the proper sub-team label Sep 7, 2022
@karthiknadig karthiknadig added bug Issue identified by VS Code Team member as probable bug area-testing and removed feature-request Request for new features or functionality labels Sep 8, 2022
@eleanorjboyd
Copy link
Member

Hello! We are currently working on a rewrite of how we do pytest discovery/execution which will fix this issue. Here is the issue which outlines that work #17242 and the currently issue which is being worked on to prototype that functionality #19790.

@eleanorjboyd eleanorjboyd removed the triage-needed Needs assignment to the proper sub-team label Sep 8, 2022
@lachaib
Copy link
Author

lachaib commented Sep 12, 2022

Hello @eleanorjboyd, so I understand this ticket is added to the meta-issue, and we can keep it open until done?

Again if you need a contribution I may be able to give it a try, otherwise I let you reach back to me when it's done.

Thanks

@eleanorjboyd
Copy link
Member

Yes, we will keep this ticket open until the issue is fixed. Thank you for your offer to help, we will let you know if anything comes up especially related to verifying this bug is resolved once the fix is pushed.

@schlamar
Copy link

Not sure if this is the exact same issue and if this will be fixed with the linked ticket. I noticed some strange behavior with unexpected parallel test execution.

Minimal pytest example:

import time
import pytest

@pytest.mark.parametrize("data", (1, 2))
def test_parallel(data):
    time.sleep(15)

Running this with the editor icon beside the line number it spawns two Python processes for the 2 tests (you can verify this with Process Explorer for example or just by checking the Python test output). This behavior with parallel execution is unexpected as this might break tests if shared resources are involved (in my case access to shared memory).

image

Running the test in the testing panel it spawns only one Python test process as expected.

image

@lachaib
Copy link
Author

lachaib commented Sep 21, 2022

Yes I get the same because explorer sees 2 tests at the same line (happens also when a test is a method of a class that gets inherited)
And so 2 nodes, and so running 2 parallel runs...

@eleanorjboyd
Copy link
Member

Hello! I have reviewed this issue and have tested a similar scenario successfully on our testing rewrite. Therefore I am going to close this issue but please comment or open a new issue if you are still seeing a problem when you try this yourself!

To use the rewrite yourself just add ”python.experiments.optInto": ["pythonTestAdapter"] to your user settings. You can confirm you have the rewrite enabled by setting your log level to trace, via the Developer: Set Log Level command in the command palette. Then check to see if Experiment 'pythonTestAdapter' is active is in your python logs.

Thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-testing bug Issue identified by VS Code Team member as probable bug
Projects
None yet
Development

No branches or pull requests

4 participants