Use fixtures to invoke xunit-style fixtures

changelog/3094.feature.rst

@@ -0,0 +1,5 @@
+`Class xunit-style <https://docs.pytest.org/en/latest/xunit_setup.html>`__ functions and methods
+now obey the scope of *autouse* fixtures.
+
+This fixes a number of surprising issues like ``setup_method`` being called before session-scoped
+autouse fixtures (see `#517 <https://github.com/pytest-dev/pytest/issues/517>`__ for an example).

doc/en/xunit_setup.rst

@@ -93,7 +93,15 @@ Remarks:
 
 * It is possible for setup/teardown pairs to be invoked multiple times
   per testing process.
+
 * teardown functions are not called if the corresponding setup function existed
   and failed/was skipped.
 
+* Prior to pytest-4.2, xunit-style functions did not obey the scope rules of fixtures, so
+  it was possible, for example, for a ``setup_method`` to be called before a
+  session-scoped autouse fixture.
+
+  Now the xunit-style functions are integrated with the fixture mechanism and obey the proper
+  scope rules of fixtures involved in the call.
+
 .. _`unittest.py module`: http://docs.python.org/library/unittest.html

src/_pytest/python.py

@@ -9,6 +9,7 @@
 import os
 import sys
 import warnings
+from functools import partial
 from textwrap import dedent
 
 import py
@@ -435,9 +436,66 @@ def _getobj(self):
         return self._importtestmodule()
 
     def collect(self):
+        self._inject_setup_module_fixture()
+        self._inject_setup_function_fixture()
         self.session._fixturemanager.parsefactories(self)
         return super(Module, self).collect()
 
+    def _inject_setup_module_fixture(self):
+        """Injects a hidden autouse, module scoped fixture into the collected module object
+        that invokes setUpModule/tearDownModule if either or both are available.
+
+        Using a fixture to invoke this methods ensures we play nicely and unsurprisingly with
+        other fixtures (#517).
+        """
+        setup_module = _get_non_fixture_func(self.obj, "setUpModule")
+        if setup_module is None:
+            setup_module = _get_non_fixture_func(self.obj, "setup_module")
+
+        teardown_module = _get_non_fixture_func(self.obj, "tearDownModule")
+        if teardown_module is None:
+            teardown_module = _get_non_fixture_func(self.obj, "teardown_module")
+
+        if setup_module is None and teardown_module is None:
+            return
+
+        @fixtures.fixture(autouse=True, scope="module")
+        def xunit_setup_module_fixture(request):
+            if setup_module is not None:
+                _call_with_optional_argument(setup_module, request.module)
+            yield
+            if teardown_module is not None:
+                _call_with_optional_argument(teardown_module, request.module)
+
+        self.obj.__pytest_setup_module = xunit_setup_module_fixture
+
+    def _inject_setup_function_fixture(self):
+        """Injects a hidden autouse, function scoped fixture into the collected module object
+        that invokes setup_function/teardown_function if either or both are available.
+
+        Using a fixture to invoke this methods ensures we play nicely and unsurprisingly with
+        other fixtures (#517).
+        """
+        setup_function = _get_non_fixture_func(self.obj, "setup_function")
+        teardown_function = _get_non_fixture_func(self.obj, "teardown_function")
+        if setup_function is None and teardown_function is None:
+            return
+
+        @fixtures.fixture(autouse=True, scope="function")
+        def xunit_setup_function_fixture(request):
+            if request.instance is not None:
+                # in this case we are bound to an instance, so we need to let
+                # setup_method handle this
+                yield
+                return
+            if setup_function is not None:
+                _call_with_optional_argument(setup_function, request.function)
+            yield
+            if teardown_function is not None:
+                _call_with_optional_argument(teardown_function, request.function)
+
+        self.obj.__pytest_setup_function = xunit_setup_function_fixture
+
     def _importtestmodule(self):
         # we assume we are only called once per module
         importmode = self.config.getoption("--import-mode")
@@ -488,19 +546,6 @@ def _importtestmodule(self):
         self.config.pluginmanager.consider_module(mod)
         return mod
 
-    def setup(self):
-        setup_module = _get_xunit_setup_teardown(self.obj, "setUpModule")
-        if setup_module is None:
-            setup_module = _get_xunit_setup_teardown(self.obj, "setup_module")
-        if setup_module is not None:
-            setup_module()
-
-        teardown_module = _get_xunit_setup_teardown(self.obj, "tearDownModule")
-        if teardown_module is None:
-            teardown_module = _get_xunit_setup_teardown(self.obj, "teardown_module")
-        if teardown_module is not None:
-            self.addfinalizer(teardown_module)
-
 
 class Package(Module):
     def __init__(self, fspath, parent=None, config=None, session=None, nodeid=None):
@@ -513,6 +558,22 @@ def __init__(self, fspath, parent=None, config=None, session=None, nodeid=None):
         self._norecursepatterns = session._norecursepatterns
         self.fspath = fspath
 
