Repository: ESSS/pytest-regressions (language: python)

Issue:
Tests from different test classes refer to same regression data file
When tests are grouped into test classes (see https://docs.pytest.org/en/stable/getting-started.html#group-multiple-tests-in-a-class), then tests with the same name, but in a different test class will refer to the same regression data file.

In order to avoid collision, the default naming of regression should include also the test class name if tests are grouped into classes.



Discussion / hints:
Thanks @kdebrab for the report!

Indeed we did not consider that when writing the plugin.
I sometimes find the existing behaviour useful, such as having two classes that test different things but which should have identical output.

I can see that this could confuse new users, though. I'd like it if the fix added a setting which reverts to the current behaviour.

Fix patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index cb09b78..47f9f29 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,3 +1,8 @@
+2.3.0 (UNRELEASED)
+------------------
+
+* `#54 <https://github.com/ESSS/pytest-regressions/pull/54>`__: New ``--with-test-class-names`` command-line flag to consider test class names when composing the expected and obtained data filenames. Needed when the same module contains different classes with the same method names.
+
 2.2.0 (2020-01-27)
 ------------------
 


Test patch:
diff --git a/src/pytest_regressions/common.py b/src/pytest_regressions/common.py
index 6faa48d..b335415 100644
--- a/src/pytest_regressions/common.py
+++ b/src/pytest_regressions/common.py
@@ -79,6 +79,7 @@ def perform_regression_check(
     basename=None,
     fullpath=None,
     force_regen=False,
+    with_test_class_names=False,
     obtained_filename=None,
     dump_aux_fn=lambda filename: [],
 ):
@@ -102,6 +103,8 @@ def perform_regression_check(
         3d views and plots to compare later). Must return the list of file names written (used to display).
     :param str extension: Extension of files compared by this check.
     :param bool force_regen: if true it will regenerate expected file.
+    :param bool with_test_class_names: if true it will use the test class name (if any) to compose
+        the basename.
     :param str obtained_filename: complete path to use to write the obtained file. By
         default will prepend `.obtained` before the file extension.
     ..see: `data_regression.Check` for `basename` and `fullpath` arguments.
@@ -112,8 +115,15 @@ def perform_regression_check(
 
     __tracebackhide__ = True
 
+    with_test_class_names = with_test_class_names or request.config.getoption(
+        "with_test_class_names"
+    )
     if basename is None:
-        basename = re.sub(r"[\W]", "_", request.node.name)
+        if (request.node.cls is not None) and (with_test_class_names):
+            basename = re.sub(r"[\W]", "_", request.node.cls.__name__) + "_"
+        else:
+            basename = ""
+        basename += re.sub(r"[\W]", "_", request.node.name)
 
     if fullpath:
         filename = source_filename = Path(fullpath)
diff --git a/src/pytest_regressions/data_regression.py b/src/pytest_regressions/data_regression.py
index a382a14..f3eac7b 100644
--- a/src/pytest_regressions/data_regression.py
+++ b/src/pytest_regressions/data_regression.py
@@ -20,6 +20,7 @@ def __init__(self, datadir, original_datadir, request):
         self.datadir = datadir
         self.original_datadir = original_datadir
         self.force_regen = False
+        self.with_test_class_names = False
 
     def check(self, data_dict, basename=None, fullpath=None):
         """
@@ -64,6 +65,7 @@ def dump(filename):
             basename=basename,
             fullpath=fullpath,
             force_regen=self.force_regen,
+            with_test_class_names=self.with_test_class_names,
         )
 
     # non-PEP 8 alias used internally at ESSS
diff --git a/src/pytest_regressions/dataframe_regression.py b/src/pytest_regressions/dataframe_regression.py
index f599cfd..e2b1907 100644
--- a/src/pytest_regressions/dataframe_regression.py
+++ b/src/pytest_regressions/dataframe_regression.py
@@ -23,6 +23,7 @@ def __init__(self, datadir, original_datadir, request):
         self.datadir = datadir
         self.original_datadir = original_datadir
         self._force_regen = False
+        self._with_test_class_names = False
 
         self._pandas_display_options = (
             "display.precision",
@@ -258,4 +259,5 @@ def check(
                 basename=basename,
                 fullpath=fullpath,
                 force_regen=self._force_regen,
+                with_test_class_names=self._with_test_class_names,
             )
diff --git a/src/pytest_regressions/file_regression.py b/src/pytest_regressions/file_regression.py
index 99a0384..d1d86a4 100644
--- a/src/pytest_regressions/file_regression.py
+++ b/src/pytest_regressions/file_regression.py
@@ -18,6 +18,7 @@ def __init__(self, datadir, original_datadir, request):
         self.datadir = datadir
         self.original_datadir = original_datadir
         self.force_regen = False
+        self.with_test_class_names = False
 
     def check(
         self,
@@ -98,6 +99,7 @@ def dump_fn(filename):
             basename=basename,
             fullpath=fullpath,
             force_regen=self.force_regen,
+            with_test_class_names=self.with_test_class_names,
             obtained_filename=obtained_filename,
         )
 
diff --git a/src/pytest_regressions/image_regression.py b/src/pytest_regressions/image_regression.py
index abde72f..a18098e 100644
--- a/src/pytest_regressions/image_regression.py
+++ b/src/pytest_regressions/image_regression.py
@@ -19,6 +19,7 @@ def __init__(self, datadir, original_datadir, request):
         self.datadir = datadir
         self.original_datadir = original_datadir
         self.force_regen = False
+        self.with_test_class_names = False
 
     def _load_image(self, filename):
         """
@@ -163,4 +164,5 @@ def dump_fn(target):
             extension=".png",
             basename=basename,
             force_regen=self.force_regen,
+            with_test_class_names=self.with_test_class_names,
         )
diff --git a/src/pytest_regressions/plugin.py b/src/pytest_regressions/plugin.py
index e00c939..640e39a 100644
--- a/src/pytest_regressions/plugin.py
+++ b/src/pytest_regressions/plugin.py
@@ -9,6 +9,12 @@ def pytest_addoption(parser):
         default=False,
         help="Re-generate all data_regression fixture data files.",
     )
