[PATCH v4 7/7] dts: improve test suite and case filtering

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

The two places where we specify which test suite and test cases to run
are complimentary and not that intuitive to use. A unified way provides
a better user experience.

The syntax in test run configuration file has not changed, but the
environment variable and the command line arguments was changed to match
the config file syntax. This required changes in the settings module
which greatly simplified the parsing of the environment variables while
retaining the same functionality.

Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 doc/guides/tools/dts.rst         |  14 ++-
 dts/framework/config/__init__.py |  12 +-
 dts/framework/runner.py          |  18 +--
 dts/framework/settings.py        | 187 ++++++++++++++-----------------
 dts/framework/test_suite.py      |   2 +-
 5 files changed, 114 insertions(+), 119 deletions(-)

diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index f686ca487c..d1c3c2af7a 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -215,28 +215,30 @@ DTS is run with ``main.py`` located in the ``dts`` directory after entering Poet
 .. code-block:: console
 
    (dts-py3.10) $ ./main.py --help
-   usage: main.py [-h] [--config-file CONFIG_FILE] [--output-dir OUTPUT_DIR] [-t TIMEOUT] [-v] [-s] [--tarball TARBALL] [--compile-timeout COMPILE_TIMEOUT] [--test-cases TEST_CASES] [--re-run RE_RUN]
+   usage: main.py [-h] [--config-file CONFIG_FILE] [--output-dir OUTPUT_DIR] [-t TIMEOUT] [-v] [-s] [--tarball TARBALL] [--compile-timeout COMPILE_TIMEOUT] [--test-suite TEST_SUITE [TEST_CASES ...]] [--re-run RE_RUN]
 
    Run DPDK test suites. All options may be specified with the environment variables provided in brackets. Command line arguments have higher priority.
 
    options:
    -h, --help            show this help message and exit
    --config-file CONFIG_FILE
-                         [DTS_CFG_FILE] configuration file that describes the test cases, SUTs and targets. (default: ./conf.yaml)
+                         [DTS_CFG_FILE] The configuration file that describes the test cases, SUTs and targets. (default: ./conf.yaml)
    --output-dir OUTPUT_DIR, --output OUTPUT_DIR
                          [DTS_OUTPUT_DIR] Output directory where DTS logs and results are saved. (default: output)
    -t TIMEOUT, --timeout TIMEOUT
                          [DTS_TIMEOUT] The default timeout for all DTS operations except for compiling DPDK. (default: 15)
    -v, --verbose         [DTS_VERBOSE] Specify to enable verbose output, logging all messages to the console. (default: False)
-   -s, --skip-setup      [DTS_SKIP_SETUP] Specify to skip all setup steps on SUT and TG nodes. (default: None)
+   -s, --skip-setup      [DTS_SKIP_SETUP] Specify to skip all setup steps on SUT and TG nodes. (default: False)
    --tarball TARBALL, --snapshot TARBALL, --git-ref TARBALL
                          [DTS_DPDK_TARBALL] Path to DPDK source code tarball or a git commit ID, tag ID or tree ID to test. To test local changes, first commit them, then use the commit ID with this option. (default: dpdk.tar.xz)
    --compile-timeout COMPILE_TIMEOUT
                          [DTS_COMPILE_TIMEOUT] The timeout for compiling DPDK. (default: 1200)
-   --test-cases TEST_CASES
-                         [DTS_TESTCASES] Comma-separated list of test cases to execute. Unknown test cases will be silently ignored. (default: )
+   --test-suite TEST_SUITE [TEST_CASES ...]
+                         [DTS_TEST_SUITES] A list containing a test suite with test cases. The first parameter is the test suite name, and the rest are test case names, which are optional. May be specified multiple times. To specify multiple test suites in the environment
+                         variable, join the lists with a comma. Examples: --test-suite suite case case --test-suite suite case ... | DTS_TEST_SUITES='suite case case, suite case, ...' | --test-suite suite --test-suite suite case ... | DTS_TEST_SUITES='suite, suite case, ...'
+                         (default: [])
    --re-run RE_RUN, --re_run RE_RUN
