Skip to content

Commit

Permalink
TestCase : Stop wrapped RefCounted classes outliving their tests
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
johnhaddon committed Nov 24, 2023
1 parent aa38ee9 commit f252eb5
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 38 deletions.
9 changes: 0 additions & 9 deletions python/GafferSceneTest/GroupTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
12 changes: 0 additions & 12 deletions python/GafferTest/ArrayPlugTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
17 changes: 0 additions & 17 deletions python/GafferTest/PerformanceMonitorTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
10 changes: 10 additions & 0 deletions python/GafferTest/TestCase.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand Down

0 comments on commit f252eb5

Please sign in to comment.