Repository: pyvista/pytest-pyvista (language: python)

Issue:
Default cache_dir value could be better?
IMO, this default value is not the best choice:

https://github.com/pyvista/pytest-pyvista/blob/374c284d67c962bdb516a77e661f415c30011b47/pytest_pyvista/pytest_pyvista.py#L88-L89

I'd like to request some clarification on this choice and propose altering it to use the current working directory.

I would expect `pytest .`  to properly handle the image cache or default to save the cache in a location that is visible/relative to the active project. This current default dumps the images in a non-visible location.

An alternative option is to have `cache_dir` be easily stable in the `conftest.py` for a project -- is this currently possible?


Discussion / hints:
As someone who only understands what's going on due to slack: the current default means that the image cache included in the repo is not used at all unless it's explicitly requested with a command-line argument. So just running `pytest` locally silently ignores the image cache, which is what we probably never want to be the default.
Precisely. Thank you for clarifying, @adeak 
cc @pyvista/developers 
I think there should not be a default at all for where the cached images are, it should be a required user setting. We should have a default directory for where new images are placed and compared to the cached images.  As in the suggestions here:

https://github.com/pyvista/pytest-pyvista/pull/3#discussion_r1019650824
Sounds like that would also fix https://github.com/pyvista/pytest-pyvista/issues/17 @MatthewFlamm.

Fix patch:
diff --git a/.github/workflows/ci_cd.yml b/.github/workflows/ci_cd.yml
index 9767536..7e88a55 100644
--- a/.github/workflows/ci_cd.yml
+++ b/.github/workflows/ci_cd.yml
@@ -27,7 +27,7 @@ jobs:
       matrix:
         include:
           - python-version: '3.7'
-            vtk-version: '8.1.2'
+            vtk-version: '9.0.3' # v8.x not supported
           - python-version: '3.8'
             vtk-version: '9.0.3'
           - python-version: '3.9'
diff --git a/.gitignore b/.gitignore
index 24fb93f..80bc47f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -77,3 +77,7 @@ target/
 
 # vscode
 .vscode/
+
+# virtual environment
+venv/
+.venv/
diff --git a/pyproject.toml b/pyproject.toml
index 4149e39..4149f18 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -33,7 +33,8 @@ tests = [
     "coverage==6.5",
     "pytest>=3.5.0",
     "pytest-cov==4.0.0",
-    "pyvista>=0.37"
+    "pyvista>=0.37",
+    "numpy<1.24",
 ]
 
 [project.entry-points."pytest11"]


Test patch:
diff --git a/pytest_pyvista/pytest_pyvista.py b/pytest_pyvista/pytest_pyvista.py
index 14b18b7..698c48b 100644
--- a/pytest_pyvista/pytest_pyvista.py
+++ b/pytest_pyvista/pytest_pyvista.py
@@ -33,6 +33,11 @@ def pytest_addoption(parser):
         action="store",
         help="Path to the image cache folder.",
     )
+    parser.addini(
+        "image_cache_dir",
+        default="image_cache_dir",
+        help="Path to the image cache folder.",
+    )
 
 
 class VerifyImageCache:
@@ -70,11 +75,10 @@ class VerifyImageCache:
     windows_skip_image_cache = False
     macos_skip_image_cache = False
 
