Repository: nose-devs/nose2

Issue:
such and normal test cases: mixed up call order of it.has_setup and setUpClass
As it is a little bit complicated to explain, I have created a working example as a gist: https://gist.github.com/jrast/109f70f9b4c52bab4252

I have several "normal" UnitTestCases and a few such tests. The problem is that the setUpClass calls are mixed up with the setup calls in such tests, as can be seen in the log file. The documentation states that I can not use Layer and setUpClass functions in the same TestCase, but it does not state that I cant use them side by side wich is the case in my example. So the question is: is this a bug or is there a missing hint in the documentation.

Either way I think this is a dangerous behaviour because test fixtures made in a setup call can be modified from outside of the current test case wich can lead to very unpredictable behaviour and wrong test results. One example of wrong behaviour is shown with the UniqueResource class wich is locked in the setup functions and unlocked in the teardown functions. As soon as the calls get mixed up a exception is thrown because the resource is already locked.

I realy hope this is not just a missing hint/warning in the docs, otherwise I have to rethink about how to structure my tests...

Discussion:
little-dude: It's still failing when using the `uses` decorator. Added a test for this.

Patch:
diff --git a/nose2/suite.py b/nose2/suite.py
index b52e0cb..f107489 100644
--- a/nose2/suite.py
+++ b/nose2/suite.py
@@ -22,6 +22,7 @@ class LayerSuite(unittest.BaseTestSuite):
         self.wasSetup = False
 
     def run(self, result):
+        self.handle_previous_test_teardown(result)
         if not self._safeMethodCall(self.setUp, result):
             return
         try:
@@ -37,6 +38,21 @@ class LayerSuite(unittest.BaseTestSuite):
             if self.wasSetup:
                 self._safeMethodCall(self.tearDown, result)
 
+    def handle_previous_test_teardown(self, result):
+        try:
+            prev = result._previousTestClass
+        except AttributeError:
+            return
+        layer_attr = getattr(prev, 'layer', None)
+        if isinstance(layer_attr, LayerSuite):
+            return
+        try:
+            suite_obj = unittest.suite.TestSuite()
+            suite_obj._tearDownPreviousClass(None, result)
+            suite_obj._handleModuleTearDown(result)
+        finally:
+            delattr(result, '_previousTestClass')
+
     def setUp(self):
         if self.layer is None:
             return


