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__


Former-commit-id: 3a61baeeb4fe4ac6b6ba2950d9371e729442a50a

Control/AthenaCommon/python/AppMgr.py

-# 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 )
 

Control/AthenaCommon/share/AthAppMgrUnitTests.ref

-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

Control/AthenaCommon/test/AthAppMgrUnitTests.py

 #!/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()