-                         [DTS_RERUN] Re-run each test case the specified number of times if a test failure occurs (default: 0)
+                         [DTS_RERUN] Re-run each test case the specified number of times if a test failure occurs. (default: 0)
 
 
 The brackets contain the names of environment variables that set the same thing.
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index c6a93b3b89..4cb5c74059 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -35,9 +35,9 @@
 
 import json
 import os.path
-import pathlib
 from dataclasses import dataclass, fields
 from enum import auto, unique
+from pathlib import Path
 from typing import Union
 
 import warlock  # type: ignore[import]
@@ -53,7 +53,6 @@
     TrafficGeneratorConfigDict,
 )
 from framework.exception import ConfigurationError
-from framework.settings import SETTINGS
 from framework.utils import StrEnum
 
 
@@ -571,7 +570,7 @@ def from_dict(d: ConfigurationDict) -> "Configuration":
         return Configuration(executions=executions)
 
 
-def load_config() -> Configuration:
+def load_config(config_file_path: Path) -> Configuration:
     """Load DTS test run configuration from a file.
 
     Load the YAML test run configuration file
@@ -581,13 +580,16 @@ def load_config() -> Configuration:
     The YAML test run configuration file is specified in the :option:`--config-file` command line
     argument or the :envvar:`DTS_CFG_FILE` environment variable.
 
+    Args:
+        config_file_path: The path to the YAML test run configuration file.
+
     Returns:
         The parsed test run configuration.
     """
-    with open(SETTINGS.config_file_path, "r") as f:
+    with open(config_file_path, "r") as f:
         config_data = yaml.safe_load(f)
 
-    schema_path = os.path.join(pathlib.Path(__file__).parent.resolve(), "conf_yaml_schema.json")
+    schema_path = os.path.join(Path(__file__).parent.resolve(), "conf_yaml_schema.json")
 
     with open(schema_path, "r") as f:
         schema = json.load(f)
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index dfee8ebd7c..db8e3ba96b 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -24,7 +24,7 @@
 import sys
 from pathlib import Path
 from types import MethodType