+    def setup(self):
+        # not using fixtures to call setup_module here because autouse fixtures
+        # from packages are not called automatically (#4085)
+        setup_module = _get_non_fixture_func(self.obj, "setUpModule")
+        if setup_module is None:
+            setup_module = _get_non_fixture_func(self.obj, "setup_module")
+        if setup_module is not None:
+            _call_with_optional_argument(setup_module, self.obj)
+
+        teardown_module = _get_non_fixture_func(self.obj, "tearDownModule")
+        if teardown_module is None:
+            teardown_module = _get_non_fixture_func(self.obj, "teardown_module")
+        if teardown_module is not None:
+            func = partial(_call_with_optional_argument, teardown_module, self.obj)
+            self.addfinalizer(func)
+
     def _recurse(self, dirpath):
         if dirpath.basename == "__pycache__":
             return False
@@ -599,8 +660,9 @@ def _get_xunit_setup_teardown(holder, attr_name, param_obj=None):
     when the callable is called without arguments, defaults to the ``holder`` object.
     Return ``None`` if a suitable callable is not found.
     """
+    # TODO: only needed because of Package!
     param_obj = param_obj if param_obj is not None else holder
-    result = _get_xunit_func(holder, attr_name)
+    result = _get_non_fixture_func(holder, attr_name)
     if result is not None:
         arg_count = result.__code__.co_argcount
         if inspect.ismethod(result):
@@ -611,7 +673,19 @@ def _get_xunit_setup_teardown(holder, attr_name, param_obj=None):
             return result
 
 
-def _get_xunit_func(obj, name):
+def _call_with_optional_argument(func, arg):
+    """Call the given function with the given argument if func accepts one argument, otherwise
+    calls func without arguments"""
+    arg_count = func.__code__.co_argcount
+    if inspect.ismethod(func):
+        arg_count -= 1
+    if arg_count:
+        func(arg)
+    else:
+        func()
+
+
+def _get_non_fixture_func(obj, name):
     """Return the attribute from the given object to be used as a setup/teardown
     xunit-style function, but only if not marked as a fixture to
     avoid calling it twice.
@@ -643,18 +717,60 @@ def collect(self):
                 )
             )
             return []
+
+        self._inject_setup_class_fixture()
+        self._inject_setup_method_fixture()
+
         return [Instance(name="()", parent=self)]
 
-    def setup(self):
-        setup_class = _get_xunit_func(self.obj, "setup_class")
-        if setup_class is not None:
-            setup_class = getimfunc(setup_class)
-            setup_class(self.obj)
+    def _inject_setup_class_fixture(self):
+        """Injects a hidden autouse, class scoped fixture into the collected class object
+        that invokes setup_class/teardown_class if either or both are available.
 
-        fin_class = getattr(self.obj, "teardown_class", None)
-        if fin_class is not None:
-            fin_class = getimfunc(fin_class)
-            self.addfinalizer(lambda: fin_class(self.obj))
+        Using a fixture to invoke this methods ensures we play nicely and unsurprisingly with
+        other fixtures (#517).
+        """
+        setup_class = _get_non_fixture_func(self.obj, "setup_class")
+        teardown_class = getattr(self.obj, "teardown_class", None)
+        if setup_class is None and teardown_class is None:
+            return
+
+        @fixtures.fixture(autouse=True, scope="class")
+        def xunit_setup_class_fixture(cls):
+            if setup_class is not None:
+                func = getimfunc(setup_class)
+                _call_with_optional_argument(func, self.obj)
+            yield
+            if teardown_class is not None:
+                func = getimfunc(teardown_class)
+                _call_with_optional_argument(func, self.obj)
+
+        self.obj.__pytest_setup_class = xunit_setup_class_fixture
+
+    def _inject_setup_method_fixture(self):
+        """Injects a hidden autouse, function scoped fixture into the collected class object
+        that invokes setup_method/teardown_method if either or both are available.
+
+        Using a fixture to invoke this methods ensures we play nicely and unsurprisingly with
+        other fixtures (#517).
+        """
+        setup_method = _get_non_fixture_func(self.obj, "setup_method")
+        teardown_method = getattr(self.obj, "teardown_method", None)
+        if setup_method is None and teardown_method is None:
+            return
+
+        @fixtures.fixture(autouse=True, scope="function")
+        def xunit_setup_method_fixture(self, request):
+            method = request.function
+            if setup_method is not None:
+                func = getattr(self, "setup_method")
+                _call_with_optional_argument(func, method)
+            yield
+            if teardown_method is not None:
+                func = getattr(self, "teardown_method")
+                _call_with_optional_argument(func, method)
+
+        self.obj.__pytest_setup_method = xunit_setup_method_fixture
 
 
 class Instance(PyCollector):
@@ -681,29 +797,9 @@ class FunctionMixin(PyobjMixin):
 
     def setup(self):
         """ perform setup for this test function. """