Test patch:
diff --git a/nose2/tests/functional/support/scenario/layers_and_non_layers/__init__.py b/nose2/tests/functional/support/scenario/layers_and_non_layers/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/nose2/tests/functional/support/scenario/layers_and_non_layers/common.py b/nose2/tests/functional/support/scenario/layers_and_non_layers/common.py
new file mode 100644
index 0000000..ae24633
--- /dev/null
+++ b/nose2/tests/functional/support/scenario/layers_and_non_layers/common.py
@@ -0,0 +1,60 @@
+import unittest
+import logging
+log = logging.getLogger(__name__)
+
+
+class UniqueResource(object):
+    _instance = None
+    used = False
+
+    def __new__(cls, *args, **kwargs):
+        if not cls._instance:
+            cls._instance = super(UniqueResource, cls).__new__(
+                cls, *args, **kwargs)
+        return cls._instance
+
+    def lock(self):
+        if not self.used:
+            self.used = True
+        else:
+            raise Exception("Resource allready used")
+
+    def unlock(self):
+        if self.used:
+            self.used = False
+        else:
+            raise Exception("Resource already unlocked")
+
+
+class NormalTest(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        log.info("Called setUpClass in NormalTest")
+        cls.unique_resource = UniqueResource()
+        cls.unique_resource.lock()
+
+    @classmethod
+    def tearDownClass(cls):
+        log.info("Called tearDownClass in NormalTest")
+        cls.unique_resource.unlock()
+
+    def test(self):
+        self.assertTrue(self.unique_resource.used)
+
+
+class NormalTestTwo(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        log.info("Called setUpClass in NormalTestTwo")
+        cls.unique_resource = UniqueResource()
+        cls.unique_resource.lock()
+
+    @classmethod
+    def tearDownClass(cls):
+        log.info("Called tearDownClass in NormalTestTwo")
+        cls.unique_resource.unlock()
+
+    def test(self):
+        self.assertTrue(self.unique_resource.used)
diff --git a/nose2/tests/functional/support/scenario/layers_and_non_layers/test_layers.py b/nose2/tests/functional/support/scenario/layers_and_non_layers/test_layers.py
new file mode 100644
index 0000000..4fb9c8c
--- /dev/null
+++ b/nose2/tests/functional/support/scenario/layers_and_non_layers/test_layers.py
@@ -0,0 +1,67 @@
+import unittest
+import logging
+from .common import UniqueResource, NormalTest, NormalTestTwo
+log = logging.getLogger(__name__)
+
+
+class Layer1(object):
+
+    @classmethod
+    def setUp(cls):
+        log.info("Called setup in layer 1")
+        cls.unique_resource = UniqueResource()
+        cls.unique_resource.lock()
+
+    @classmethod
+    def tearDown(cls):
+        log.info("Called teardown in layer 2")
+        cls.unique_resource.unlock()
+
+
+class Layer2(object):
+
+    @classmethod
+    def setUp(cls):
+        log.info("Called setup in layer 2")
+        cls.unique_resource = UniqueResource()
+        cls.unique_resource.lock()
+
+    @classmethod
+    def tearDown(cls):
+        log.info("Called teardown in layer 2")
+        cls.unique_resource.unlock()
+
+
+class Layer3(Layer2):
+
+    @classmethod
+    def setUp(cls):
+        log.info("Called setup in layer 3")
+
+    @classmethod
+    def tearDown(cls):
+        log.info("Called teardown in layer 3")
+
+
+class LayerTest1(unittest.TestCase):
+
+    layer = Layer1
+
+    def test(self):
+        self.assertTrue(self.layer.unique_resource.used)
+
+
+class LayerTest2(unittest.TestCase):
+
+    layer = Layer2
+
+    def test(self):
+        self.assertTrue(self.layer.unique_resource.used)
+
+
+class LayerTest3(unittest.TestCase):
+
+    layer = Layer2
+
+    def test(self):
+        self.assertTrue(self.layer.unique_resource.used)
diff --git a/nose2/tests/functional/support/scenario/layers_and_non_layers/test_such_with_has_setup.py b/nose2/tests/functional/support/scenario/layers_and_non_layers/test_such_with_has_setup.py
new file mode 100644
index 0000000..14268fe
--- /dev/null
+++ b/nose2/tests/functional/support/scenario/layers_and_non_layers/test_such_with_has_setup.py
@@ -0,0 +1,24 @@
+from nose2.tools import such
+import logging
+from .common import UniqueResource, NormalTest, NormalTestTwo
+log = logging.getLogger(__name__)
+
+
+with such.A('system with setup') as it:
+
+    @it.has_setup
+    def setup():
+        log.info("Called setup in such test")
+        it.unique_resource = UniqueResource()
+        it.unique_resource.lock()
+
+    @it.has_teardown
+    def teardown():
+        log.info("Called teardown in such test")
+        it.unique_resource.unlock()
+
+    @it.should('do something')
+    def test(case):
+        it.assertTrue(it.unique_resource.used)
+
+it.createTests(globals())
diff --git a/nose2/tests/functional/support/scenario/layers_and_non_layers/test_such_with_uses_decorator.py b/nose2/tests/functional/support/scenario/layers_and_non_layers/test_such_with_uses_decorator.py
new file mode 100644
index 0000000..ee15921
--- /dev/null
+++ b/nose2/tests/functional/support/scenario/layers_and_non_layers/test_such_with_uses_decorator.py
@@ -0,0 +1,51 @@
+from nose2.tools import such
+import logging
+from .common import UniqueResource, NormalTest, NormalTestTwo
+log = logging.getLogger(__name__)
+
+
+class Layer1(object):
+
+    description = 'Layer1'
+
+    @classmethod
+    def setUp(cls):
+        log.info("Called setup in layer 1")
+        it.unique_resource = UniqueResource()
+        it.unique_resource.lock()
+
+    @classmethod
+    def tearDown(cls):
+        log.info("Called teardown in layer 2")
+        it.unique_resource.unlock()
+
+
+class Layer2(object):
+
+    description = 'Layer2'
+
+    @classmethod
+    def setUp(cls):
+        log.info("Called setup in layer 2")
+
+    @classmethod
+    def tearDown(cls):
+        log.info("Called teardown in layer 2")
+
+with such.A('system with setup') as it:
+
+    it.uses(Layer1)
+
+    @it.should('do something')
+    def test(case):
+        it.assertTrue(it.unique_resource.used)
+
+    with it.having('another setup'):
+
+        it.uses(Layer2)
+
+        @it.should('do something else')
+        def test(case):
+            it.assertTrue(it.unique_resource.used)
+
+it.createTests(globals())
diff --git a/nose2/tests/functional/test_layers_plugin.py b/nose2/tests/functional/test_layers_plugin.py
index e072854..9666dba 100644
--- a/nose2/tests/functional/test_layers_plugin.py
+++ b/nose2/tests/functional/test_layers_plugin.py
@@ -120,3 +120,14 @@ Base
         self.assertTestRunOutputMatches(proc, stderr='ERROR: LayerSuite')
         self.assertTestRunOutputMatches(proc, stderr='FAIL')
         self.assertTestRunOutputMatches(proc, stderr='Bad Error in Layer setUp!')
+
+    def test_layers_and_non_layers(self):
+        proc = self.runIn(
+            'scenario/',
+            'layers_and_non_layers',
+            '-v',
+            '--plugin=nose2.plugins.layers',
+            )
+        self.assertTestRunOutputMatches(proc, stderr='Ran 12 tests in')
+        self.assertTestRunOutputMatches(proc, stderr='OK')
+        self.assertEqual(proc.poll(), 0)