tidy up env

testplan/common/entity/base.py

@@ -744,7 +744,14 @@ def filter_locals(cls, local_vars):
 
     def context_input(self) -> Dict[str, Any]:
         """All attr of self in a dict for context resolution"""
-        return {attr: getattr(self, attr) for attr in dir(self)}
+        ctx = {}
+        for attr in dir(self):
+            if attr == "env":
+                ctx["env"] = self._env
+            elif attr:
+                ctx[attr] = getattr(self, attr)
+        return ctx
+        # return {attr: getattr(self, attr) for attr in dir(self)}
 
 
 class RunnableStatus(EntityStatus):

testplan/common/utils/context.py

@@ -97,40 +97,6 @@ def expand(value, contextobj, constructor=None):
         return value
 
 
-def expand_env(
-    orig: Dict[str, str],
-    overrides: Dict[str, Union[str, ContextValue]],
-    contextobj,
-):
-    """
-    Copies the `orig` dict of environment variables.
-    Applies specified overrides.
-    Removes keys that have value of None as override.
-    Expands context values as strings.
-    Returns as a copy.
-
-    :param orig: The initial environment variables. Usually `os.environ` is to
-                 be passed in. This will not be modified.
-    :type orig: ``dict`` of ``str`` to ``str``
-    :param overrides: Keys and values to be overriden. Values can be strings or
-                      context objects.
-    :type overrides: ``dict`` of ``str`` to either ``str`` or ``ContextValue``
-    :param contextobj: The context object that can be used to expand context
-                       values.
-    :type contextobj: ``object``
-
-    :return: Copied, overridden and expanded environment variables
-    :rtype: ``dict``
-    """
-    env = orig.copy()
-    env.update(overrides)
-    return {
-        key: expand(val, contextobj, str)
-        for key, val in env.items()
-        if val is not None
-    }
-
-
 def render(template: Union[Template, TempitaTemplate, str], context) -> str:
     """
     Renders the template with the given context, that used for expression resolution.

testplan/runnable/base.py

@@ -869,7 +869,7 @@ def add(
             self.cfg.test_lister.log_test_info(task_info.materialized_test)
             return None
 
-        if resource is None or self.cfg.interactive_port:
+        if resource is None or self._is_interactive_run():
             # use local runner for interactive
             resource = self.resources.first()
             # just enqueue the materialized test

testplan/testing/base.py

@@ -479,7 +479,9 @@ class ProcessRunnerTest(Test):
     :type binary: ``str``
     :param description: Description of test instance.
     :type description: ``str``
-    :param proc_env: Environment overrides for ``subprocess.Popen``.
+    :param proc_env: Environment overrides for ``subprocess.Popen``;
+        context value (when referring to other driver) and jinja2 template (when
+        referring to self) will be resolved.
     :type proc_env: ``dict``
     :param proc_cwd: Directory override for ``subprocess.Popen``.
     :type proc_cwd: ``str``
@@ -681,14 +683,16 @@ def timeout_callback(self):
             )
 
     def get_proc_env(self):
+        """
+        Fabricate the env var for subprocess.
+        Precedence: user-specified > hardcoded > system env
+
+        """
 
+        # start with system env
         env = os.environ.copy()
-        proc_env = {
-            key.upper(): render(val, self.context_input())
-            for key, val in self.cfg.proc_env.items()
-        }
-        env.update(proc_env)
 
+        # override with hardcoded values
         json_ouput = os.path.join(self.runpath, "output.json")
         self.logger.debug("Json output: %s", json_ouput)
         env["JSON_REPORT"] = json_ouput
@@ -706,6 +710,13 @@ def get_proc_env(self):
                     ).upper()
                 ] = str(value)
 
+        # override with user specified values
+        proc_env = {
+            key.upper(): render(val, self.context_input())
+            for key, val in self.cfg.proc_env.items()
+        }
+        env.update(proc_env)
+
         return env
 
     def run_tests(self):

testplan/testing/multitest/driver/app.py

@@ -92,7 +92,9 @@ class App(Driver):
         can be a :py:class:`~testplan.common.utils.context.ContextValue`
         and will be expanded on runtime.
     :param shell: Invoke shell for command execution.
-    :param env: Environmental variables to be made available to child process.
+    :param env: Environmental variables to be made available to child process;
+        context value (when referring to other driver) and jinja2 template (when
+        referring to self) will be resolved.
     :param binary_strategy: Whether to copy / link binary to runpath.
     :param logname: Base name of driver logfile under `app_path`, in which
         Testplan will look for `log_regexps` as driver start-up condition.
@@ -139,6 +141,7 @@ def __init__(
         self._retcode = None
         self._log_matcher = None
         self._resolved_bin = None
+        self._env = None
 
     @emphasized
     @property
@@ -179,17 +182,22 @@ def cmd(self) -> str:
 
     @emphasized
     @property
-    def env(self) -> Dict[str, str]:
+    def env(self) -> Or(None, Dict[str, str]):
         """Environment variables."""
+
+        if self._env:
+            return self._env
+
         if isinstance(self.cfg.env, dict):
-            return {
-                key: expand(val, self.context, str)
-                if is_context(val)
-                else render(val, self.context_input())
+            ctx = self.context_input()
+            self._env = {
+                key: expand(val, self.context, str) if is_context(val)
+                # allowing None val for child class use case
+                else (render(val, ctx) if val is not None else None)
                 for key, val in self.cfg.env.items()
             }
-        else:
-            return None
+
+        return self._env
 
     @emphasized
     @property
@@ -236,26 +244,32 @@ def binpath(self) -> str:
     @emphasized
     @property
     def binary(self) -> str:
-        """The actual binary to execute"""
-        if not self._binary:
+        """The actual binary to execute, might be copied/linked to runpath"""
+
+        if self._binary and os.path.isfile(self._binary):
+            return self._binary
+
+        if os.path.isfile(self.resolved_bin):
+
             if self.cfg.path_cleanup is True:
                 name = os.path.basename(self.cfg.binary)
             else:
                 name = "{}-{}".format(
                     os.path.basename(self.cfg.binary), uuid.uuid4()
                 )
-
-            if os.path.isfile(self.resolved_bin):
-                target = os.path.join(self.binpath, name)
-                if self.cfg.binary_strategy == "copy":
-                    shutil.copyfile(self.resolved_bin, target)
-                    self._binary = target
-                elif self.cfg.binary_strategy == "link" and not IS_WIN:
-                    os.symlink(os.path.abspath(self.resolved_bin), target)
-                    self._binary = target
-                # else binary_strategy is noop then we don't do anything
-                else:
-                    self._binary = self.resolved_bin
+            target = os.path.join(self.binpath, name)
+
+            if self.cfg.binary_strategy == "copy":
+                shutil.copyfile(self.resolved_bin, target)
+                self._binary = target
+            elif self.cfg.binary_strategy == "link" and not IS_WIN:
+                os.symlink(os.path.abspath(self.resolved_bin), target)
+                self._binary = target
+            # else binary_strategy is noop then we don't do anything
+            else:
+                self._binary = self.resolved_bin
+        else:
+            self._binary = self.resolved_bin
 
         return self._binary
 

testplan/testing/multitest/driver/base.py

@@ -407,9 +407,7 @@ def install_files(self) -> None:
                 install_file = render(install_file, context)
                 if not os.path.isfile(install_file):
                     raise ValueError("{} is not a file".format(install_file))
-                instantiate(
-                    install_file, self.context_input(), self._install_target()
-                )
+                instantiate(install_file, context, self._install_target())
             elif isinstance(install_file, tuple):
                 if len(install_file) != 2:
                     raise ValueError(
@@ -419,7 +417,7 @@ def install_files(self) -> None:
                 src, dst = install_file
                 # may have jinja2/tempita template in file path
                 src = render(src, context)
-                dst = render(src, context)
+                dst = render(dst, context)
                 if not os.path.isabs(dst):
                     dst = os.path.join(self._install_target(), dst)
                 instantiate(src, self.context_input(), dst)

testplan/testing/multitest/driver/zookeeper.py

@@ -3,7 +3,7 @@
 """
 import os
 import socket