-        if hasattr(self, "_preservedparent"):
-            obj = self._preservedparent
-        elif isinstance(self.parent, Instance):
-            obj = self.parent.newinstance()
+        if isinstance(self.parent, Instance):
+            self.parent.newinstance()
             self.obj = self._getobj()
-        else:
-            obj = self.parent.obj
-        if inspect.ismethod(self.obj):
-            setup_name = "setup_method"
-            teardown_name = "teardown_method"
-        else:
-            setup_name = "setup_function"
-            teardown_name = "teardown_function"
-        setup_func_or_method = _get_xunit_setup_teardown(
-            obj, setup_name, param_obj=self.obj
-        )
-        if setup_func_or_method is not None:
-            setup_func_or_method()
-        teardown_func_or_method = _get_xunit_setup_teardown(
-            obj, teardown_name, param_obj=self.obj
-        )
-        if teardown_func_or_method is not None:
-            self.addfinalizer(teardown_func_or_method)
 
     def _prunetraceback(self, excinfo):
         if hasattr(self, "_obj") and not self.config.option.fulltrace:

src/_pytest/unittest.py

@@ -7,6 +7,7 @@
 import traceback
 
 import _pytest._code
+import pytest
 from _pytest.compat import getimfunc
 from _pytest.config import hookimpl
 from _pytest.outcomes import fail
@@ -32,24 +33,18 @@ class UnitTestCase(Class):
     # to declare that our children do not support funcargs
     nofuncargs = True
 
-    def setup(self):
-        cls = self.obj
-        if getattr(cls, "__unittest_skip__", False):
-            return  # skipped
-        setup = getattr(cls, "setUpClass", None)
-        if setup is not None:
-            setup()
-        teardown = getattr(cls, "tearDownClass", None)
-        if teardown is not None:
-            self.addfinalizer(teardown)
-        super(UnitTestCase, self).setup()
-
     def collect(self):
         from unittest import TestLoader
 
         cls = self.obj
         if not getattr(cls, "__test__", True):
             return
+
+        skipped = getattr(cls, "__unittest_skip__", False)
+        if not skipped:
+            self._inject_setup_teardown_fixtures(cls)
+            self._inject_setup_class_fixture()
+
         self.session._fixturemanager.parsefactories(self, unittest=True)
         loader = TestLoader()
         foundsomething = False
@@ -68,6 +63,44 @@ def collect(self):
                 if ut is None or runtest != ut.TestCase.runTest:
                     yield TestCaseFunction("runTest", parent=self)
 
+    def _inject_setup_teardown_fixtures(self, cls):
+        """Injects a hidden auto-use fixture to invoke setUpClass/setup_method and corresponding
+        teardown functions (#517)"""
+        class_fixture = _make_xunit_fixture(
+            cls, "setUpClass", "tearDownClass", scope="class", pass_self=False
+        )
+        if class_fixture:
+            cls.__pytest_class_setup = class_fixture
+
+        method_fixture = _make_xunit_fixture(
+            cls, "setup_method", "teardown_method", scope="function", pass_self=True
+        )
+        if method_fixture:
+            cls.__pytest_method_setup = method_fixture
+
+
+def _make_xunit_fixture(obj, setup_name, teardown_name, scope, pass_self):
+    setup = getattr(obj, setup_name, None)
+    teardown = getattr(obj, teardown_name, None)
+    if setup is None and teardown is None:
+        return None
+
+    @pytest.fixture(scope=scope, autouse=True)
+    def fixture(self, request):
+        if setup is not None:
+            if pass_self:
+                setup(self, request.function)
+            else:
+                setup()
+        yield
+        if teardown is not None:
+            if pass_self:
+                teardown(self, request.function)
+            else:
+                teardown()
+
+    return fixture
+
 
 class TestCaseFunction(Function):
     nofuncargs = True
@@ -77,9 +110,6 @@ class TestCaseFunction(Function):
     def setup(self):
         self._testcase = self.parent.obj(self.name)
         self._fix_unittest_skip_decorator()
-        self._obj = getattr(self._testcase, self.name)
-        if hasattr(self._testcase, "setup_method"):
-            self._testcase.setup_method(self._obj)
         if hasattr(self, "_request"):
             self._request._fillfixtures()
 
@@ -97,11 +127,7 @@ def _fix_unittest_skip_decorator(self):
             setattr(self._testcase, "__name__", self.name)
 
     def teardown(self):
-        if hasattr(self._testcase, "teardown_method"):
-            self._testcase.teardown_method(self._obj)
-        # Allow garbage collection on TestCase instance attributes.
         self._testcase = None
-        self._obj = None
 
     def startTest(self, testcase):
         pass

testing/python/collect.py

@@ -240,9 +240,6 @@ def prop(self):
         assert result.ret == EXIT_NOTESTSCOLLECTED
 
 
-@pytest.mark.filterwarnings(
-    "ignore:usage of Generator.Function is deprecated, please use pytest.Function instead"
-)
 class TestFunction(object):
     def test_getmodulecollector(self, testdir):
         item = testdir.getitem("def test_func(): pass")