-    cache_dir = None
-
     def __init__(
         self,
         test_name,
+        cache_dir,
         *,
         error_value=500,
         warning_value=200,
@@ -83,15 +87,10 @@ def __init__(
     ):
         self.test_name = test_name
 
-        if self.cache_dir is None:
-            # Reset image cache with new images
-            this_path = pathlib.Path(__file__).parent.absolute()
-            self.cache_dir = os.path.join(this_path, "image_cache")
-
-            # not working with pytest-pyvista tests for some reason
-            # self.cache_dir = appdirs.user_cache_dir(appname="pytest-pyvista", appauthor="pyvista")
+        self.cache_dir = cache_dir
 
         if not os.path.isdir(self.cache_dir):
+            warnings.warn(f"pyvista test cache image dir: {self.cache_dir} does not yet exist'  Creating empty cache.")
             os.mkdir(self.cache_dir)
 
         self.error_value = error_value
@@ -114,9 +113,8 @@ def __call__(self, plotter):
         if self.skip:
             return
 
-        # Image cache is only valid for VTK9+
         if not VTK9:
-            return
+            raise RuntimeError("Image cache is only valid for VTK9+")
 
         if self.ignore_image_cache:
             return
@@ -177,8 +175,11 @@ def verify_image_cache(request, pytestconfig):
     VerifyImageCache.fail_extra_image_cache = pytestconfig.getoption(
         "fail_extra_image_cache"
     )
-    VerifyImageCache.cache_dir = pytestconfig.getoption("image_cache_dir")
 
-    verify_image_cache = VerifyImageCache(request.node.name)
+    cache_dir = pytestconfig.getoption("image_cache_dir")
+    if cache_dir is None:
+        cache_dir = pytestconfig.getini("image_cache_dir")
+
+    verify_image_cache = VerifyImageCache(request.node.name, cache_dir)
     pyvista.global_theme.before_close_callback = verify_image_cache
     return verify_image_cache
diff --git a/tests/test_pyvista.py b/tests/test_pyvista.py
index 8575554..c0071e4 100644
--- a/tests/test_pyvista.py
+++ b/tests/test_pyvista.py
@@ -1,5 +1,10 @@
 # -*- coding: utf-8 -*-
+import os
+import pytest
 
+import pyvista as pv
+
+pv.OFF_SCREEN = True
 
 def test_arguments(testdir):
     """Test pytest arguments"""
@@ -18,42 +23,115 @@ def test_args(verify_image_cache):
     result.stdout.fnmatch_lines("*[Pp]assed*")
 
 
+def make_cached_images(test_path, path="image_cache_dir"):
+    """Makes image cache in `test_path\path`."""
+    d = os.path.join(test_path, path)
+    os.mkdir(d)
+    sphere = pv.Sphere()
+    plotter = pv.Plotter()
+    plotter.add_mesh(sphere, color="red")
+    plotter.screenshot(os.path.join(d, "imcache.png"))
+
+
 def test_verify_image_cache(testdir):
     """Test regular usage of the `verify_image_cache` fixture"""
+    make_cached_images(testdir.tmpdir)
     testdir.makepyfile(
         """
         import pyvista as pv
         pv.OFF_SCREEN = True
         def test_imcache(verify_image_cache):
             sphere = pv.Sphere()
-            pv.plot(sphere.points)
             plotter = pv.Plotter()
-            plotter.add_points(sphere.points)
-            plotter.add_points(sphere.points + 1)
+            plotter.add_mesh(sphere, color="red")
             plotter.show()
         """
     )
 
-    result = testdir.runpytest()
+    result = testdir.runpytest("--fail_extra_image_cache")
     result.stdout.fnmatch_lines("*[Pp]assed*")
 
 
+def test_verify_image_cache_fail_regression(testdir):
+   """Test regression of the `verify_image_cache` fixture"""
+   make_cached_images(testdir.tmpdir)
+   testdir.makepyfile(
+       """
+       import pytest
+       import pyvista as pv
+       pv.OFF_SCREEN = True
+       def test_imcache(verify_image_cache):
+           sphere = pv.Sphere()
+           plotter = pv.Plotter()
+           plotter.add_mesh(sphere, color="blue")
+           plotter.show()
+       """
+   )
+
+   result = testdir.runpytest("--fail_extra_image_cache")
+   result.stdout.fnmatch_lines("*[Ff]ailed*")
+   result.stdout.fnmatch_lines("*Exceeded image regression error*")
+
+
 def test_skip(testdir):
     """Test `skip` flag of `verify_image_cache`"""
+    make_cached_images(testdir.tmpdir)
     testdir.makepyfile(
         """
+        import pytest
         import pyvista as pv
         pv.OFF_SCREEN = True
         def test_imcache(verify_image_cache):
             verify_image_cache.skip = True
             sphere = pv.Sphere()
-            pv.plot(sphere.points)
             plotter = pv.Plotter()
-            plotter.add_points(sphere.points)
-            plotter.add_points(sphere.points + 1)
+            plotter.add_mesh(sphere, color="blue")
+            plotter.show()
+         """
+    )
+
+    result = testdir.runpytest("--fail_extra_image_cache")
+    result.stdout.fnmatch_lines("*[Pp]assed*")
+
+
+def test_image_cache_dir_commandline(testdir):
+    """Test setting image_cache_dir via CLI option."""
+    make_cached_images(testdir.tmpdir, "newdir")
+    testdir.makepyfile(
+        """
+        import pyvista as pv
+        pv.OFF_SCREEN = True
+        def test_imcache(verify_image_cache):
+            sphere = pv.Sphere()
+            plotter = pv.Plotter()
+            plotter.add_mesh(sphere, color="red")
             plotter.show()
         """
     )
 
-    result = testdir.runpytest()
+    result = testdir.runpytest("--fail_extra_image_cache", "--image_cache_dir", "newdir")
+    result.stdout.fnmatch_lines("*[Pp]assed*")
+
+
+def test_image_cache_dir_ini(testdir):
+    """Test setting image_cache_dir via config."""
+    make_cached_images(testdir.tmpdir, "newdir")
+    testdir.makepyfile(
+        """
+        import pyvista as pv
+        pv.OFF_SCREEN = True
+        def test_imcache(verify_image_cache):
+            sphere = pv.Sphere()
+            plotter = pv.Plotter()
+            plotter.add_mesh(sphere, color="red")
+            plotter.show()
+        """
+    )
+    testdir.makepyprojecttoml(
+        """
+        [tool.pytest.ini_options]
+        image_cache_dir = "newdir"
+        """
+    )
+    result = testdir.runpytest("--fail_extra_image_cache")
     result.stdout.fnmatch_lines("*[Pp]assed*")