[1246] review comments

src/bin/bind10/bind10_src.py.in

@@ -44,12 +44,12 @@ import os
 # installed on the system
 if "B10_FROM_SOURCE" in os.environ:
     SPECFILE_LOCATION = os.environ["B10_FROM_SOURCE"] + "/src/bin/bind10/bob.spec"
-    ADD_LIBEXEC_AND_LD_PATH = False
+    ADD_LIBEXEC_PATH = False
 else:
     PREFIX = "@prefix@"
     DATAROOTDIR = "@datarootdir@"
     SPECFILE_LOCATION = "@datadir@/@PACKAGE@/bob.spec".replace("${datarootdir}", DATAROOTDIR).replace("${prefix}", PREFIX)
-    ADD_LIBEXEC_AND_LD_PATH = True
+    ADD_LIBEXEC_PATH = True
     
 import subprocess
 import signal
@@ -189,7 +189,7 @@ class ProcessInfo:
         # on construction (self.env).
         spawn_env = copy.deepcopy(os.environ)
         spawn_env.update(self.env)
-        if ADD_LIBEXEC_AND_LD_PATH:
+        if ADD_LIBEXEC_PATH:
             spawn_env['PATH'] = "@@LIBEXECDIR@@:" + spawn_env['PATH']
         self.process = subprocess.Popen(self.args,
                                         stdin=subprocess.PIPE,
@@ -359,7 +359,7 @@ class BoB:
     def start_creator(self):
         self.curproc = 'b10-sockcreator'
         creator_path = os.environ['PATH']
-        if ADD_LIBEXEC_AND_LD_PATH:
+        if ADD_LIBEXEC_PATH:
             creator_path = "@@LIBEXECDIR@@:" + creator_path
         self.sockcreator = isc.bind10.sockcreator.Creator(creator_path)
 
@@ -592,7 +592,11 @@ class BoB:
         # a cleaner solution, but for a short term workaround we specify the
         # path here, unconditionally, and without even bothering which
         # environment variable should be used.
-        if ADD_LIBEXEC_AND_LD_PATH:
+        #
+        # We reuse the ADD_LIBEXEC_PATH variable to see whether we need to
+        # do this, as the conditions that make this workaround needed are
+        # the same as for the libexec path addition
+        if ADD_LIBEXEC_PATH:
             cur_path = os.getenv('DYLD_LIBRARY_PATH')
             cur_path = '' if cur_path is None else ':' + cur_path
             c_channel_env['DYLD_LIBRARY_PATH'] = "@@LIBDIR@@" + cur_path

src/bin/bind10/tests/bind10_test.py.in

@@ -361,6 +361,10 @@ class TestStartStopProcessesBob(unittest.TestCase):
     of processes and that the right processes are started and stopped
     according to changes in configuration.
     """
+    def check_environment_unchanged(self):
+        # Check whether the environment has not been changed
+        self.assertEqual(original_os_environ, os.environ)
+
     def check_started(self, bob, core, auth, resolver):
         """
         Check that the right sets of services are started. The ones that
@@ -380,6 +384,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
         self.assertEqual(bob.stats, core)
         self.assertEqual(bob.stats_httpd, core)
         self.assertEqual(bob.cmdctl, core)
+        self.check_environment_unchanged()
 
     def check_preconditions(self, bob):
         self.check_started(bob, False, False, False)
@@ -390,6 +395,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
         should be started. Some processes still need to be running.
         """
         self.check_started(bob, True, False, False)
+        self.check_environment_unchanged()
 
     def check_started_both(self, bob):
         """
@@ -397,18 +403,21 @@ class TestStartStopProcessesBob(unittest.TestCase):
         (auth and resolver) are enabled.
         """
         self.check_started(bob, True, True, True)
+        self.check_environment_unchanged()
 
     def check_started_auth(self, bob):
         """
         Check the set of processes needed to run auth only is started.
         """
         self.check_started(bob, True, True, False)
+        self.check_environment_unchanged()
 
     def check_started_resolver(self, bob):
         """
         Check the set of processes needed to run resolver only is started.
         """
         self.check_started(bob, True, False, True)
+        self.check_environment_unchanged()
 
     def check_started_dhcp(self, bob, v4, v6):
         """
@@ -427,6 +436,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
         # there should be exactly one DHCPv6 daemon (if v6==True)
         self.assertEqual(v4==True, v4found==1)
         self.assertEqual(v6==True, v6found==1)
+        self.check_environment_unchanged()
 
     # Checks the processes started when starting neither auth nor resolver
     # is specified.
@@ -627,10 +637,6 @@ class TestStartStopProcessesBob(unittest.TestCase):
         #bob.cfg_start_dhcp4 = True
         #self.check_started_dhcp(bob, True, True)
 
-    def test_unchanged_environment(self):
-        # Check whether the environment has not been changed
-        self.assertEqual(original_os_environ, os.environ)
-
 class TestBossCmd(unittest.TestCase):
     def test_ping(self):
         """