Add --generate_subdirs option for doc mode

README.rst

@@ -323,6 +323,14 @@ These are the flags you can use when calling ``pytest`` in the command line:
     These are the pre-processed images generated for the tests. They are `not` the images
     generated by a documentation build (use ``--doc_images_dir`` for specifying that).
 
+* ``--doc_generate_subdirs`` saves generated test images in separate sub-directories
+  instead of saving them directly to the ``doc_generated_image_dir``. Without this option,
+  generated images are saved as ``<doc_generated_image_dir>/<test_name>.png``; with this
+  option enabled, they are instead saved as
+  ``<doc_generated_image_dir>/<test_name>/<image_name>.png``, where the image name has the format
+  ``<os-version>_<machine>_<gpu-vendor>_<python-version>_<pyvista-version>_<vtk-version>_<using-ci>``.
+  This can be useful for providing context about how an image was generated.
+
 * ``--doc_failed_image_dir <DIR>`` dumps copies of cached and generated test images when
   there is a warning or error raised. This directory is useful for reviewing test
   failures. It is relative to `pytest root path <https://docs.pytest.org/en/latest/reference/reference.html#pytest.Config.rootpath>`.
@@ -379,6 +387,14 @@ Configure the image format to be ``jpg`` for both unit tests and when using ``--
     image_format = "jpg"
     doc_image_format = "jpg"
 
+Enable the generation of test images inside of sub-directories for both unit tests and when using ``--doc_mode``.
+
+.. code-block:: toml
+
+    [tool.pytest.ini_options]
+    generate_subdirs = True
+    doc_generate_subdirs = True
+
 Contributing
 ------------
 Contributions are always welcome. Tests can be run with `tox`_, please ensure

pytest_pyvista/doc_mode.py

@@ -18,7 +18,9 @@
 from .pytest_pyvista import DEFAULT_ERROR_THRESHOLD
 from .pytest_pyvista import DEFAULT_WARNING_THRESHOLD
 from .pytest_pyvista import _check_compare_fail
+from .pytest_pyvista import _EnvInfo
 from .pytest_pyvista import _get_file_paths
+from .pytest_pyvista import _get_generated_image_path
 from .pytest_pyvista import _get_option_from_config_or_ini
 from .pytest_pyvista import _ImageFormats
 from .pytest_pyvista import _test_compare_images
@@ -31,11 +33,12 @@ class _DocModeInfo:
     doc_image_cache_dir: Path
     doc_generated_image_dir: Path
     doc_failed_image_dir: Path
+    doc_generate_subdirs: bool
     doc_image_format: _ImageFormats
     _tempdirs: ClassVar[list[tempfile.TemporaryDirectory]] = []
 
     @classmethod
-    def init_dirs(cls, config: pytest.Config) -> None:
+    def init_from_config(cls, config: pytest.Config) -> None:
         def require_existing_dir(option: str) -> Path:
             """Fetch a required directory option and ensure it's valid."""
             path = _get_option_from_config_or_ini(config, option, is_dir=True)
@@ -62,6 +65,9 @@ def optional_dir_with_temp(option: str, prefix: str) -> Path:
         cls.doc_generated_image_dir = optional_dir_with_temp("doc_generated_image_dir", prefix="pytest_doc_generated_image_dir")
         cls.doc_failed_image_dir = optional_dir_with_temp("doc_failed_image_dir", prefix="pytest_doc_failed_image_dir")
 
+        cls.doc_image_format = cast("_ImageFormats", _get_option_from_config_or_ini(config, "doc_image_format"))
+        cls.doc_generate_subdirs = bool(_get_option_from_config_or_ini(config, "doc_generate_subdirs"))
+
 
 class _TestCaseTuple(NamedTuple):
     test_name: str
@@ -73,7 +79,9 @@ def _flatten_path(path: Path) -> Path:
     return Path("_".join(path.parts))
 
 
-def _preprocess_build_images(build_images_dir: Path, output_dir: Path, image_format: _ImageFormats = "png") -> list[Path]:
+def _preprocess_build_images(
+    build_images_dir: Path, output_dir: Path, *, image_format: _ImageFormats = "png", generate_subdirs: bool = False
+) -> list[Path]:
     """
     Read images from the build dir, resize them, and save to a flat output dir.
 
@@ -85,13 +93,15 @@ def _preprocess_build_images(build_images_dir: Path, output_dir: Path, image_for
     input_gif = _get_file_paths(build_images_dir, ext="gif")
     input_jpg = _get_file_paths(build_images_dir, ext="jpg")
     output_paths = []
-    output_dir.mkdir(exist_ok=True)
     for input_path in input_png + input_gif + input_jpg:
+        output_dir.mkdir(exist_ok=True)
         # input image from the docs may come from a nested directory,
         # so we flatten the file's relative path
         output_file_name = _flatten_path(input_path.relative_to(build_images_dir))
         output_file_name = output_file_name.with_suffix("." + image_format)
-        output_path = output_dir / output_file_name
+        output_path = _get_generated_image_path(
+            parent=output_dir, image_name=output_file_name, generate_subdirs=generate_subdirs, env_info=_EnvInfo()
+        )
         output_paths.append(output_path)
 
         # Ensure image size is max 400x400 and save to output
@@ -130,10 +140,14 @@ def add_to_dict(filepath: Path, key: str) -> None:
         test_cases_dict[test_name].setdefault(key, filepath)
 
     # process test images
+    generate_subdirs = _DocModeInfo.doc_generate_subdirs
     test_image_paths = _preprocess_build_images(
-        _DocModeInfo.doc_images_dir, _DocModeInfo.doc_generated_image_dir, image_format=_DocModeInfo.doc_image_format
+        _DocModeInfo.doc_images_dir,
+        _DocModeInfo.doc_generated_image_dir,
+        image_format=_DocModeInfo.doc_image_format,
+        generate_subdirs=generate_subdirs,
     )
-    [add_to_dict(path, "docs") for path in test_image_paths]  # type: ignore[func-returns-value]
+    [add_to_dict(path.parent if generate_subdirs else path, "docs") for path in test_image_paths]  # type: ignore[func-returns-value]
 
     # process cached images
     cache_dir = _DocModeInfo.doc_image_cache_dir
@@ -181,12 +195,14 @@ def _save_failed_test_image(source_path: Path, category: Literal["warnings", "er
         rel = source_path.relative_to(_DocModeInfo.doc_image_cache_dir)
         dest_relative_dir = Path("from_cache") / rel.parent
     else:
-        dest_relative_dir = Path("from_build")
+        rel = source_path.relative_to(_DocModeInfo.doc_generated_image_dir)
+        dest_relative_dir = Path("from_build") / rel.parent
 
     dest_dir = _DocModeInfo.doc_failed_image_dir / category / dest_relative_dir
     dest_dir.mkdir(exist_ok=True, parents=True)
     dest_path = dest_dir / source_path.name
-    shutil.copy(source_path, dest_path)
+    copy_method = shutil.copytree if source_path.is_dir() else shutil.copy
+    copy_method(source_path, dest_path)
 
 
 def test_static_images(test_case: _TestCaseTuple) -> None:
@@ -205,10 +221,15 @@ def test_static_images(test_case: _TestCaseTuple) -> None:
         else _get_file_paths(test_case.cached_image_path, ext=_DocModeInfo.doc_image_format)
     )
     current_cached_image_path = cached_image_paths[0]
+    docs_image_path = (
+        test_case.docs_image_path
+        if test_case.docs_image_path.is_file()
+        else _get_file_paths(test_case.docs_image_path, ext=_DocModeInfo.doc_image_format)[0]
+    )
 
     warn_msg, fail_msg = _test_compare_images(
         test_name=test_case.test_name,
-        test_image=test_case.docs_image_path,
+        test_image=docs_image_path,
         cached_image=current_cached_image_path,
         allowed_error=DEFAULT_ERROR_THRESHOLD,
         allowed_warning=DEFAULT_WARNING_THRESHOLD,
@@ -219,7 +240,7 @@ def test_static_images(test_case: _TestCaseTuple) -> None:
         # Compare build image to other known valid versions
         msg_start = "This test has multiple cached images. It initially failed (as above)"
         for path in cached_image_paths[1:]:
-            error = pv.compare_images(pv.read(test_case.docs_image_path), pv.read(path))
+            error = pv.compare_images(pv.read(docs_image_path), pv.read(path))
             if _check_compare_fail(test_case.test_name, error, allowed_error=DEFAULT_ERROR_THRESHOLD) is None:
                 # Convert failure into a warning
                 warn_msg = fail_msg + (f"\n{msg_start} but passed when compared to:\n\t{path}")
@@ -230,22 +251,28 @@ def test_static_images(test_case: _TestCaseTuple) -> None:
             fail_msg += f"\n{msg_start} and failed again for all images in:\n\t{_DocModeInfo.doc_image_cache_dir / test_case.test_name!s}"
 
     if fail_msg:
-        _save_failed_test_image(test_case.docs_image_path, "errors")
+        _save_failed_test_image(docs_image_path, "errors")
         # Save all cached images since they all failed
         for path in cached_image_paths:
             _save_failed_test_image(path, "errors")
         pytest.fail(fail_msg)
 
     if warn_msg:
         parent_dir: Literal["errors_as_warnings", "warnings"] = "errors_as_warnings" if test_case.cached_image_path.is_dir() else "warnings"
-        _save_failed_test_image(test_case.docs_image_path, parent_dir)
+        _save_failed_test_image(docs_image_path, parent_dir)
         _save_failed_test_image(current_cached_image_path, parent_dir)
         warnings.warn(warn_msg, stacklevel=2)
 
 
-def _test_both_images_exist(filename: str, docs_image_path: Path, cached_image_path: Path) -> tuple[str | None, Path | None]:
-    if docs_image_path is None or cached_image_path is None:
-        if docs_image_path is None:
+def _test_both_images_exist(filename: str, docs_image_path: Path | None, cached_image_path: Path | None) -> tuple[str | None, Path | None]:
+    def has_no_images(path: Path | None) -> bool:
+        return path is None or (path.is_dir() and len(_get_file_paths(path, ext=_DocModeInfo.doc_image_format)) == 0)
+
+    build_has_no_images = has_no_images(docs_image_path)
+    cache_has_no_images = has_no_images(cached_image_path)
+
+    if build_has_no_images or cache_has_no_images:
+        if build_has_no_images:
             source_path = cached_image_path
             exists = "cache"
             missing = "docs build"

pytest_pyvista/pytest_pyvista.py

@@ -166,11 +166,6 @@ def pytest_addoption(parser) -> None:  # noqa: ANN001
         action="store_true",
         help="Prevent test failure if a generated test image has no use.",
     )
-    group.addoption(
-        "--generate_subdirs",
-        action="store_true",
-        help="Save generated images to sub-directories. The image names are determined by the environment info.",
-    )
     group.addoption(
         "--add_missing_images",
         action="store_true",
@@ -244,6 +239,19 @@ def _add_common_pytest_options(parser, *, doc: bool = False) -> None:  # noqa: A
         default=None,
         help="Path to dump images from failed tests from the current run.",
     )
+    group.addoption(
+        f"--{prefix}generate_subdirs",
+        action="store_const",
+        const=True,
+        default=None,
+        help="Save generated images to sub-directories. The image names are determined by the environment info.",
+    )
+    parser.addini(
+        f"{prefix}generate_subdirs",
+        default=False,
+        type="bool",
+        help="Save generated images to sub-directories. The image names are determined by the environment info.",
+    )
     group.addoption(
         f"--{prefix}image_format",
         action="store",
@@ -320,7 +328,7 @@ class VerifyImageCache:
     allow_unused_generated = False
     add_missing_images = False
     reset_only_failed = False
-    generate_subdirs = None
+    generate_subdirs: bool = False
     image_format: _ImageFormats
 
     def __init__(  # noqa: PLR0913
@@ -455,7 +463,7 @@ def remove_plotter_close_callback() -> None:
             # Compare test image to other known valid versions
             msg_start = "This test has multiple cached images. It initially failed (as above)"
             for path in cached_image_paths[1:]:
-                error = _compare_images(plotter, str(path))
+                error = _compare_images(plotter, path)
                 if _check_compare_fail(test_name, error, allowed_error=allowed_error) is None:
                     # Convert failure into a warning
                     warn_msg = fail_msg + (f"\n{msg_start} but passed when compared to:\n\t{path}")
@@ -486,10 +494,9 @@ def remove_plotter_close_callback() -> None:
 
     def _save_generated_image(self, plotter: pyvista.Plotter, image_name: str, parent_dir: Path | None = None) -> None:
         parent = cast("Path", self.generated_image_dir) if parent_dir is None else parent_dir
-        generated_image_path = (
-            parent / Path(image_name).with_suffix("") / f"{self.env_info}.{self.image_format}" if self.generate_subdirs else parent / image_name
+        generated_image_path = _get_generated_image_path(
+            parent=parent, image_name=image_name, generate_subdirs=self.generate_subdirs, env_info=self.env_info
         )
-        generated_image_path.parent.mkdir(exist_ok=True, parents=True)
         plotter.screenshot(generated_image_path)
 
     def _save_failed_test_images(
@@ -552,6 +559,13 @@ def remove_suffix(s: str) -> str:
     return "test_" + remove_suffix(image_name)
 
 
+def _get_generated_image_path(parent: Path, image_name: Path | str, *, generate_subdirs: bool, env_info: str | _EnvInfo) -> Path:
+    name = Path(image_name)
+    generated_image_path = parent / name.with_suffix("") / f"{env_info}{name.suffix}" if generate_subdirs else parent / name
+    generated_image_path.parent.mkdir(exist_ok=True, parents=True)
+    return generated_image_path
+
+
 def _get_file_paths(dir_: Path, ext: str) -> list[Path]:
     """Get all paths of files with a specific extension inside a directory tree."""
     return sorted(dir_.rglob(f"*.{ext}"))
@@ -660,12 +674,12 @@ def _ensure_dir_exists(dirpath: str | Path, msg_name: str) -> None:
 
 
 @overload
-def _get_option_from_config_or_ini(pytestconfig: pytest.Config, option: str, *, is_dir: Literal[True] = True) -> Path | None: ...
+def _get_option_from_config_or_ini(pytestconfig: pytest.Config, option: str, *, is_dir: Literal[False] = False) -> str | bool | None: ...
 @overload
-def _get_option_from_config_or_ini(pytestconfig: pytest.Config, option: str, *, is_dir: Literal[False] = False) -> str | None: ...
+def _get_option_from_config_or_ini(pytestconfig: pytest.Config, option: str, *, is_dir: Literal[True] = True) -> Path | None: ...
 @overload
-def _get_option_from_config_or_ini(pytestconfig: pytest.Config, option: str, *, is_dir: bool) -> Path | str | None: ...
-def _get_option_from_config_or_ini(pytestconfig: pytest.Config, option: str, *, is_dir: bool = False) -> Path | str | None:
+def _get_option_from_config_or_ini(pytestconfig: pytest.Config, option: str, *, is_dir: bool) -> Path | str | bool | None: ...
+def _get_option_from_config_or_ini(pytestconfig: pytest.Config, option: str, *, is_dir: bool = False) -> Path | str | bool | None:
     value = pytestconfig.getoption(option)
     if value is None:
         value = pytestconfig.getini(option)
@@ -708,8 +722,7 @@ def pytest_configure(config: pytest.Config) -> None:
     if config.getoption("doc_mode"):
         from pytest_pyvista.doc_mode import _DocModeInfo  # noqa: PLC0415
 
-        _DocModeInfo.init_dirs(config)
-        _DocModeInfo.doc_image_format = cast("_ImageFormats", _get_option_from_config_or_ini(config, "doc_image_format"))
+        _DocModeInfo.init_from_config(config)
 
     # create a image names directory for individual or multiple workers to write to
     if config.getoption("disallow_unused_cache"):