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

Ensure cache supporting files still exist after --cache-clear #6296

Merged

Conversation

nicoddemus
Copy link
Member

Fix #6290

@blueyed
Copy link
Contributor

blueyed commented Nov 30, 2019

I've thought about doing it that way also initially, but this still removes e.g. the .git I have there - and I think the idea with the supporting files was to only create them initially (for a new cache base dir), and then do not mess with them, unless the cache root gets created again.
Therefore I think "clearing the cache" should only clear the cache (contents/data), not remove the base directory and everything in it (above the cache data itself).
rm -rf .pytest-cache can be used easily if you want to remove everything really.

@blueyed
Copy link
Contributor

blueyed commented Nov 30, 2019

To be clear: this fixes the initial / reported issue, but from my point of view there's another ("it should only remove the cache contents"), which I've tried to address also then in my patch.

@nicoddemus
Copy link
Member Author

I've thought about doing it that way also initially, but this still removes e.g. the .git I have there

Sorry, which .git you mean? The .pytest_cache directory as far as I concerned is owned by pytest, so users shouldn't really be putting custom files/directories there, if that's the case.

it should only remove the cache contents"

I see and it is really not that complicated, but on the other hand I think most people expect --clear-cache to wipe everything (and it is simpler). Plus by doing that we ensure users get updated files when that applies.

As I said, if even with above points you still think we should only wipe d and v directories, I won't discuss further and will update the PR. 😁

@blueyed
Copy link
Contributor

blueyed commented Nov 30, 2019

I've thought about doing it that way also initially, but this still removes e.g. the .git I have there

Sorry, which .git you mean? The .pytest_cache directory as far as I concerned is owned by pytest, so users shouldn't really be putting custom files/directories there, if that's the case.

I've put it into a Git repo to e.g. stash last-failed state etc.. IIRC I've posted a script to the mailing-list for this initially. It also helps with debugging/seeing issues with e.g. the lf plugin.

Plus by doing that we ensure users get updated files when that applies.

That's a good point, but it also renoves e.g. a customized .gitignore (imaginary use case probably only).

I'd prefer it to only remove the contents still. But then those could be put into constants / a list maybe (using "d" and "v" is not very descriptive, also where it is used).

@nicoddemus
Copy link
Member Author

I'd prefer it to only remove the contents still.

OK, done. 👍

Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

👍

@blueyed
Copy link
Contributor

blueyed commented Dec 1, 2019

You might want to state this explicitly in the changelog also (the second fix/improvement).

changelog/6290.bugfix.rst Outdated Show resolved Hide resolved
@nicoddemus nicoddemus merged commit 23f6adc into pytest-dev:master Dec 1, 2019
@nicoddemus nicoddemus deleted the clear-cache-supporting-files branch December 1, 2019 13:37
blueyed added a commit to blueyed/pytest that referenced this pull request Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'--cache-clear' option to regenerate the cache folder is missing some files
3 participants