From 3a61baeeb4fe4ac6b6ba2950d9371e729442a50a Mon Sep 17 00:00:00 2001
From: Frank Winklmeier <frank.winklmeier@cern.ch>
Date: Wed, 5 Sep 2018 11:52:40 +0200
Subject: [PATCH] AppMgr.py: Fix bug in backwards-compatibility code
Fix a (probably) harmless bug in some of the backwards-compatibility
code in AppMgr. There was a double negation (`not not`) and a comparison
to the wrong configurable type (Service instead of AlgTool).
In addition made the code handle lists of tools correct and added a unit
test for ToolSvc.__iadd__
---
Control/AthenaCommon/python/AppMgr.py | 21 +++---
.../AthenaCommon/share/AthAppMgrUnitTests.ref | 14 ++--
.../AthenaCommon/test/AthAppMgrUnitTests.py | 69 +++++++++++++------
3 files changed, 68 insertions(+), 36 deletions(-)
diff --git a/Control/AthenaCommon/python/AppMgr.py b/Control/AthenaCommon/python/AppMgr.py
index 35389b0641f..73977078a52 100755
--- a/Control/AthenaCommon/python/AppMgr.py
+++ b/Control/AthenaCommon/python/AppMgr.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
+# Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration
# File: AthenaCommon/share/AppMgr.py
# Author: Wim Lavrijsen (WLavrijsen@lbl.gov)
@@ -68,14 +68,19 @@ def release_metadata():
### associator for public tools ----------------------------------------------
def iadd( self, tool ):
- # only add once (allow silently)
- if tool in self.getChildren():
- return self
- # this is only allowed for new-style AlgTools
- if not not isinstance( tool, Configurable.ConfigurableService ):
- raise TypeError( '"%s" is not an AlgTool' %\
- (hasattr(tool,'name') and tool.name() or str(value) ) )
+ if not type(tool) in (list,tuple):
+ tool = (tool,)
+
+ # only add once (allow silently)
+ tool = [t for t in tool if t not in self.getChildren()]
+ if len(tool)==0: return self
+
+ # this is only allowed for new-style AlgTool
+ for t in tool:
+ if not isinstance( t, Configurable.ConfigurableAlgTool ):
+ raise TypeError( '"%s" is not an AlgTool' %
+ (hasattr(t,'name') and t.name() or "This configurable" ) )
super( GaudiSvcConf.ToolSvc, self ).__iadd__( tool )
diff --git a/Control/AthenaCommon/share/AthAppMgrUnitTests.ref b/Control/AthenaCommon/share/AthAppMgrUnitTests.ref
index 2a669070d6a..decc61df1b3 100644
--- a/Control/AthenaCommon/share/AthAppMgrUnitTests.ref
+++ b/Control/AthenaCommon/share/AthAppMgrUnitTests.ref
@@ -1,19 +1,21 @@
-Mon Mar 5 12:38:02 EST 2018
-WARNING: TCMALLOCDIR not defined, will use libc malloc
+Wed Sep 12 13:56:37 CEST 2018
+Preloading tcmalloc_minimal.so
Py:Athena INFO including file "AthenaCommon/Preparation.py"
-Py:Athena INFO using release [?-21.0.0] [i686-slc5-gcc43-dbg] [?/?] -- built on [?]
+Py:Athena INFO using release [AthenaExternals-22.0.1] [x86_64-slc6-gcc62-opt] [2.0.10/22f26cd] -- built on [2018-08-27T2029]
Py:Athena INFO including file "AthenaCommon/Atlas.UnixStandardJob.py"
Py:Athena INFO executing ROOT6Setup
Py:Athena INFO including file "AthenaCommon/Execution.py"
Py:Athena INFO including file "AthenaCommon/AthAppMgrUnitTests.py"
test1NamedSingleton (__main__.BasicAthAppMgrTestCase)
-Test that instances w/ same name are the same ... Py:Athena INFO using release [?-21.0.0] [i686-slc5-gcc43-dbg] [?/?] -- built on [?]
+Test that instances w/ same name are the same ... Py:Athena INFO using release [AthenaExternals-22.0.1] [x86_64-slc6-gcc62-opt] [2.0.10/22f26cd] -- built on [2018-08-27T2029]
ok
test2CppAppMgr (__main__.BasicAthAppMgrTestCase)
-Test communication b/w Py-proxy and C++ app mgr ... Py:Athena INFO using release [?-21.0.0] [i686-slc5-gcc43-dbg] [?/?] -- built on [?]
+Test communication b/w Py-proxy and C++ app mgr ... Py:Athena INFO using release [AthenaExternals-22.0.1] [x86_64-slc6-gcc62-opt] [2.0.10/22f26cd] -- built on [2018-08-27T2029]
ok
+test3ToolSvc (__main__.BasicAthAppMgrTestCase)
+Test ToolSvc __iadd__ ... ok
----------------------------------------------------------------------
-Ran 2 tests in 5.315s
+Ran 3 tests in 7.230s
OK
diff --git a/Control/AthenaCommon/test/AthAppMgrUnitTests.py b/Control/AthenaCommon/test/AthAppMgrUnitTests.py
index c5ead827e3c..89724590c60 100755
--- a/Control/AthenaCommon/test/AthAppMgrUnitTests.py
+++ b/Control/AthenaCommon/test/AthAppMgrUnitTests.py
@@ -1,14 +1,13 @@
#!/usr/bin/env python
-# Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
+# Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration
# Author: Sebastien Binet (binet@cern.ch)
"""Unit tests for verifying basic features of AthAppMgr."""
-import unittest, sys
+import unittest
from AthenaCommon.AppMgr import AthAppMgr
-from AthenaCommon.Logging import log as msg
### data ---------------------------------------------------------------------
__version__ = '$Revision: 1.1 $'
@@ -22,7 +21,6 @@ import sys
import os
### helper class to sanitize output-------------------------------------------
-import re
from tempfile import NamedTemporaryFile
class ShutUp(object):
"""
@@ -54,8 +52,8 @@ class BasicAthAppMgrTestCase( unittest.TestCase ):
app1 = AthAppMgr( "MyApp" )
app2 = AthAppMgr( "MyApp" )
- self.failUnless( app1 == app2, "instances are not equal !" )
- self.failUnless( app1 is app2, "instances are not identical !" )
+ self.assertTrue( app1 == app2, "instances are not equal !" )
+ self.assertTrue( app1 is app2, "instances are not identical !" )
def test2CppAppMgr( self ):
"""Test communication b/w Py-proxy and C++ app mgr"""
@@ -63,49 +61,49 @@ class BasicAthAppMgrTestCase( unittest.TestCase ):
createSvc = app.CreateSvc[:]
dlls = app.Dlls[:]
app.CreateSvc += [ "StoreGateSvc/abcde" ]
- self.failUnless( app.CreateSvc == createSvc + [ "StoreGateSvc/abcde" ] )
+ self.assertTrue( app.CreateSvc == createSvc + [ "StoreGateSvc/abcde" ] )
app.Dlls += [ "NothingThereLikeSuch" ]
- self.failUnless( app.Dlls == [] )
- self.failUnless( app._cppApp == None )
+ self.assertTrue( app.Dlls == [] )
+ self.assertTrue( app._cppApp is None )
Cout.instance.mute()
## instantiates the C++ application mgr (un-configured)
cppApp = app.getHandle()
Cout.instance.unMute()
- self.failUnless( app.Dlls == ["AthenaServices"] )
- self.failUnless( app._cppApp != None )
+ self.assertTrue( app.Dlls == ["AthenaServices"] )
+ self.assertTrue( app._cppApp is not None )
## now Dlls calls should be re-directed to C++ app
Cout.instance.mute()
app.Dlls += [ "AthenaKernel" ]
Cout.instance.unMute()
- self.failUnless( set(app.Dlls[:]) ==
+ self.assertTrue( set(app.Dlls[:]) ==
set(["AthenaServices","AthenaKernel"]) )
- self.failUnless( cppApp.CreateSvc == [] )
+ self.assertTrue( cppApp.CreateSvc == [] )
app.CreateSvc += [ "StoreGateSvc/svcA" ]
- self.failUnless( cppApp.CreateSvc == [ "StoreGateSvc/svcA" ] )
+ self.assertTrue( cppApp.CreateSvc == [ "StoreGateSvc/svcA" ] )
## configure the application (not just the app mgr)
app.setup()
- self.failUnless( cppApp.CreateSvc == app.CreateSvc )
+ self.assertTrue( cppApp.CreateSvc == app.CreateSvc )
# really want to test that ?
- self.failUnless( cppApp.CreateSvc[0] == "ToolSvc/ToolSvc" )
+ self.assertTrue( cppApp.CreateSvc[0] == "ToolSvc/ToolSvc" )
app.CreateSvc += [ "StoreGateSvc/svcB" ]
- self.failUnless( cppApp.CreateSvc == app.CreateSvc )
+ self.assertTrue( cppApp.CreateSvc == app.CreateSvc )
app.CreateSvc = [ "StoreGateSvc/svcC" ]
- self.failUnless( app.CreateSvc == [ "StoreGateSvc/svcC" ] )
- self.failUnless( cppApp.CreateSvc == [ "StoreGateSvc/svcC" ] )
+ self.assertTrue( app.CreateSvc == [ "StoreGateSvc/svcC" ] )
+ self.assertTrue( cppApp.CreateSvc == [ "StoreGateSvc/svcC" ] )
## test all properties are synchronized b/w py-proxy & C++
for p in AthAppMgr.__slots__:
- self.failUnless( getattr(cppApp,p) == getattr(app,p),
+ self.assertTrue( getattr(cppApp,p) == getattr(app,p),
"Property [%s] DIFFERS !!" % str(p) )
## test that multiple calls to setup() are stable
@@ -117,10 +115,37 @@ class BasicAthAppMgrTestCase( unittest.TestCase ):
Cout.instance.mute()
app.setup()
Cout.instance.unMute()
- self.failUnless( createSvc == app.CreateSvc[:] )
+ self.assertTrue( createSvc == app.CreateSvc[:] )
pass
return
-
+
+ def test3ToolSvc( self ):
+ """Test ToolSvc __iadd__"""
+ from AthenaCommon.AppMgr import ToolSvc
+ from AthenaCommon.Configurable import ConfigurableAlgTool, ConfigurableService
+
+ t1 = ConfigurableAlgTool('tool1')
+ t2 = ConfigurableAlgTool('tool2')
+ s1 = ConfigurableService('svc1')
+
+ ## Check silent adding of duplicate
+ ToolSvc += t1
+ self.assertEqual( ToolSvc.getChildren(), [t1] )
+ ToolSvc += t1
+ self.assertEqual( ToolSvc.getChildren(), [t1] )
+ ToolSvc += [t1,t2]
+ self.assertEqual( ToolSvc.getChildren(), [t1,t2] )
+
+ ## Check type checking
+ with self.assertRaises(TypeError):
+ ToolSvc += s1
+
+ with self.assertRaises(TypeError):
+ ToolSvc += [s1]
+
+ return
+
+
## actual test run
if __name__ == '__main__':
loader = unittest.TestLoader()
--
GitLab