From 25eb8920ffc153c425dff6a5a51a4c483680c510 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9rome=20Perrin?= <jerome@nexedi.com>
Date: Tue, 2 May 2023 05:12:22 +0200
Subject: [PATCH] tests: execute `addCleanup` cleanups with ZODB connection

unittest executes the cleanups after `tearDown`, after the ZODB
connection is closed, so accessing database objects cause errors.

According to python unittest documentation, it is safe to call
`doCleanups` ourselves when we need the cleanup to be executed earlier,
this is a typical case where we want the cleanup to be called before
closing the database connections.
---
 .../ERP5Type/tests/ERP5TypeLiveTestCase.py    |  5 +-
 product/ERP5Type/tests/ERP5TypeTestCase.py    |  1 +
 .../tests/testDynamicClassGeneration.py       | 94 ++++++++++---------
 3 files changed, 57 insertions(+), 43 deletions(-)

diff --git a/product/ERP5Type/tests/ERP5TypeLiveTestCase.py b/product/ERP5Type/tests/ERP5TypeLiveTestCase.py
index 13f7f3739d..02d5adde49 100644
--- a/product/ERP5Type/tests/ERP5TypeLiveTestCase.py
+++ b/product/ERP5Type/tests/ERP5TypeLiveTestCase.py
@@ -162,7 +162,10 @@ class ERP5TypeLiveTestCase(ERP5TypeTestCaseMixin):
         self._setUpDummyMailHost()
 
     setUp = PortalTestCase.setUp
-    tearDown = PortalTestCase.tearDown
+
+    def tearDown(self):
+        self.doCleanups()
+        PortalTestCase.tearDown(self)
 
     def _app(self):
         '''Returns the app object for a test.'''
diff --git a/product/ERP5Type/tests/ERP5TypeTestCase.py b/product/ERP5Type/tests/ERP5TypeTestCase.py
index 85fb49b3d0..3f4de16786 100644
--- a/product/ERP5Type/tests/ERP5TypeTestCase.py
+++ b/product/ERP5Type/tests/ERP5TypeTestCase.py
@@ -1346,6 +1346,7 @@ class ERP5TypeCommandLineTestCase(ERP5TypeTestCaseMixin):
       '''Tears down the fixture. Do not override,
          use the hooks instead.
       '''
+      self.doCleanups()
       if not int(os.environ.get('erp5_save_data_fs', 0)):
         # Drop remaining activities if some of them failed.
         # However, we should not do more activity cleaning, because properly
diff --git a/product/ERP5Type/tests/testDynamicClassGeneration.py b/product/ERP5Type/tests/testDynamicClassGeneration.py
index 7fef78f177..84fe3db9c7 100644
--- a/product/ERP5Type/tests/testDynamicClassGeneration.py
+++ b/product/ERP5Type/tests/testDynamicClassGeneration.py
@@ -3237,17 +3237,19 @@ class Test(ERP5TypeTestCase):
     # set a request key, that should not be set from the test request
     self.portal.REQUEST.set('foo', 'something from main request')
 
-    # ERP5TypeLiveTestCase.runLiveTest patches ERP5TypeTestCase bases, thus it
-    # needs to be restored after calling runLiveTest
-    base_tuple = ERP5TypeTestCase.__bases__
-    ERP5TypeTestLoader.loadTestsFromNames = loadTestsFromNames
-    try:
-      self._component_tool.runLiveTest('testRunLiveTest')
-    finally:
-      ERP5TypeTestCase.__bases__ = base_tuple
-      ERP5TypeTestLoader.loadTestsFromNames = ERP5TypeTestLoader_loadTestsFromNames
+    def runLiveTest(test_name):
+      # ERP5TypeLiveTestCase.runLiveTest patches ERP5TypeTestCase bases, thus it
+      # needs to be restored after calling runLiveTest
+      base_tuple = ERP5TypeTestCase.__bases__
+      ERP5TypeTestLoader.loadTestsFromNames = loadTestsFromNames
+      try:
+        self._component_tool.runLiveTest(test_name)
+      finally:
+        ERP5TypeTestCase.__bases__ = base_tuple
+        ERP5TypeTestLoader.loadTestsFromNames = ERP5TypeTestLoader_loadTestsFromNames
+      return self._component_tool.readTestOutput()
 
