man-group / pytest-plugins

A grab-bag of nifty pytest plugins
MIT License
559 stars 83 forks source link

pytest-virtualenv unsafe defaults with explicit workspace #195

Closed LilyFoote closed 2 years ago

LilyFoote commented 2 years ago

Hi! I've been trying to use pytest-virtualenv, but I've been running into some issues. One is the known issue with pip install . (#163). The other is a nasty data loss bug when using an explicit workspace:

I have a directory in which I have a very simple test suite. I am trying to test running that test suite via an external program similar to coverage. With coverage, one can do coverage run -m unittest, and I'm trying to implement a similar tool.

In my main test suite I'm trying to do something like:

def test_run_unittest_suite(virtualenv):
    virtualenv.install_package("my_package")
    result = virtualenv.run("my_package run -m unittest", capture=True)
    ...  # assert some things

The first issue I ran into is that I need to run from the same directory as the example test suite. I can do this with VirtualEnv(workspace="project-dir"):

from pytest_virtualenv import VirtualEnv

def test_run_unittest_suite():
    virtualenv = VirtualEnv(workspace="project-dir")
    # virtualenv.install_package("my_package")  # skip trying to install for now
    # Use python -m unittest instead as a starting point
    result = virtualenv.run("python -m unittest", capture=True)
    ...

This works, for one test run, but deletes project-dir when cleaning up virtualenv. As you might be able to imagine, this really confused me for a while, until I figured out what was going on. Thankfully, the code in my project-dir was easy enough to re-create.

As a workaround, I discovered that I can do:

from pytest_virtualenv import VirtualEnv

def test_run_unittest_suite():
    virtualenv = VirtualEnv(workspace="project-dir")
    virtualenv.delete = False
    result = virtualenv.run("python -m unittest", capture=True)
    ...

However, I think this would be better fixed here.

The first fix I recommend is exposing the delete parameter from Workspace in VirtualEnv:

from pytest_virtualenv import VirtualEnv

def test_run_unittest_suite():
    virtualenv = VirtualEnv(workspace="project-dir", delete=False)
    ...

I also suggest changing the default behaviour when workspace is explicitly provided to delete=False. delete=True when the workspace is truly temporary is great, but when the workspace is persistent it's a nasty trap for the unwary.

eeaston commented 2 years ago

Hi there - thanks for raising! Agreed this is surprising behaviour, your suggestion of exposing the delete parameter is a good one, same for defaulting it to False when a workspace is provided.