-from typing import Iterable
+from typing import Iterable, Sequence
 
 from .config import (
     BuildTargetConfiguration,
@@ -83,7 +83,7 @@ class DTSRunner:
 
     def __init__(self):
         """Initialize the instance with configuration, logger, result and string constants."""
-        self._configuration = load_config()
+        self._configuration = load_config(SETTINGS.config_file_path)
         self._logger = get_dts_logger()
         if not os.path.exists(SETTINGS.output_dir):
             os.makedirs(SETTINGS.output_dir)
@@ -129,7 +129,7 @@ def run(self):
             #. Execution teardown
 
         The test cases are filtered according to the specification in the test run configuration and
-        the :option:`--test-cases` command line argument or
+        the :option:`--test-suite` command line argument or
         the :envvar:`DTS_TESTCASES` environment variable.
         """
         sut_nodes: dict[str, SutNode] = {}
@@ -147,7 +147,9 @@ def run(self):
                 )
                 execution_result = self._result.add_execution(execution)
                 # we don't want to modify the original config, so create a copy
-                execution_test_suites = list(execution.test_suites)
+                execution_test_suites = list(
+                    SETTINGS.test_suites if SETTINGS.test_suites else execution.test_suites
+                )
                 if not execution.skip_smoke_tests:
                     execution_test_suites[:0] = [TestSuiteConfig.from_dict("smoke_tests")]
                 try:
@@ -226,7 +228,7 @@ def _get_test_suites_with_cases(
             test_suite_class = self._get_test_suite_class(test_suite_config.test_suite)
             test_cases = []
             func_test_cases, perf_test_cases = self._filter_test_cases(
-                test_suite_class, set(test_suite_config.test_cases + SETTINGS.test_cases)
+                test_suite_class, test_suite_config.test_cases
             )
             if func:
                 test_cases.extend(func_test_cases)
@@ -302,7 +304,7 @@ def is_test_suite(object) -> bool:
         )
 
     def _filter_test_cases(
-        self, test_suite_class: type[TestSuite], test_cases_to_run: set[str]
+        self, test_suite_class: type[TestSuite], test_cases_to_run: Sequence[str]
     ) -> tuple[list[MethodType], list[MethodType]]:
         """Filter `test_cases_to_run` from `test_suite_class`.
 
@@ -331,7 +333,9 @@ def _filter_test_cases(
                 (name, method) for name, method in name_method_tuples if name in test_cases_to_run
             ]
             if len(name_method_tuples) < len(test_cases_to_run):
-                missing_test_cases = test_cases_to_run - {name for name, _ in name_method_tuples}
+                missing_test_cases = set(test_cases_to_run) - {
+                    name for name, _ in name_method_tuples
+                }
                 raise ConfigurationError(
                     f"Test cases {missing_test_cases} not found among methods "
                     f"of {test_suite_class.__name__}."
diff --git a/dts/framework/settings.py b/dts/framework/settings.py
index 2b8bfbe0ed..688e8679a7 100644
--- a/dts/framework/settings.py
+++ b/dts/framework/settings.py
@@ -48,10 +48,11 @@
 
     The path to a DPDK tarball, git commit ID, tag ID or tree ID to test.
 
-.. option:: --test-cases
-.. envvar:: DTS_TESTCASES
+.. option:: --test-suite
+.. envvar:: DTS_TEST_SUITES
 
-    A comma-separated list of test cases to execute. Unknown test cases will be silently ignored.
+        A test suite with test cases which may be specified multiple times.
+        In the environment variable, the suites are joined with a comma.
 
 .. option:: --re-run, --re_run
 .. envvar:: DTS_RERUN
@@ -71,83 +72,13 @@
 
 import argparse
 import os
-from collections.abc import Callable, Iterable, Sequence
 from dataclasses import dataclass, field
 from pathlib import Path
-from typing import Any, TypeVar
+from typing import Any
 
+from .config import TestSuiteConfig
 from .utils import DPDKGitTarball
 
-_T = TypeVar("_T")
-
-
-def _env_arg(env_var: str) -> Any:
-    """A helper method augmenting the argparse Action with environment variables.
-
-    If the supplied environment variable is defined, then the default value
-    of the argument is modified. This satisfies the priority order of
-    command line argument > environment variable > default value.
-
-    Arguments with no values (flags) should be defined using the const keyword argument
-    (True or False). When the argument is specified, it will be set to const, if not specified,
-    the default will be stored (possibly modified by the corresponding environment variable).
-
-    Other arguments work the same as default argparse arguments, that is using
-    the default 'store' action.
-
-    Returns:
-          The modified argparse.Action.
-    """
-
-    class _EnvironmentArgument(argparse.Action):
-        def __init__(
-            self,
-            option_strings: Sequence[str],
-            dest: str,
-            nargs: str | int | None = None,
-            const: bool | None = None,
-            default: Any = None,
-            type: Callable[[str], _T | argparse.FileType | None] = None,
-            choices: Iterable[_T] | None = None,
-            required: bool = False,
-            help: str | None = None,
-            metavar: str | tuple[str, ...] | None = None,
-        ) -> None:
-            env_var_value = os.environ.get(env_var)
-            default = env_var_value or default
-            if const is not None:
-                nargs = 0
-                default = const if env_var_value else default
-                type = None
-                choices = None
-                metavar = None
-            super(_EnvironmentArgument, self).__init__(
-                option_strings,
-                dest,
-                nargs=nargs,
-                const=const,
-                default=default,
-                type=type,
-                choices=choices,
-                required=required,
-                help=help,
-                metavar=metavar,
-            )
-
-        def __call__(
-            self,
-            parser: argparse.ArgumentParser,
-            namespace: argparse.Namespace,
-            values: Any,
-            option_string: str = None,
-        ) -> None:
-            if self.const is not None:
-                setattr(namespace, self.dest, self.const)
-            else:
-                setattr(namespace, self.dest, values)
-
-    return _EnvironmentArgument
-
 
 @dataclass(slots=True)
 class Settings:
@@ -171,7 +102,7 @@ class Settings:
     #:
     compile_timeout: float = 1200
     #:
-    test_cases: list[str] = field(default_factory=list)
+    test_suites: list[TestSuiteConfig] = field(default_factory=list)
     #:
     re_run: int = 0
 
@@ -180,6 +111,31 @@ class Settings:
 
 
 def _get_parser() -> argparse.ArgumentParser:
+    """Create the argument parser for DTS.
+
+    Command line options take precedence over environment variables, which in turn take precedence
+    over default values.
+
+    Returns:
+        argparse.ArgumentParser: The configured argument parser with defined options.
+    """
+
+    def env_arg(env_var: str, default: Any) -> Any:
+        """A helper function augmenting the argparse with environment variables.
+
+        If the supplied environment variable is defined, then the default value
+        of the argument is modified. This satisfies the priority order of
+        command line argument > environment variable > default value.
+
+        Args:
+            env_var: Environment variable name.
+            default: Default value.
+
+        Returns:
+            Environment variable or default value.
+        """
+        return os.environ.get(env_var) or default
+
     parser = argparse.ArgumentParser(
         description="Run DPDK test suites. All options may be specified with the environment "
         "variables provided in brackets. Command line arguments have higher priority.",
@@ -188,25 +144,23 @@ def _get_parser() -> argparse.ArgumentParser:
 
     parser.add_argument(
         "--config-file",
-        action=_env_arg("DTS_CFG_FILE"),
-        default=SETTINGS.config_file_path,
+        default=env_arg("DTS_CFG_FILE", SETTINGS.config_file_path),
         type=Path,
-        help="[DTS_CFG_FILE] configuration file that describes the test cases, SUTs and targets.",
+        help="[DTS_CFG_FILE] The configuration file that describes the test cases, "
+        "SUTs and targets.",
     )
 
     parser.add_argument(
         "--output-dir",
         "--output",
-        action=_env_arg("DTS_OUTPUT_DIR"),
-        default=SETTINGS.output_dir,
+        default=env_arg("DTS_OUTPUT_DIR", SETTINGS.output_dir),
         help="[DTS_OUTPUT_DIR] Output directory where DTS logs and results are saved.",
     )
 
     parser.add_argument(
         "-t",
         "--timeout",
-        action=_env_arg("DTS_TIMEOUT"),
-        default=SETTINGS.timeout,
+        default=env_arg("DTS_TIMEOUT", SETTINGS.timeout),
         type=float,
         help="[DTS_TIMEOUT] The default timeout for all DTS operations except for compiling DPDK.",
     )
@@ -214,9 +168,8 @@ def _get_parser() -> argparse.ArgumentParser:
     parser.add_argument(
         "-v",
         "--verbose",
-        action=_env_arg("DTS_VERBOSE"),
-        default=SETTINGS.verbose,
-        const=True,
+        action="store_true",
+        default=env_arg("DTS_VERBOSE", SETTINGS.verbose),
         help="[DTS_VERBOSE] Specify to enable verbose output, logging all messages "
         "to the console.",
     )
@@ -224,8 +177,8 @@ def _get_parser() -> argparse.ArgumentParser:
     parser.add_argument(
         "-s",
         "--skip-setup",
-        action=_env_arg("DTS_SKIP_SETUP"),
-        const=True,
+        action="store_true",
+        default=env_arg("DTS_SKIP_SETUP", SETTINGS.skip_setup),
         help="[DTS_SKIP_SETUP] Specify to skip all setup steps on SUT and TG nodes.",
     )
 
@@ -233,8 +186,7 @@ def _get_parser() -> argparse.ArgumentParser:
         "--tarball",
         "--snapshot",
         "--git-ref",
-        action=_env_arg("DTS_DPDK_TARBALL"),
-        default=SETTINGS.dpdk_tarball_path,
+        default=env_arg("DTS_DPDK_TARBALL", SETTINGS.dpdk_tarball_path),
         type=Path,
         help="[DTS_DPDK_TARBALL] Path to DPDK source code tarball or a git commit ID, "
         "tag ID or tree ID to test. To test local changes, first commit them, "
@@ -243,36 +195,71 @@ def _get_parser() -> argparse.ArgumentParser:
 
     parser.add_argument(
         "--compile-timeout",
-        action=_env_arg("DTS_COMPILE_TIMEOUT"),
-        default=SETTINGS.compile_timeout,
+        default=env_arg("DTS_COMPILE_TIMEOUT", SETTINGS.compile_timeout),
         type=float,
         help="[DTS_COMPILE_TIMEOUT] The timeout for compiling DPDK.",
     )
 
     parser.add_argument(
-        "--test-cases",
-        action=_env_arg("DTS_TESTCASES"),
-        default="",
-        help="[DTS_TESTCASES] Comma-separated list of test cases to execute.",
+        "--test-suite",
+        action="append",
+        nargs="+",
+        metavar=("TEST_SUITE", "TEST_CASES"),
+        default=env_arg("DTS_TEST_SUITES", SETTINGS.test_suites),
+        help="[DTS_TEST_SUITES] A list containing a test suite with test cases. "
+        "The first parameter is the test suite name, and the rest are test case names, "
+        "which are optional. May be specified multiple times. To specify multiple test suites in "
+        "the environment variable, join the lists with a comma. "
+        "Examples: "
+        "--test-suite suite case case --test-suite suite case ... | "
+        "DTS_TEST_SUITES='suite case case, suite case, ...' | "
+        "--test-suite suite --test-suite suite case ... | "
+        "DTS_TEST_SUITES='suite, suite case, ...'",
     )
 
     parser.add_argument(
         "--re-run",
         "--re_run",
-        action=_env_arg("DTS_RERUN"),
-        default=SETTINGS.re_run,
+        default=env_arg("DTS_RERUN", SETTINGS.re_run),
         type=int,
         help="[DTS_RERUN] Re-run each test case the specified number of times "
-        "if a test failure occurs",
+        "if a test failure occurs.",
     )
 
     return parser
 
 
+def _process_test_suites(args: str | list[list[str]]) -> list[TestSuiteConfig]:
+    """Process the given argument to a list of :class:`TestSuiteConfig` to execute.
+
+    Args:
+        args: The arguments to process. The args is a string from an environment variable
+              or a list of from the user input containing tests suites with tests cases,
+              each of which is a list of [test_suite, test_case, test_case, ...].
+
+    Returns:
+        A list of test suite configurations to execute.
+    """
+    if isinstance(args, str):
+        # Environment variable in the form of "suite case case, suite case, suite, ..."
+        args = [suite_with_cases.split() for suite_with_cases in args.split(",")]
+
+    test_suites_to_run = []
+    for suite_with_cases in args:
+        test_suites_to_run.append(
+            TestSuiteConfig(test_suite=suite_with_cases[0], test_cases=suite_with_cases[1:])
+        )
+
+    return test_suites_to_run
+
+
 def get_settings() -> Settings:
     """Create new settings with inputs from the user.
 
     The inputs are taken from the command line and from environment variables.
+
+    Returns:
+        The new settings object.
     """
     parsed_args = _get_parser().parse_args()
     return Settings(
@@ -287,6 +274,6 @@ def get_settings() -> Settings:
             else Path(parsed_args.tarball)
         ),
         compile_timeout=parsed_args.compile_timeout,
-        test_cases=(parsed_args.test_cases.split(",") if parsed_args.test_cases else []),
+        test_suites=_process_test_suites(parsed_args.test_suite),
         re_run=parsed_args.re_run,
     )
diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 365f80e21a..1957ea7328 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -40,7 +40,7 @@ class TestSuite(object):
     and functional test cases (all other test cases).
 
     By default, all test cases will be executed. A list of testcase names may be specified
-    in the YAML test run configuration file and in the :option:`--test-cases` command line argument
+    in the YAML test run configuration file and in the :option:`--test-suite` command line argument
     or in the :envvar:`DTS_TESTCASES` environment variable to filter which test cases to run.
     The union of both lists will be used. Any unknown test cases from the latter lists
     will be silently ignored.
-- 
2.34.1