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

tmp_path: basetemp with retention #10829

Open
soxofaan opened this issue Mar 22, 2023 · 4 comments
Open

tmp_path: basetemp with retention #10829

soxofaan opened this issue Mar 22, 2023 · 4 comments
Labels
plugin: tmpdir related to the tmpdir builtin plugin status: help wanted developers would like help from experts on this topic type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature

Comments

@soxofaan
Copy link
Contributor

soxofaan commented Mar 22, 2023

from the tmp_path how-to docs:

Temporary directories are by default created as sub-directories of the system temporary directory. ... entries older than 3 temporary directories will be removed. ... can be configured with tmp_path_retention_count ...

Using the --basetemp option will remove the directory before every run, effectively meaning the temporary directories
of only the most recent run will be kept.

Is there any particular reason that the retention feature (keep 3 by default) is not supported when --basetemp is used?
It seems like these features should be orthogonal, but that's not the case which is counter-intuitive.

Current workaround is apparently to use the PYTEST_DEBUG_TEMPROOT env var instead of --basetemp but that feature is not documented as far as I could find.

I understand that there are concerns about backward compatibility, but I think basetemp+retention can be implented in a backward compatible way:

  • use --basetemp, but no tmp_path_retention_count config set: use custom temp root without retention (existing behavior)
  • use --basetemp and tmp_path_retention_count: use custom temp root with retention (this feature request)
@RonnyPfannschmidt
Copy link
Member

RonnyPfannschmidt commented Mar 22, 2023

that behaviour is basically from about 10-15 years ago, nobody ever sat down and designed a replacement that dont share the downsides

i'd like to propose 2 potential solutions

  1. introduce a temproot option - it defaults to $TMP/pytest-of-$user
    or the content of the env var PYTEST_DEBUG_TEMPROOT
  2. introduce a --make-basetemp=./somename option
    it will always create the given folder, and it will always fail on preexisting folders

@soxofaan
Copy link
Contributor Author

  1. introduce a temproot option - it defaults to $TMP/pytest-of-$user or the content of the env var PYTEST_DEBUG_TEMPROOT

How is that different from the existing basetemp option? I'm not sure I understand how that would address my question

  1. introduce a --make-basetemp=./somename option it will always create the given folder, and it will always fail on preexisting folders

I think you have a different use case or modus operandi in mind here, especially the "fail on preexisting folders" seems specific to certain CI/CD workflows, I'm guessing.

@RonnyPfannschmidt
Copy link
Member

  1. Temproot and basetemp refer to different levels in the tmpdir

Basetemp stands in for a numbered per test run folder

However it nukes whatever it finds there

Fail on preexisting is very relevant, it's very easy to accidentally have things run against the wrong folder

People have destroyed their source trees or other folders when misusing the option

@Zac-HD Zac-HD added status: help wanted developers would like help from experts on this topic type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature plugin: tmpdir related to the tmpdir builtin plugin labels Apr 8, 2023
soxofaan added a commit to soxofaan/pytest that referenced this issue Oct 23, 2024
Improve coverage of current handling of `--basetemp` option and its lack
of retention functionality.
@soxofaan
Copy link
Contributor Author

Temproot and basetemp refer to different levels in the tmpdir

thanks, makes sense now.

I guess that this feature request (or any other equivalent improvement) will not be fixed quickly, so I decided to at least document the current behavior better in #12912

soxofaan added a commit to soxofaan/pytest that referenced this issue Oct 28, 2024
nicoddemus added a commit that referenced this issue Oct 31, 2024
Improve coverage of current handling of `--basetemp` option and its lack of retention functionality.

Also document `PYTEST_DEBUG_TEMPROOT`.

Related to #10829

---------

Co-authored-by: Bruno Oliveira <[email protected]>
patchback bot pushed a commit that referenced this issue Oct 31, 2024
Improve coverage of current handling of `--basetemp` option and its lack of retention functionality.

Also document `PYTEST_DEBUG_TEMPROOT`.

Related to #10829

---------

Co-authored-by: Bruno Oliveira <[email protected]>
(cherry picked from commit a1a4918)
nicoddemus pushed a commit that referenced this issue Oct 31, 2024
Improve coverage of current handling of `--basetemp` option and its lack of retention functionality.

Also document `PYTEST_DEBUG_TEMPROOT`.

Related to #10829

---------

Co-authored-by: Bruno Oliveira <[email protected]>
(cherry picked from commit a1a4918)

Co-authored-by: Stefaan Lippens <[email protected]>
Glyphack pushed a commit to Glyphack/pytest that referenced this issue Nov 23, 2024
Improve coverage of current handling of `--basetemp` option and its lack of retention functionality.

Also document `PYTEST_DEBUG_TEMPROOT`.

Related to pytest-dev#10829

---------

Co-authored-by: Bruno Oliveira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: tmpdir related to the tmpdir builtin plugin status: help wanted developers would like help from experts on this topic type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature
Projects
None yet
Development

No branches or pull requests

3 participants