-    output = self._component_tool.readTestOutput()
+    output = runLiveTest('testRunLiveTest')
     expected_msg_re = re.compile('Ran 1 test.*OK', re.DOTALL)
     self.assertRegex(output, expected_msg_re)
 
@@ -3265,18 +3267,35 @@ class Test(ERP5TypeTestCase):
     self._component_tool.reset(force=True,
                                reset_portal_type_at_transaction_boundary=True)
 
-    base_tuple = ERP5TypeTestCase.__bases__
-    ERP5TypeTestLoader.loadTestsFromNames = loadTestsFromNames
-    try:
-      self._component_tool.runLiveTest('testRunLiveTest')
-    finally:
-      ERP5TypeTestCase.__bases__ = base_tuple
-      ERP5TypeTestLoader.loadTestsFromNames = ERP5TypeTestLoader_loadTestsFromNames
-
-    output = self._component_tool.readTestOutput()
+    output = runLiveTest('testRunLiveTest')
     expected_msg_re = re.compile('Ran 2 tests.*FAILED \(failures=1\)', re.DOTALL)
     self.assertRegex(output, expected_msg_re)
 
+    # Now try addCleanup
+    source_code = self._getValidSourceCode() +  '''
+  def test_02_addCleanup(self):
+    self.portal.portal_activities.setTitle("changed")
+    self.tic()
+    def cleanup():
+      self.portal.portal_activities.setTitle({activity_tool_title!r})
+      self.tic()
+    self.addCleanup(cleanup)
+
+  def test_03_checkAfterCleanUp(self):
+    self.assertEqual(self.portal.portal_activities.getTitle(), {activity_tool_title!r})
+'''.format(activity_tool_title=self.portal.portal_activities.getTitle())
+
+    component.setTextContent(source_code)
+    self.tic()
+    self.assertEqual(component.getValidationState(), 'validated')
+    self.assertModuleImportable('testRunLiveTest')
+    self._component_tool.reset(force=True,
+                               reset_portal_type_at_transaction_boundary=True)
+
+    output = runLiveTest('testRunLiveTest')
+    expected_msg_re = re.compile('Ran 3 test.*OK', re.DOTALL)
+    self.assertRegex(output, expected_msg_re)
+
   def testERP5Broken(self):
     # Create a broken ghost object
     import erp5.portal_type
@@ -3517,39 +3536,30 @@ class TestZodbDocumentComponentReload(ERP5TypeTestCase):
   def testAsComposedDocumentCacheIsCorrectlyFlushed(self):
     component = self.portal.portal_components['document.erp5.BusinessProcess']
     component_original_text_content = component.getTextContent()
+    self.addCleanup(
+      self._setBusinessProcessComponentTextContent,
+      component_original_text_content)
 
-    # Use try/finally to restore the content of
-    # document.erp5.BusinessProcess as using addCleanup function here raises
-    # with :
-    #   ConnectionStateError: Shouldn't load state for
-    #   Products.DCWorkflow.Scripts.Scripts 0x099ad38a68606074
-    #   when the connection is closed
-    try:
-      self._setBusinessProcessComponentTextContent(
-        component_original_text_content + """
+    self._setBusinessProcessComponentTextContent(
+      component_original_text_content + """
   def getVersion(self):
     return 1
         """
-      )
+    )
 
-      movement = self.portal.newContent(portal_type='Movement')
-      composed_movement = movement.asComposedDocument()
-      self.assertEqual(composed_movement.getVersion(), 1)
+    movement = self.portal.newContent(portal_type='Movement')
+    composed_movement = movement.asComposedDocument()
+    self.assertEqual(composed_movement.getVersion(), 1)
 
-      self._setBusinessProcessComponentTextContent(
-        component_original_text_content + """
+    self._setBusinessProcessComponentTextContent(
+      component_original_text_content + """
   def getVersion(self):
     return 2
         """
-      )
-
-      composed_movement = movement.asComposedDocument()
-      self.assertEqual(composed_movement.getVersion(), 2)
+    )
 
-    finally:
-      self._setBusinessProcessComponentTextContent(
-        component_original_text_content
-      )
+    composed_movement = movement.asComposedDocument()
+    self.assertEqual(composed_movement.getVersion(), 2)
 
 
 def test_suite():
-- 
2.30.9