[PATCH v2 6/7] dts: remove multi-inheritance classes

[Luca Vizzarro <luca.vizzarro@arm.com>]

Multi inheritance has proven to be flaky in Python, therefore revert
back to a simpler approach where classes will only inherit one base
class. As part of this change, instead of making the
ScapyTrafficGenerator class inehrit a PythonShell, use simple class
composition, by making the shell a class attribute.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
---
 .../remote_session/interactive_shell.py       |  9 +----
 .../testbed_model/traffic_generator/scapy.py  | 38 +++++++++----------
 .../traffic_generator/traffic_generator.py    |  6 +--
 dts/framework/utils.py                        | 14 -------
 4 files changed, 22 insertions(+), 45 deletions(-)

diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
index 6b7ca0b6af..d7e566e5c4 100644
--- a/dts/framework/remote_session/interactive_shell.py
+++ b/dts/framework/remote_session/interactive_shell.py
@@ -37,10 +37,9 @@
 from framework.params import Params
 from framework.settings import SETTINGS
 from framework.testbed_model.node import Node
-from framework.utils import MultiInheritanceBaseClass
 
 
-class InteractiveShell(MultiInheritanceBaseClass, ABC):
+class InteractiveShell(ABC):
     """The base class for managing interactive shells.
 
     This class shouldn't be instantiated directly, but instead be extended. It contains
@@ -90,20 +89,15 @@ def __init__(
         name: str | None = None,
         privileged: bool = False,
         app_params: Params = Params(),
-        **kwargs,
     ) -> None:
         """Create an SSH channel during initialization.
 
-        Additional keyword arguments can be passed through `kwargs` if needed for fulfilling other
-        constructors in the case of multiple inheritance.
-
         Args:
             node: The node on which to run start the interactive shell.
             name: Name for the interactive shell to use for logging. This name will be appended to
                 the name of the underlying node which it is running on.
             privileged: Enables the shell to run as superuser.
             app_params: The command line parameters to be passed to the application on startup.
-            **kwargs: Any additional arguments if any.
         """
         self._node = node
         if name is None:
@@ -112,7 +106,6 @@ def __init__(
         self._app_params = app_params
         self._privileged = privileged
         self._timeout = SETTINGS.timeout
-        super().__init__(**kwargs)
 
     def _setup_ssh_channel(self):
         self._ssh_channel = self._node.main_session.interactive_session.session.invoke_shell()
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index 520561b2eb..78a6ded74c 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -34,7 +34,7 @@
 from .capturing_traffic_generator import CapturingTrafficGenerator
 
 
-class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
+class ScapyTrafficGenerator(CapturingTrafficGenerator):
     """Provides access to scapy functions on a traffic generator node.
 
     This class extends the base with remote execution of scapy functions. All methods for
