From f252eb55cb7bec230e7ed2f4f79df9ea3f8c9114 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 24 Nov 2023 15:54:01 +0000 Subject: [PATCH] TestCase : Stop wrapped `RefCounted` classes outliving their tests We were already doing this in a few specific tests but are now doing it for all. Our intention is still that our Python code should never make circular references between Python objects (and we have checks for that in `GafferUITest.TestCase.tearDown()` among other places), so we're not calling `gc.collect()`. But we can't do anything about the unfortunate circular references created by RefCountedBinding itself, and it's preferable to have those broken predictably at the end of the test rather than unpredictably when a later test runs. The immediate motivation for doing this is to destroy the `LocalDispatcher` instances and their associated `JobPool` in `LocalDispatcherTest`, before we run any other tests. If we don't, then the completed BackgroundTasks in the jobs cause a failure in `BoxTest.testComputeNodeCastDoesntRequirePython`. See previous commit for more details. --- python/GafferSceneTest/GroupTest.py | 9 --------- python/GafferTest/ArrayPlugTest.py | 12 ------------ python/GafferTest/PerformanceMonitorTest.py | 17 ----------------- python/GafferTest/TestCase.py | 10 ++++++++++ 4 files changed, 10 insertions(+), 38 deletions(-) diff --git a/python/GafferSceneTest/GroupTest.py b/python/GafferSceneTest/GroupTest.py index 480119874ce..02b7eb5a567 100644 --- a/python/GafferSceneTest/GroupTest.py +++ b/python/GafferSceneTest/GroupTest.py @@ -538,15 +538,6 @@ def testGroupInABox( self ) : self.assertTrue( s["g2"]["in"][0].getInput().isSame( b["out"] ) ) self.assertTrue( b["out"].getInput().isSame( b["g1"]["out"] ) ) - # this test was causing crashes elsewhere when the script - # was finally garbage collected, so we force the collection - # here so we can be sure the problem is fixed. - del s - del b - while gc.collect() : - pass - IECore.RefCounted.collectGarbage() - def testSetsWithRenaming( self ) : l1 = GafferSceneTest.TestLight() diff --git a/python/GafferTest/ArrayPlugTest.py b/python/GafferTest/ArrayPlugTest.py index af71ce2d9b1..4c75d9881e6 100644 --- a/python/GafferTest/ArrayPlugTest.py +++ b/python/GafferTest/ArrayPlugTest.py @@ -515,17 +515,5 @@ def testSerialisationUsesIndices( self ) : self.assertEqual( s2["n"]["in"][0].getInput(), s2["a"]["sum"] ) self.assertEqual( s2["n"]["in"][1].getInput(), s2["a"]["sum"] ) - def tearDown( self ) : - - # some bugs in the InputGenerator only showed themselves when - # the ScriptNode was deleted during garbage collection, often - # in totally unrelated tests. so we run the garbage collector - # here to localise any problems to this test, making them - # easier to diagnose and fix. - - while gc.collect() : - pass - IECore.RefCounted.collectGarbage() - if __name__ == "__main__": unittest.main() diff --git a/python/GafferTest/PerformanceMonitorTest.py b/python/GafferTest/PerformanceMonitorTest.py index f274b3356eb..b8fb9f37c26 100644 --- a/python/GafferTest/PerformanceMonitorTest.py +++ b/python/GafferTest/PerformanceMonitorTest.py @@ -46,23 +46,6 @@ class PerformanceMonitorTest( GafferTest.TestCase ) : - def setUp( self ) : - - GafferTest.TestCase.setUp( self ) - - # Clean up any garbage from previous tests. - # If python were to trigger the garbage collection - # of an old plug during a test here, it could - # clear the hash cache and give us unexpected - # results. - ## \todo If we clear or invalidate the hash cache - # selectively for only dirtied/deleted plugs, - # this won't be an issue and we could remove this - # workaround. - IECore.RefCounted.collectGarbage() - while gc.collect() : - pass - def testStatistics( self ) : m = Gaffer.PerformanceMonitor() diff --git a/python/GafferTest/TestCase.py b/python/GafferTest/TestCase.py index fa410204467..9ef5ee2d15e 100644 --- a/python/GafferTest/TestCase.py +++ b/python/GafferTest/TestCase.py @@ -104,6 +104,16 @@ def tearDown( self ) : # failure. traceback.clear_frames( self._outcome.expectedFailure[1].__traceback__ ) + # Garbage collect any wrapped RefCounted classes (they have an internal + # circular reference from C++->Python->C++), so they are destroyed now + # rather than potentially polluting other tests. Note that we're _not_ + # also calling `gc.collect()` because our intention is to never create + # circular references in Python code, and + # `GafferUITest.TestCast.tearDown()` is checking for them in all Widget + # subclasses. + ## \todo Fix Cortex so that wrapped classes don't require garbage collection. + IECore.RefCounted.collectGarbage() + for root, dirs, files in os.walk( self.temporaryDirectory() ) : for fileName in [ p for p in files + dirs if not ( pathlib.Path( root ) / p ).is_symlink() ] : ( pathlib.Path( root ) / fileName ).chmod( stat.S_IRWXU )