+    group.addoption(
+        "--with-test-class-names",
+        action="store_true",
+        default=False,
+        help="Do not ignore the names of the test classes when composing the name of the regression data files.",
+    )
 
 
 @pytest.fixture
diff --git a/tests/test_filenames.py b/tests/test_filenames.py
new file mode 100644
index 0000000..4c2c6f8
--- /dev/null
+++ b/tests/test_filenames.py
@@ -0,0 +1,62 @@
+import pytest
+from pathlib import Path
+
+
+def test_foo(datadir, num_regression):
+    """
+    Dumb test, just to generate a expected csv file that must be different from the one gathered
+    from `TestClass::test_foo`.
+    """
+    data = {"ones": [1, 1, 1, 1], "zeros": [0, 0, 0, 0]}
+    num_regression.check(data)
+
+    expected_filename = f"{test_foo.__name__}.csv"
+    obtained_filename = f"{test_foo.__name__}.obtained.csv"
+    assert (datadir / expected_filename).exists()
+    assert (datadir / obtained_filename).exists()
+
+
+class TestClass:
+    def test_foo(self, datadir, num_regression):
+        """
+        Since 2.2.1, pytest-regressions use the test class name to compose the name of the datafiles, by default.
+        This tests asserts this behavior.
+        """
+        data = {"twos": [2, 2, 2, 2], "threes": [3, 3, 3, 3]}
+
+        # equivalent to use "--with-test-class-names" parameter
+        num_regression._with_test_class_names = True
+        num_regression.check(data)
+
+        expected_filename = f"{TestClass.__name__}_{TestClass.test_foo.__name__}.csv"
+        obtained_filename = (
+            f"{TestClass.__name__}_{TestClass.test_foo.__name__}.obtained.csv"
+        )
+        assert (datadir / expected_filename).exists()
+        assert (datadir / obtained_filename).exists()
+
+
+class TestClassWithIgnoredName:
+    def test_foo(self, datadir, num_regression):
+        """
+        Specifies to not use the class name to compose the expected data filename. The filename coincides with the
+        expected data filename used by `test_foo` function. The regression test then fails because the expected
+        data is different.
+        """
+        data = {"fours": [4, 4, 4, 4], "fives": [5, 5, 5, 5]}
+
+        # Trying to compare this data with `test_foo` expected data.
+        with pytest.raises(AssertionError):
+            num_regression.check(data)
+
+        # Assert that we have the expected names, without class name
+        expected_filename = f"{TestClassWithIgnoredName.test_foo.__name__}.csv"
+        obtained_filename = f"{TestClassWithIgnoredName.test_foo.__name__}.obtained.csv"
+        assert (datadir / expected_filename).exists()
+        assert (datadir / obtained_filename).exists()
+
+        # Assert that don't have files with class name
+        expected_with_class_name = f"{TestClassWithIgnoredName.__name__}_{TestClassWithIgnoredName.test_foo.__name__}.csv"
+        obtained_with_class_name = f"{TestClassWithIgnoredName.__name__}_{TestClassWithIgnoredName.test_foo.__name__}.obtained.csv"
+        assert not (datadir / expected_with_class_name).exists()
+        assert not (datadir / obtained_with_class_name).exists()
diff --git a/tests/test_filenames/TestClass_test_foo.csv b/tests/test_filenames/TestClass_test_foo.csv
new file mode 100644
index 0000000..e2f5862
--- /dev/null
+++ b/tests/test_filenames/TestClass_test_foo.csv
@@ -0,0 +1,5 @@
+,twos,threes
+0,2,3
+1,2,3
+2,2,3
+3,2,3
diff --git a/tests/test_filenames/test_foo.csv b/tests/test_filenames/test_foo.csv
new file mode 100644
index 0000000..276dd3a
--- /dev/null
+++ b/tests/test_filenames/test_foo.csv
@@ -0,0 +1,5 @@
+,ones,zeros
+0,1,0
+1,1,0
+2,1,0
+3,1,0