-from typing import Optional
+from typing import Optional, Dict
 
 from schema import Or
 
@@ -77,7 +77,7 @@ def __init__(
         )
         self._host = host
         self._port = port
-        self.env = self.cfg.env.copy() if self.cfg.env else {}
+        self._env = None
         self.config = None
         self.zkdata_path = None
         self.zklog_path = None
@@ -101,6 +101,15 @@ def port(self) -> int:
         """Port to listen on."""
         return self._port
 
+    @emphasized
+    @property
+    def env(self) -> Dict[str, str]:
+        """Environment variables."""
+        if self._env is not None:
+            return self._env
+        self._env = self.cfg.env.copy() if self.cfg.env else {}
+        return self._env
+
     @emphasized
     @property
     def connection_str(self) -> str:

tests/functional/testplan/runners/pools/test_auto_part.py

@@ -35,7 +35,7 @@ def test_auto_parts_discover():
         assert len(pool.added_items) == 5
         for task in pool.added_items.values():
             assert task.weight == 45
-
+        mockplan.run()
         assert pool.size == 2
 
 
@@ -64,7 +64,8 @@ def test_auto_parts_discover_interactive(runpath):
     )
 
     local_pool = mockplan.resources.get(mockplan.resources.first())
-    # validate that only on etask added to the local pool without split
+    # validate that only one task added to the local pool without split
+
     assert len(pool.added_items) == 0
     assert len(local_pool.added_items) == 1
 
@@ -94,5 +95,5 @@ def test_auto_weight_discover():
         assert len(pool.added_items) == 2
         for task in pool.added_items.values():
             assert task.weight == 140
-
+        mockplan.run()
         assert pool.size == 1

tests/unit/testplan/common/utils/test_context.py

@@ -8,7 +8,6 @@
 from testplan.common.utils.context import (
     ContextValue,
     expand,
-    expand_env,
     render,
 )
 
@@ -65,23 +64,6 @@ def test_expand(driver_context):
     assert expand(cv, driver_context, int) == 123
 
 
-def test_expand_env(driver_context):
-    env = dict(a="1", b="2")
-    overrides = dict(
-        c="str",
-        d="{{notcontext}}",
-        e=ContextValue("driver", "{{host}}"),
-        b=ContextValue("driver", "{{port}}"),
-    )
-    result = expand_env(env, overrides, driver_context)
-
-    assert result["a"] == "1"
-    assert result["b"] == "123"
-    assert result["c"] == "str"
-    assert result["d"] == "{{notcontext}}"
-    assert result["e"] == "host.ms.com"
-
-
 def test_render():
     context = dict(a="1", b=2)
     expected = "1:2"