Merge pull request #2167 from fogo/parametrize-ids-silent-failure

RonnyPfannschmidt · web-flow · commit 8f1450114f2b · 2017-01-03T09:17:54.000+01:00
No longer silently ignore errors in parametrize callable ids
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -8,6 +8,8 @@ New Features
 * Added an ini option ``doctest_encoding`` to specify which encoding to use for doctest files.
   Thanks `@wheerd`_ for the PR (`#2101`_).
 
+* pytest now warns when a callable ids raises in a parametrized test. Thanks `@fogo`_ for the PR.
+
 *
 
 
@@ -39,6 +41,7 @@ Changes
 .. _@fushi: https://github.com/fushi
 .. _@mattduck: https://github.com/mattduck
 .. _@wheerd: https://github.com/wheerd
+.. _@fogo: https://github.com/fogo
 
 .. _#1512: https://github.com/pytest-dev/pytest/issues/1512
 .. _#1874: https://github.com/pytest-dev/pytest/pull/1874
diff --git a/_pytest/python.py b/_pytest/python.py
@@ -928,12 +928,17 @@ def _find_parametrized_scope(argnames, arg2fixturedefs, indirect):
 
 def _idval(val, argname, idx, idfn, config=None):
     if idfn:
+        s = None
         try:
             s = idfn(val)
-            if s:
-                return _escape_strings(s)
         except Exception:
-            pass
+            # See issue https://github.com/pytest-dev/pytest/issues/2169
+            import warnings
+            msg = "Raised while trying to determine id of parameter %s at position %d." % (argname, idx)
+            msg += '\nUpdate your code as this will raise an error in pytest-4.0.'
+            warnings.warn(msg)
+        if s:
+            return _escape_strings(s)
 
     if config:
         hook_id = config.hook.pytest_make_parametrize_id(config=config, val=val)
diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py
@@ -296,18 +296,58 @@ def ids(val):
     @pytest.mark.issue351
     def test_idmaker_idfn_exception(self):
         from _pytest.python import idmaker
+        from _pytest.recwarn import WarningsRecorder
+
+        class BadIdsException(Exception):
+            pass
 
         def ids(val):
-            raise Exception("bad code")
+            raise BadIdsException("ids raised")
+
+        rec = WarningsRecorder()
+        with rec:
+            idmaker(("a", "b"), [(10.0, IndexError()),
+                                          (20, KeyError()),
+                                          ("three", [1, 2, 3]),
+            ], idfn=ids)
+
+        assert [str(i.message) for i in rec.list] == [
+            "Raised while trying to determine id of parameter a at position 0."
+            "\nUpdate your code as this will raise an error in pytest-4.0.",
+            "Raised while trying to determine id of parameter b at position 0."
+            "\nUpdate your code as this will raise an error in pytest-4.0.",
+            "Raised while trying to determine id of parameter a at position 1."
+            "\nUpdate your code as this will raise an error in pytest-4.0.",
+            "Raised while trying to determine id of parameter b at position 1."
+            "\nUpdate your code as this will raise an error in pytest-4.0.",
+            "Raised while trying to determine id of parameter a at position 2."
+            "\nUpdate your code as this will raise an error in pytest-4.0.",
+            "Raised while trying to determine id of parameter b at position 2."
+            "\nUpdate your code as this will raise an error in pytest-4.0.",
+        ]
 
-        result = idmaker(("a", "b"), [(10.0, IndexError()),
-                                      (20, KeyError()),
-                                      ("three", [1, 2, 3]),
-        ], idfn=ids)
-        assert result == ["10.0-b0",
-                          "20-b1",
-                          "three-b2",
-                         ]
+
+    def test_parametrize_ids_exception(self, testdir):
+        """
+        :param testdir: the instance of Testdir class, a temporary
+        test directory.
+        """
+        testdir.makepyfile("""
+                import pytest
+
+                def ids(arg):
+                    raise Exception("bad ids")
+
+                @pytest.mark.parametrize("arg", ["a", "b"], ids=ids)
+                def test_foo(arg):
+                    pass
+            """)
+        result = testdir.runpytest("--collect-only")
+        result.stdout.fnmatch_lines([
+            "<Module 'test_parametrize_ids_exception.py'>",
+            "  <Function 'test_foo[a]'>",
+            "  <Function 'test_foo[b]'>",
+        ])
 
     def test_idmaker_with_ids(self):
         from _pytest.python import idmaker
