[PR #3337] add testcase for tool lookup

Add a unit test to show find_program_path does not alter env['ENV']['PATH'].

A little cleanup in Tool/__init__.py: don't use mutable object as default
value in function signature (checkers complain about this); getter/setter
usage seemed unnecessary - kept one of the two but use modern syntax
(checkers complain about use-before set, which is fixed by the change)

Signed-off-by: Mats Wichmann <mats@linux.com>

2 files changed, 46 insertions, 23 deletions

diff --git a/src/engine/SCons/Tool/ToolTests.py b/src/engine/SCons/Tool/ToolTests.py
index f005143..f127f91 100644
--- a/src/engine/SCons/Tool/ToolTests.py
+++ b/src/engine/SCons/Tool/ToolTests.py
@@ -30,11 +30,14 @@ import TestUnit
 
 import SCons.Errors
 import SCons.Tool
+from SCons.Environment import Environment
+
 
 class ToolTestCase(unittest.TestCase):
     def test_Tool(self):
         """Test the Tool() function"""
-        class Environment(object):
+
+        class DummyEnvironment(object):
             def __init__(self):
                 self.dict = {}
             def Detect(self, progs):
@@ -53,7 +56,8 @@ class ToolTestCase(unittest.TestCase):
                 return key in self.dict
             def subst(self, string, *args, **kwargs):
                 return string
-        env = Environment()
+
+        env = DummyEnvironment()
         env['BUILDERS'] = {}
         env['ENV'] = {}
         env['PLATFORM'] = 'test'
@@ -78,6 +82,23 @@ class ToolTestCase(unittest.TestCase):
             raise
 
 
+    def test_pathfind(self):
+        """Test that find_program_path() does not alter PATH"""
+
+        PHONY_PATHS = [
+            r'C:\cygwin64\bin',
+            r'C:\cygwin\bin',
+            '/usr/local/dummy/bin',
+        ]
+
+        # Note this test cannot use the dummy environment,
+        # as function being tested calls env.WhereIs()
+        env = Environment()
+        pre_path = env['ENV']['PATH']
+        tool = SCons.Tool.find_program_path(env, 'no_tool', default_paths=PHONY_PATHS)
+        assert env['ENV']['PATH'] == pre_path, env['ENV']['PATH']
+
+
 if __name__ == "__main__":
     suite = unittest.makeSuite(ToolTestCase, 'test_')
     TestUnit.run(suite)
diff --git a/src/engine/SCons/Tool/__init__.py b/src/engine/SCons/Tool/__init__.py
index 166c3b0..cc935d1 100644
--- a/src/engine/SCons/Tool/__init__.py
+++ b/src/engine/SCons/Tool/__init__.py
@@ -104,7 +104,9 @@ TOOL_ALIASES = {
 
 
 class Tool(object):
-    def __init__(self, name, toolpath=[], **kw):
+    def __init__(self, name, toolpath=None, **kw):
+        if toolpath is None:
+            toolpath = []
 
         # Rename if there's a TOOL_ALIAS for this tool
         self.name = TOOL_ALIASES.get(name, name)
@@ -394,7 +396,8 @@ def _call_env_subst(env, string, *args, **kw):
 
 
 class _ShLibInfoSupport(object):
-    def get_libtype(self):
+    @property
+    def libtype(self):
         return 'ShLib'
 
     def get_lib_prefix(self, env, *args, **kw):
@@ -411,7 +414,8 @@ class _ShLibInfoSupport(object):
 
 
 class _LdModInfoSupport(object):
-    def get_libtype(self):
+    @property
+    def libtype(self):
         return 'LdMod'
 
     def get_lib_prefix(self, env, *args, **kw):
@@ -428,7 +432,8 @@ class _LdModInfoSupport(object):
 
 
 class _ImpLibInfoSupport(object):
-    def get_libtype(self):
+    @property
+    def libtype(self):
         return 'ImpLib'
 
     def get_lib_prefix(self, env, *args, **kw):
@@ -480,25 +485,21 @@ class _LibInfoGeneratorBase(object):
                         'ImpLib': _ImpLibInfoSupport}
 
     def __init__(self, libtype, infoname):
-        self.set_libtype(libtype)
-        self.set_infoname(infoname)
+        self.libtype = libtype
+        self.infoname = infoname
+
+    @property
+    def libtype(self):
+        return self._support.libtype
 
-    def set_libtype(self, libtype):
+    @libtype.setter
+    def libtype(self, libtype):
         try:
             support_class = self._support_classes[libtype]
         except KeyError:
             raise ValueError('unsupported libtype %r' % libtype)
         self._support = support_class()
 
-    def get_libtype(self):
-        return self._support.get_libtype()
-
-    def set_infoname(self, infoname):
-        self.infoname = infoname
-
-    def get_infoname(self):
-        return self.infoname
-
     def get_lib_prefix(self, env, *args, **kw):
         return self._support.get_lib_prefix(env, *args, **kw)
 
@@ -518,9 +519,8 @@ class _LibInfoGeneratorBase(object):
         try:
             libtype = kw['generator_libtype']
         except KeyError:
-            libtype = self.get_libtype()
-        infoname = self.get_infoname()
-        return 'Versioned%s%s' % (libtype, infoname)
+            libtype = self.libtype
+        return 'Versioned%s%s' % (libtype, self.infoname)
 
     def generate_versioned_lib_info(self, env, args, result=None, **kw):
         callback = self.get_versioned_lib_info_generator(**kw)
@@ -730,7 +730,7 @@ class _LibSonameGenerator(_LibInfoGeneratorBase):
 
         if not soname:
             # fallback to library name (as returned by appropriate _LibNameGenerator)
-            soname = _LibNameGenerator(self.get_libtype())(env, libnode)
+            soname = _LibNameGenerator(self.libtype)(env, libnode)
             if Verbose:
                 print("_LibSonameGenerator: FALLBACK: soname=%r" % soname)
 
@@ -1316,7 +1316,7 @@ def tool_list(platform, env):
     return [x for x in tools if x]
 
 
-def find_program_path(env, key_program, default_paths=[]):
+def find_program_path(env, key_program, default_paths=None):
     """
     Find the location of a tool using various means.
 
@@ -1338,6 +1338,8 @@ def find_program_path(env, key_program, default_paths=[]):
 
     # Finally, add the defaults and check again. Do not change
     # ['ENV']['PATH'] permananetly, the caller can do that if needed.
+    if default_paths is None:
+        return path
     save_path = env['ENV']['PATH']
     for p in default_paths:
         env.AppendENVPath('PATH', p)