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

Document --basetemp directory-clearaing behavior better #7554

Closed
mysterious-progression opened this issue Jul 28, 2020 · 4 comments · Fixed by #7555
Closed

Document --basetemp directory-clearaing behavior better #7554

mysterious-progression opened this issue Jul 28, 2020 · 4 comments · Fixed by #7555
Labels
good first issue easy issue that is friendly to new contributor type: docs documentation improvement, missing or needing clarification

Comments

@mysterious-progression
Copy link
Contributor

mysterious-progression commented Jul 28, 2020

[Edit by @bluetech:] The --basetemp documentation does not mention the fact that it clears the directory. The user should be aware of this. Also, the --help output of --bastemp can be improved.

Original issue:


To make temporary directories in locations other than /tmp you can use the --basetemp flag to assign this directory. However, after running your test, pytest will deleted EVERYTHING in that folder, not just the files/directories created by the test. The documentation does not specify this behavior and I believe this is a serious bug. Deleting files and folders not created by pytest should not be default behavior.

To replicate create this file, test_bug.py

import pytest
import os

def test_create_file(tmp_path):
    d = tmp_path / "sub"
    d.mkdir()
    p = d / "hello.txt"
    p.write_text("sh")
    assert p.read_text() == "sh"


Then in the directory containing this file run pytest --basetemp=. -s test_bug.py
MacOS 10.15.5,
pip list
Package Version


appdirs 1.4.4
attrs 19.3.0
CacheControl 0.12.6
cachy 0.3.0
certifi 2020.6.20
cfgv 3.1.0
chardet 3.0.4
cleo 0.7.6
clikit 0.4.3
Deprecated 1.2.10
detect-secrets 0.14.1
distlib 0.3.1
filelock 3.0.12
html5lib 1.1
identify 1.4.25
idna 2.10
jsonschema 3.2.0
keyring 20.0.1
lockfile 0.12.2
more-itertools 8.4.0
msgpack 1.0.0
nodeenv 1.4.0
packaging 20.4
pastel 0.2.0
pexpect 4.8.0
pip 20.1.1
pkginfo 1.5.0.1
pluggy 0.13.1
poetry 1.0.10
pre-commit 2.6.0
ptyprocess 0.6.0
py 1.9.0
PyGithub 1.51
PyJWT 1.7.1
pylev 1.3.0
pyparsing 2.4.7
pyrsistent 0.14.11
pytest 5.4.3
PyYAML 5.3.1
requests 2.24.0
requests-toolbelt 0.8.0
ruamel.yaml 0.16.10
ruamel.yaml.clib 0.2.0
setuptools 49.2.0
shellingham 1.3.2
six 1.15.0
toml 0.10.1
tomlkit 0.5.11
urllib3 1.25.10
virtualenv 20.0.27
wcwidth 0.2.5
webencodings 0.5.1
wheel 0.34.2
wrapt 1.12.1

@The-Compiler
Copy link
Member

FWIW pytest --help says:

  --basetemp=dir        base temporary directory for this test run.(warning:
                        this directory is removed if it exists)

Also see #7119 for some more discussion. With pytest 6 and #7170, it also tries to prevent accidents:

Exit with an error if the --basetemp argument is empty, the current working directory or parent directory of it. This is done to protect against accidental data loss, as any directory passed to this argument is cleared.

So I believe the only thing left would perhaps be a warning in the tmpdir docs - is that the part of documentation where you looked, @mattreex?

@mysterious-progression
Copy link
Contributor Author

mysterious-progression commented Jul 28, 2020

@The-Compiler Correct. If this is expected behavior, it should be referenced in the documentation. Intuitively, you would expect only the documents and directories created by pytest to be removed. However, as a non-expert on this, I am not sure it is necessary to delete all the contents in the specified directory.

As aside note this is warning is not correct, strictly speaking.

--basetemp=dir        base temporary directory for this test run.(warning:
                        this directory is removed if it exists)

Since it is the contents of the directory, not the directory itself that is removed.

@RonnyPfannschmidt
Copy link
Member

i do wonder if pytest should maintain a number of backup dirs it renames the basetmp into before dropping files

@bluetech bluetech changed the title Pytest deleted EVERYTHING in the folder assigned using --basetemp Document --basetemp directory-clearaing behavior better Jul 28, 2020
@bluetech bluetech added good first issue easy issue that is friendly to new contributor type: docs documentation improvement, missing or needing clarification labels Jul 28, 2020
@bluetech
Copy link
Member

I edited the title, added some text to the top of the issue and applied the status: easy label. Hopefully someone will help with a PR for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue easy issue that is friendly to new contributor type: docs documentation improvement, missing or needing clarification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants