[v1,4/4] dts: refine pre-test setup and teardown steps

Message ID 20240423091252.62924-5-juraj.linkes@pantheon.tech (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series node and inheritance improvements |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS

Commit Message

Juraj Linkeš April 23, 2024, 9:12 a.m. UTC
  The major part is the removal of _set_up_execution() and
_tear_down_execution() of Node in lieu of using super() in the
superclasses, which simplifies the code while achieving the same thing.

The minor changes are the move of virtual devices and build target
setup/teardown into SutNode from Node since both are DPDK-related which
are only going to run on the SutNode.

Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 dts/framework/testbed_model/node.py     | 65 ++-----------------------
 dts/framework/testbed_model/sut_node.py | 49 ++++++++++++++-----
 2 files changed, 40 insertions(+), 74 deletions(-)
  

Comments

Luca Vizzarro April 23, 2024, 9:19 a.m. UTC | #1
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
  
Jeremy Spewock April 30, 2024, 4:15 p.m. UTC | #2
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
  

Patch

diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 74061f6262..1a98209822 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -16,12 +16,7 @@ 
 from ipaddress import IPv4Interface, IPv6Interface
 from typing import Any, Callable, Type, Union
 
-from framework.config import (
-    OS,
-    BuildTargetConfiguration,
-    ExecutionConfiguration,
-    NodeConfiguration,
-)
+from framework.config import OS, ExecutionConfiguration, NodeConfiguration
 from framework.exception import ConfigurationError
 from framework.logger import DTSLogger, get_dts_logger
 from framework.settings import SETTINGS
@@ -36,7 +31,6 @@ 
 from .linux_session import LinuxSession
 from .os_session import InteractiveShellType, OSSession
 from .port import Port
-from .virtual_device import VirtualDevice
 
 
 class Node(ABC):
@@ -55,7 +49,6 @@  class Node(ABC):
         lcores: The list of logical cores that DTS can use on the node.
             It's derived from logical cores present on the node and the test run configuration.
         ports: The ports of this node specified in the test run configuration.
-        virtual_devices: The virtual devices used on the node.
     """
 
     main_session: OSSession
@@ -65,8 +58,6 @@  class Node(ABC):
     ports: list[Port]
     _logger: DTSLogger
     _other_sessions: list[OSSession]
-    _execution_config: ExecutionConfiguration
-    virtual_devices: list[VirtualDevice]
 
     def __init__(self, node_config: NodeConfiguration):
         """Connect to the node and gather info during initialization.
@@ -94,7 +85,6 @@  def __init__(self, node_config: NodeConfiguration):
         ).filter()
 
         self._other_sessions = []
-        self.virtual_devices = []
         self._init_ports()
 
     def _init_ports(self) -> None:
@@ -106,67 +96,20 @@  def _init_ports(self) -> None:
     def set_up_execution(self, execution_config: ExecutionConfiguration) -> None:
         """Execution setup steps.
 
-        Configure hugepages and call :meth:`_set_up_execution` where
-        the rest of the configuration steps (if any) are implemented.
+        Configure hugepages on all DTS node types. Additional steps can be added by
+        extending the method in subclasses with the use of super().
 
         Args:
             execution_config: The execution test run configuration according to which
                 the setup steps will be taken.
         """
         self._setup_hugepages()