@@ -56,6 +56,8 @@ class also extends :class:`.capturing_traffic_generator.CapturingTrafficGenerato
     first.
     """
 
+    _shell: PythonShell
+
     _config: ScapyTrafficGeneratorConfig
 
     #: Name of sniffer to ensure the same is used in all places
@@ -82,25 +84,21 @@ def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig, **kwargs)
             tg_node.config.os == OS.linux
         ), "Linux is the only supported OS for scapy traffic generation"
 
-        super().__init__(node=tg_node, config=config, tg_node=tg_node, **kwargs)
-        self.start_application()
+        super().__init__(tg_node=tg_node, config=config, **kwargs)
+        self._shell = PythonShell(tg_node, "scapy", privileged=True)
 
     def setup(self, ports: Iterable[Port]):
         """Extends :meth:`.traffic_generator.TrafficGenerator.setup`.
 
-        Brings up the port links.
+        Starts up the traffic generator and brings up the port links.
         """
-        super().setup(ports)
         self._tg_node.main_session.bring_up_link(ports)
+        self._shell.start_application()
+        self._shell.send_command("from scapy.all import *")
 
-    def start_application(self) -> None:
-        """Extends :meth:`framework.remote_session.interactive_shell.start_application`.
-
-        Adds a command that imports everything from the scapy library immediately after starting
-        the shell for usage in later calls to the methods of this class.
-        """
-        super().start_application()
-        self.send_command("from scapy.all import *")
+    def close(self):
+        """Close traffic generator."""
+        self._shell.close()
 
     def _send_packets(self, packets: list[Packet], port: Port) -> None:
         """Implementation for sending packets without capturing any received traffic.
@@ -116,7 +114,7 @@ def _send_packets(self, packets: list[Packet], port: Port) -> None:
             "verbose=True",
             ")",
         ]
-        self.send_command(f"\n{self._python_indentation}".join(send_command))
+        self._shell.send_command(f"\n{self._python_indentation}".join(send_command))
 
     def _send_packets_and_capture(
         self,
@@ -155,7 +153,7 @@ def _shell_set_packet_list(self, packets: list[Packet]) -> None:
             packets: The list of packets to recreate in the shell.
         """
         self._logger.info("Building a list of packets to send.")
-        self.send_command(
+        self._shell.send_command(
             f"{self._send_packet_list_name} = [{', '.join(map(Packet.command, packets))}]"
         )
 
@@ -202,7 +200,7 @@ def _shell_create_sniffer(
         """
         self._shell_set_packet_list(packets_to_send)
 
-        self.send_command("import time")
+        self._shell.send_command("import time")
         sniffer_commands = [
             f"{self._sniffer_name} = AsyncSniffer(",
             f"iface='{recv_port.logical_name}',",
@@ -220,7 +218,7 @@ def _shell_create_sniffer(
         if filter_config:
             sniffer_commands.insert(-1, f"filter='{filter_config}'")
 
-        self.send_command(f"\n{self._python_indentation}".join(sniffer_commands))
+        self._shell.send_command(f"\n{self._python_indentation}".join(sniffer_commands))
 
     def _shell_start_and_stop_sniffing(self, duration: float) -> list[Packet]:
         """Start asynchronous sniffer, run for a set `duration`, then collect received packets.
@@ -237,12 +235,12 @@ def _shell_start_and_stop_sniffing(self, duration: float) -> list[Packet]:
             A list of all packets that were received while the sniffer was running.
         """
         sniffed_packets_name = "gathered_packets"
-        self.send_command(f"{self._sniffer_name}.start()")
+        self._shell.send_command(f"{self._sniffer_name}.start()")
         # Insert a one second delay to prevent timeout errors from occurring
         time.sleep(duration + 1)
-        self.send_command(f"{sniffed_packets_name} = {self._sniffer_name}.stop(join=True)")
+        self._shell.send_command(f"{sniffed_packets_name} = {self._sniffer_name}.stop(join=True)")
         # An extra newline is required here due to the nature of interactive Python shells
-        packet_strs = self.send_command(
+        packet_strs = self._shell.send_command(
             f"for pakt in {sniffed_packets_name}: print(bytes_base64(pakt.build()))\n"
         )
         # In the string of bytes "b'XXXX'", we only want the contents ("XXXX")
diff --git a/dts/framework/testbed_model/traffic_generator/traffic_generator.py b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
index 804662e114..fbd9771eba 100644
--- a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
+++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
@@ -17,10 +17,10 @@
 from framework.logger import DTSLogger, get_dts_logger
 from framework.testbed_model.node import Node
 from framework.testbed_model.port import Port
-from framework.utils import MultiInheritanceBaseClass, get_packet_summaries
+from framework.utils import get_packet_summaries
 
 
-class TrafficGenerator(MultiInheritanceBaseClass, ABC):
+class TrafficGenerator(ABC):
     """The base traffic generator.
 
     Exposes the common public methods of all traffic generators and defines private methods
@@ -48,13 +48,13 @@ def __init__(self, tg_node: Node, config: TrafficGeneratorConfig, **kwargs):
         self._config = config
         self._tg_node = tg_node
         self._logger = get_dts_logger(f"{self._tg_node.name} {self._config.type}")
-        super().__init__(**kwargs)
 
     def setup(self, ports: Iterable[Port]):
         """Setup the traffic generator."""
 
     def teardown(self, ports: Iterable[Port]):
         """Teardown the traffic generator."""
+        self.close()
 
     def send_packet(self, packet: Packet, port: Port) -> None:
         """Send `packet` and block until it is fully sent.
diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index d6f4c11d58..24277633c0 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -299,20 +299,6 @@ def _make_packet() -> Packet:
     return [_make_packet() for _ in range(number_of)]
 
 
-class MultiInheritanceBaseClass:
-    """A base class for classes utilizing multiple inheritance.
-
-    This class enables it's subclasses to support both single and multiple inheritance by acting as
-    a stopping point in the tree of calls to the constructors of superclasses. This class is able
-    to exist at the end of the Method Resolution Order (MRO) so that subclasses can call
-    :meth:`super.__init__` without repercussion.
-    """
-
-    def __init__(self) -> None:
-        """Call the init method of :class:`object`."""
-        super().__init__()
-
-
 def to_pascal_case(text: str) -> str:
     """Convert `text` from snake_case to PascalCase."""
     return "".join([seg.capitalize() for seg in text.split("_")])
-- 
2.43.0