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

[luizyao]

Description

Before I used --cache-clear option, my cache folder looked like this:

.pytest_cache/
├── .gitignore
├── CACHEDIR.TAG
├── README.md
└── v
    └── cache
        ├── nodeids
        └── stepwise

2 directories, 5 files

then I add --cache-clear option to re-execute, my cache folder looks like this：

.pytest_cache/
└── v
    └── cache
        ├── nodeids
        └── stepwise

2 directories, 2 files

Three files are missing: .gitignore CACHEDIR.TAG and README.md.

This lets me commit files that I don't want to upload while using git commit.

Question

I check the src/_pytest/cacheprovider.py file in source codes.

    @classmethod
    def for_config(cls, config):
        cachedir = cls.cache_dir_from_config(config)
        if config.getoption("cacheclear") and cachedir.exists():
            rm_rf(cachedir)
            cachedir.mkdir()
        return cls(cachedir, config)

I think this issue is caused by this line: cachedir.mkdir(), it will make cache_dir_exists_already True, so that _ensure_supporting_files function not to be called.

       try:
            if path.parent.is_dir():
                cache_dir_exists_already = True
            else:
                cache_dir_exists_already = self._cachedir.exists()
                path.parent.mkdir(exist_ok=True, parents=True)
        except (IOError, OSError):
            self.warn("could not create cache path {path}", path=path)
            return
        if not cache_dir_exists_already:
            self._ensure_supporting_files()

I don't understand why this operation was added. Is there anything else I don't know about it？

[nicoddemus]

Hi @luizyao,

Thanks for the detailed description, appreciate it.

It seems the problem is just that we are not calling self._ensure_supporting_files() is called after cachedir.mkdir() in for_config. That should be enough to fix the problem.

Would you like to open a PR with that fix?

[luizyao]

Hi, @nicoddemus:

I don't think it's a good idea to call an instance method(self._ensure_supporting_files()) in a classmethod(for_config).

How about this:

        try:
            if path.parent.is_dir():
                cache_dir_exists_already = True
            else:
                # cache_dir_exists_already = self._cachedir.exists() 
                # if self._cache is an exist non-empty dir, make cache_dir_exists_already true 
                cache_dir_exists_already = self._cachedir.exists() and next(self._cachedir.glob('*'), None)
                path.parent.mkdir(exist_ok=True, parents=True)
        except (IOError, OSError):
            self.warn("could not create cache path {path}", path=path)
            return
        if not cache_dir_exists_already:
            self._ensure_supporting_files()

[blueyed]

IIRC the idea is to not re-create the ensuring files if the cache dir exists already (likely more information might be available from git-blame).
I think --cache-clear should not remove the "supporting files" instead.

[blueyed]

I suggest this:

diff --git i/src/_pytest/cacheprovider.py w/src/_pytest/cacheprovider.py
index db086fc2b..543e292e3 100755
--- i/src/_pytest/cacheprovider.py
+++ w/src/_pytest/cacheprovider.py
@@ -50,6 +50,10 @@ def for_config(cls, config):
         if config.getoption("cacheclear") and cachedir.exists():
             rm_rf(cachedir)
             cachedir.mkdir()
+        if config.getoption("cacheclear"):
+            for d in [cachedir.joinpath("d"), cachedir.joinpath("v")]:
+                if d.exists():
+                    rm_rf(d)
         return cls(cachedir, config)

     @staticmethod
diff --git i/testing/test_cacheprovider.py w/testing/test_cacheprovider.py
index 6bf4f5f19..7fc876ea6 100644
--- i/testing/test_cacheprovider.py
+++ w/testing/test_cacheprovider.py
@@ -272,6 +272,7 @@ def test_3(): assert 0
         )
         result = testdir.runpytest(str(p), "--lf", "--cache-clear")
         result.stdout.fnmatch_lines(["*1 failed*2 passed*"])
+        assert testdir.tmpdir.join(".pytest_cache", "README.md").exists()

         # Run this again to make sure clear-cache is robust
         if os.path.isdir(".pytest_cache"):

[RonnyPfannschmidt]

Another approach would be to drop the mkdir after the clear, then the normal setup would make the cache dir

This trades less io in the general case for potentially not having all help files in the cache dir

[nicoddemus]

Another approach would be to drop the mkdir after the clear, then the normal setup would make the cache dir

I like this one.

potentially not having all help files in the cache dir

Not sure, the directory itself won't exist at all, and _ensure_supporting_files will create all files then right?

[nicoddemus]

Went ahead with this in #6296, we can continue the discussion there.