-        self._set_up_execution(execution_config)
-        self._execution_config = execution_config
-        for vdev in execution_config.vdevs:
-            self.virtual_devices.append(VirtualDevice(vdev))
-
-    def _set_up_execution(self, execution_config: ExecutionConfiguration) -> None:
-        """Optional additional execution setup steps for subclasses.
-
-        Subclasses should override this if they need to add additional execution setup steps.
-        """
 
     def tear_down_execution(self) -> None:
         """Execution teardown steps.
 
         There are currently no common execution teardown steps common to all DTS node types.
-        """
-        self.virtual_devices = []
-        self._tear_down_execution()
-
-    def _tear_down_execution(self) -> None:
-        """Optional additional execution teardown steps for subclasses.
-
-        Subclasses should override this if they need to add additional execution teardown steps.
-        """
-
-    def set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
-        """Build target setup steps.
-
-        There are currently no common build target setup steps common to all DTS node types.
-
-        Args:
-            build_target_config: The build target test run configuration according to which
-                the setup steps will be taken.
-        """
-        self._set_up_build_target(build_target_config)
-
-    def _set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
-        """Optional additional build target setup steps for subclasses.
-
-        Subclasses should override this if they need to add additional build target setup steps.
-        """
-
-    def tear_down_build_target(self) -> None:
-        """Build target teardown steps.
-
-        There are currently no common build target teardown steps common to all DTS node types.
-        """
-        self._tear_down_build_target()
-
-    def _tear_down_build_target(self) -> None:
-        """Optional additional build target teardown steps for subclasses.
-
-        Subclasses should override this if they need to add additional build target teardown steps.
+        Additional steps can be added by extending the method in subclasses with the use of super().
         """
 
     def create_session(self, name: str) -> OSSession:
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 800fbef860..ead6cc5a43 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -20,6 +20,7 @@ 
 from framework.config import (
     BuildTargetConfiguration,
     BuildTargetInfo,
+    ExecutionConfiguration,
     NodeInfo,
     SutNodeConfiguration,
 )
@@ -107,10 +108,12 @@  class SutNode(Node):
     or the git commit ID, tag ID or tree ID to test.
 
     Attributes:
-        config: The SUT node configuration
+        config: The SUT node configuration.
+        virtual_devices: The virtual devices used on the node.
     """
 
     config: SutNodeConfiguration
+    virtual_devices: list[VirtualDevice]
     _dpdk_prefix_list: list[str]
     _dpdk_timestamp: str
     _build_target_config: BuildTargetConfiguration | None
@@ -131,6 +134,7 @@  def __init__(self, node_config: SutNodeConfiguration):
             node_config: The SUT node's test run configuration.
         """
         super().__init__(node_config)
+        self.virtual_devices = []
         self._dpdk_prefix_list = []
         self._build_target_config = None
         self._env_vars = {}
@@ -228,25 +232,44 @@  def get_build_target_info(self) -> BuildTargetInfo:
     def _guess_dpdk_remote_dir(self) -> PurePath:
         return self.main_session.guess_dpdk_remote_dir(self._remote_tmp_dir)
 
-    def _set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
-        """Setup DPDK on the SUT node.
+    def set_up_execution(self, execution_config: ExecutionConfiguration) -> None:
+        """Extend the execution setup with vdev config.
 
-        Additional build target setup steps on top of those in :class:`Node`.
+        Args:
+            execution_config: The execution test run configuration according to which
+                the setup steps will be taken.
+        """
+        super().set_up_execution(execution_config)
+        for vdev in execution_config.vdevs:
+            self.virtual_devices.append(VirtualDevice(vdev))
+
+    def tear_down_execution(self) -> None:
+        """Extend the execution teardown with virtual device teardown."""
+        super().tear_down_execution()
+        self.virtual_devices = []
+
+    def set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
+        """Set up DPDK the SUT node and bind ports.
+
+        DPDK setup includes setting all internals needed for the build, the copying of DPDK tarball
+        and then building DPDK. The drivers are bound to those that DPDK needs.
+
+        Args:
+            build_target_config: The build target test run configuration according to which
+                the setup steps will be taken.
         """
-        # we want to ensure that dpdk_version and compiler_version is reset for new
-        # build targets
-        self._dpdk_version = None
-        self._compiler_version = None
         self._configure_build_target(build_target_config)
         self._copy_dpdk_tarball()
         self._build_dpdk()
         self.bind_ports_to_driver()
 
-    def _tear_down_build_target(self) -> None:
-        """Bind ports to the operating system drivers.
-
-        Additional build target teardown steps on top of those in :class:`Node`.
-        """
+    def tear_down_build_target(self) -> None:
+        """Reset DPDK variables and bind port driver to the OS driver."""
+        self._env_vars = {}
+        self._build_target_config = None
+        self.__remote_dpdk_dir = None
+        self._dpdk_version = None
+        self._compiler_version = None
         self.bind_ports_to_driver(for_dpdk=False)
 
     def _configure_build_target(self, build_target_config: BuildTargetConfiguration) -> None: