Repository: Apkawa/pytest-image-diff (language: python)

Issue:
Using NamedTemporaryFile doesn't behave as expected on Windows
* pytest-image-diff version: 0.0.7
* Python version: 3.9.7
* Operating System: Windows 10

### Description

When attempting to write to a `NamedTemporaryFile` on Windows, the file can't be open when given to splinter. If the file is open, an "Access Denied" exception is raised.

### What I Did

This can be fixed by making the temporary file persistent, allowing it to close and then removing it manually. I can create a PR to fix this, but my current solution is to replace line 55 in splinter.py with:

            with NamedTemporaryFile(suffix=".png", delete=False) as tf:
                temp_image_path = pathlib.Path(tf.name)

            try:
                browser.driver.save_screenshot(os.fspath(temp_image_path))
                result = image_regression(os.fspath(temp_image_path), threshold, suffix)
            finally:
                temp_image_path.unlink(missing_ok=True)

            return result

If this looks OK to you I'll create a proper PR.
feat: allow xpath for test regression against element + Windows support
Warning: This patch does two things, which I sadly didn't have time to split out into separate PRs.

1) Allow a user to give an optional `xpath` qualifier when calling `screenshot_regression`. The regression will then be tested against this single element instead of the whole page.
2) Make pytest-image-diff with splinter work properly on Windows (see issue #1). Since Windows requires us to close the temp file first, we make the temp file persistent, close it, and then tells splinter to write to it. Afterwards we clean up the temporary file before returning from the function.
Using NamedTemporaryFile doesn't behave as expected on Windows
* pytest-image-diff version: 0.0.7
* Python version: 3.9.7
* Operating System: Windows 10

### Description

When attempting to write to a `NamedTemporaryFile` on Windows, the file can't be open when given to splinter. If the file is open, an "Access Denied" exception is raised.

### What I Did

This can be fixed by making the temporary file persistent, allowing it to close and then removing it manually. I can create a PR to fix this, but my current solution is to replace line 55 in splinter.py with:

            with NamedTemporaryFile(suffix=".png", delete=False) as tf:
                temp_image_path = pathlib.Path(tf.name)

            try:
                browser.driver.save_screenshot(os.fspath(temp_image_path))
                result = image_regression(os.fspath(temp_image_path), threshold, suffix)
            finally:
                temp_image_path.unlink(missing_ok=True)

            return result

If this looks OK to you I'll create a proper PR.
feat: allow xpath for test regression against element + Windows support
Warning: This patch does two things, which I sadly didn't have time to split out into separate PRs.

1) Allow a user to give an optional `xpath` qualifier when calling `screenshot_regression`. The regression will then be tested against this single element instead of the whole page.
2) Make pytest-image-diff with splinter work properly on Windows (see issue #1). Since Windows requires us to close the temp file first, we make the temp file persistent, close it, and then tells splinter to write to it. Afterwards we clean up the temporary file before returning from the function.


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 97f083b..f2148d2 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -9,29 +9,47 @@ on:
 
 jobs:
   test:
-    runs-on: ubuntu-20.04
-    name: "python ${{ matrix.python-version }} ${{ matrix.toxenv }}"
     strategy:
       fail-fast: false
       matrix:
+        os: [ubuntu-20.04]
         python-version: [ 3.7, 3.8, 3.9, "3.10"]
         toxenv: [""]
         experimental: [ false ]
+        chrome: ["stable"]
         include:
           - toxenv: qa
             python-version: 3.7
             experimental: false
+            os: ubuntu-20.04
           - toxenv: type
             python-version: 3.7
             experimental: false
-          - toxenv: splinter
+            os: ubuntu-20.04
+          - toxenv: splinter-linux
             experimental: false
             python-version: 3.7
+            os: ubuntu-20.04
+
+          - toxenv: splinter-windows
+            # Do not support selenium-windows
+            experimental: true
+            python-version: 3.7
+            os: windows-latest
+            chrome: "959905"
+
+          - experimental: false
+            python-version: "3.9"
+            os: windows-latest
           - experimental: true
             python-version: "3.11.0-alpha - 3.11"
+            os: ubuntu-20.04
           - experimental: true
             python-version: "pypy-3.7"
+            os: ubuntu-20.04
 
+    runs-on: ${{ matrix.os }}
+    name: "${{ matrix.os }} python ${{ matrix.python-version }} ${{ matrix.toxenv }}"
 
     continue-on-error: ${{ matrix.experimental }}
     env:
@@ -39,8 +57,10 @@ jobs:
     steps:
       # chrome headless
       - uses: browser-actions/setup-chrome@latest
+        with:
+          chrome-version: ${{ matrix.chrome }}
       - uses: actions/checkout@v2
-      - name: Set up python ${{ matrix.python-version}}
+      - name: Set up python ${{ matrix.python-version }}
         uses: actions/setup-python@v2
         with:
           python-version: ${{ matrix.python-version }}
diff --git a/setup.cfg b/setup.cfg
index 150dbf0..c74d206 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -20,7 +20,7 @@ universal = 1
 
 [flake8]
 ignore = D203
-exclude = 
+exclude =
 	.git/,
 	.tox/,
 	docs/,
diff --git a/setup.py b/setup.py
index 76c7162..fec3000 100644
--- a/setup.py
+++ b/setup.py
@@ -54,9 +54,7 @@ def read(fname):
     python_requires=">=3.6, <4",
     entry_points={"pytest11": ["image_diff = pytest_image_diff.plugin"]},
     install_requires=["pytest", "typing_extensions", "diffimg", "imgdiff"],
-    extras_require={
-        "splinter": ["pytest-splinter>=2.1.0", "chromedriver-binary==2.40.1"]
-    },
+    extras_require={"splinter": ["pytest-splinter>=2.1.0"]},
     zip_safe=False,
     include_package_data=True,
     keywords=["pytest"],
diff --git a/tox.ini b/tox.ini
index 0fb03b7..62c67a7 100644
--- a/tox.ini
+++ b/tox.ini
@@ -2,13 +2,21 @@
 minversion = 2.3
 skip_missing_interpreters = true
 envlist =
-    py3{6,7,8,9,10,11}
+    py3{6,7,8,9,10,11}-{linux,windows}
 
 [testenv]
+# environment will be skipped if regular expression does not match against the sys.platform string
+platform = linux: linux
+           macos: darwin
+           windows: win32
+
 changedir = {toxinidir}
 deps =
     -r{toxinidir}/requirements-dev.txt
     splinter: -e .[splinter]
+    splinter-linux: chromedriver-binary-auto
+    splinter-windows: chromedriver-binary==95.0.4638.69.0
+
 setenv =
     PYTHONPATH = {toxinidir}
 passenv =


Test patch:
diff --git a/pytest_image_diff/_types.py b/pytest_image_diff/_types.py
index a66105c..a5621c5 100644
--- a/pytest_image_diff/_types.py
+++ b/pytest_image_diff/_types.py
@@ -1,9 +1,10 @@
+from pathlib import Path
 from typing import BinaryIO, Union, Tuple, Optional
 
 from PIL.Image import Image
 from typing_extensions import Literal, Protocol
 
-PathOrFileType = Union[str, bytes, BinaryIO]
+PathOrFileType = Union[str, bytes, Path, BinaryIO]
 ImageFileType = Union[Image, PathOrFileType]
 ImageSize = Tuple[int, int]
 
diff --git a/pytest_image_diff/helpers.py b/pytest_image_diff/helpers.py
index c51f98f..432fd7c 100644
--- a/pytest_image_diff/helpers.py
+++ b/pytest_image_diff/helpers.py
@@ -1,6 +1,9 @@
 import os
+import pathlib
 import shutil
 import typing
+from contextlib import contextmanager
+from tempfile import NamedTemporaryFile
 
 from PIL.Image import Image
 from _pytest import junitxml
@@ -27,6 +30,19 @@ def build_filename(
     )
 
 
+@contextmanager
+def temp_file(suffix: typing.Optional[str] = None) -> typing.Iterator[pathlib.Path]:
+    with NamedTemporaryFile(suffix=suffix, delete=False) as tf:
+        temp_image_path = pathlib.Path(tf.name)
+    try:
+        yield temp_image_path
+    finally:
+        try:
+            temp_image_path.unlink()
+        except FileNotFoundError:
+            pass
+
+
 class TestInfo:
     test_name: str
     class_name: str
@@ -58,13 +74,13 @@ def get_test_info(
     return TestInfo.get_test_info(request, suffix, prefix)
 
 
-def image_save(image: ImageFileType, path: str) -> None:
+def image_save(image: ImageFileType, path: typing.Union[str, pathlib.Path]) -> None:
     if isinstance(image, Image):
         image.save(path)
     elif isinstance(image, str):
         if not os.path.exists(image):
             raise ValueError("Image maybe path. Path not exists!")
-        shutil.copyfile(image, path)
+        shutil.copyfile(image, str(path))
     elif hasattr(image, "read"):
         with open(path, "wb") as f:
             image = typing.cast(typing.BinaryIO, image)
diff --git a/pytest_image_diff/plugin.py b/pytest_image_diff/plugin.py
index fef88d6..6b2d5b3 100644
--- a/pytest_image_diff/plugin.py
+++ b/pytest_image_diff/plugin.py
@@ -1,5 +1,4 @@
 import os
-from tempfile import NamedTemporaryFile
 from typing import Optional, NamedTuple, cast, Callable, Generator
 
 import pytest
@@ -8,7 +7,7 @@
 from _pytest.runner import CallInfo
 
 from ._types import ImageFileType, ImageRegressionCallableType, ImageDiffCallableType
-from .helpers import get_test_info, build_filename, image_save, ensure_dirs
+from .helpers import get_test_info, build_filename, image_save, ensure_dirs, temp_file
 from .image_diff import _diff
 
 try:
@@ -18,7 +17,7 @@
 
 
 @pytest.hookimpl(tryfirst=True, hookwrapper=True)  # type: ignore
-def pytest_runtest_makereport(item: Function, call: CallInfo) -> None:
+def pytest_runtest_makereport(item: Function, call: CallInfo):  # type: ignore
     # execute all other hooks to obtain the report object
     outcome = yield
     rep = outcome.get_result()
@@ -180,8 +179,6 @@ def _factory(
     ) -> bool:
         if threshold is None:
             threshold = image_diff_threshold
-        image_temp_file = NamedTemporaryFile(suffix=".jpg")
-        image_2_temp_file = NamedTemporaryFile(suffix=".jpg")
 
         _info = _image_diff_info(image, suffix)
         diff_path = cast(str, _info.diff_name)
@@ -198,9 +195,12 @@ def _cleanup() -> None:
 
         request.addfinalizer(_cleanup)
 
-        image_save(image, path=image_temp_file.name)
-        image_save(image_2, path=image_2_temp_file.name)
-        diff_ratio = _diff(image_temp_file.name, image_2_temp_file.name, diff_path)
+        with temp_file(suffix=".jpg") as image_temp_file, temp_file(
+            suffix=".jpg"
+        ) as image_2_temp_file:
+            image_save(image, path=image_temp_file)
+            image_save(image_2, path=image_2_temp_file)
+            diff_ratio = _diff(image_temp_file, image_2_temp_file, diff_path)
         assert diff_ratio <= threshold, "Image not equals! See %s" % diff_path  # noqa
         return True
 
diff --git a/pytest_image_diff/splinter.py b/pytest_image_diff/splinter.py
index 22b5abb..86678da 100644
--- a/pytest_image_diff/splinter.py
+++ b/pytest_image_diff/splinter.py
@@ -1,8 +1,10 @@
-import pytest
-from tempfile import NamedTemporaryFile
+import os
 from typing import Optional, Generator
+
+import pytest
 from typing_extensions import Protocol
 
+from .helpers import temp_file
 
 try:
     # Check pytest-splinter
@@ -19,6 +21,7 @@ def __call__(
             browser: Optional[Browser] = None,
             threshold: Optional[float] = None,
             suffix: Optional[str] = None,
+            xpath: Optional[str] = "",
         ) -> bool:
             pass
 
@@ -36,6 +39,8 @@ def screenshot_regression(
         :param browser: optional, by default from `browser` fixture
         :param threshold: float, by default from `image_diff_threshold`
         :param suffix: str, need for multiple checks  by one test
+        :param xpath: str, optional xpath expression to select an element to
+                screenshot instead of page
         """
         default_browser = browser
 
@@ -43,15 +48,34 @@ def _factory(
             browser: Optional[Browser] = None,
             threshold: Optional[float] = None,
             suffix: Optional[str] = "",
+            xpath: Optional[str] = "",
         ) -> bool:
             if browser is None:
                 browser = default_browser
 
             if threshold is None:
                 threshold = image_diff_threshold
-            tf = NamedTemporaryFile(suffix=".png")
-            image = tf.name
-            browser.driver.save_screenshot(image)
-            return image_regression(image, threshold, suffix)
+
+            with temp_file(suffix=".png") as temp_image_path:
+                screenshot_path = os.fspath(temp_image_path)
+
+                if xpath:
+                    # `unique_file=False` since we already have a temporary file
+                    #
+                    # Since an xpath screenshot composes its own file name,
+                    # we need to give it the prefix and
+                    # suffix as separate parameters. `:-4` for the path without extension,
+                    # then suffix given manually.
+                    browser.find_by_xpath(xpath).first.screenshot(
+                        screenshot_path[:-4],
+                        suffix=screenshot_path[-4:],
+                        unique_file=False,
+                        full=True,
+                    )
+                else:
+                    browser.driver.save_screenshot(screenshot_path)
+
+                result = image_regression(screenshot_path, threshold, suffix)
+                return result
 
         yield _factory
diff --git a/tests/test_splinter.py b/tests/test_splinter.py
index 3d70a8b..4759861 100644
--- a/tests/test_splinter.py
+++ b/tests/test_splinter.py
@@ -50,3 +50,14 @@ def test_splinter(browser, screenshot_regression):
     screenshot_regression()
     browser.driver.set_window_size(800, 600)
     screenshot_regression(suffix="small_window")
+
+
+@with_splinter
+def test_splinter_with_xpath(browser, screenshot_regression):
+    tf = tempfile.NamedTemporaryFile(suffix=".html")
+    tf.write(b"<html><body><h1>Example</h1></body></html>")
+    tf.flush()
+
+    browser.driver.set_window_size(1280, 1024)
+    browser.visit("file://" + tf.name)
+    screenshot_regression(xpath="//h1")