The current handling of the configuration loading is inconsistent. After
the whole configuration is loaded, if there are any CLI or environment
overrides set, the code forcefully modifies the frozen configuration to
use them.

This changes the handling by passing the environment/CLI settings as
part of the configuration context and handle the overrides directly at
the field level before these are validated. As a positive side effect,
the validator won't complain if a field is missing from the file but it
has been specified as an environment/CLI override.

Bugzilla ID: 1360
Bugzilla ID: 1598

Signed-off-by: Nicholas Pratte <npra...@iol.unh.edu>
Signed-off-by: Luca Vizzarro <luca.vizza...@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepa...@arm.com>
---
 dts/framework/config/__init__.py | 40 ++++++++++++++++++++++++++++----
 dts/framework/runner.py          | 18 +++-----------
 2 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 2496f48e20..5510e3547b 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -36,7 +36,7 @@
 from enum import Enum, auto, unique
 from functools import cached_property
 from pathlib import Path, PurePath
-from typing import TYPE_CHECKING, Annotated, Any, Literal, NamedTuple
+from typing import TYPE_CHECKING, Annotated, Any, Literal, NamedTuple, 
TypedDict, cast
 
 import yaml
 from pydantic import (
@@ -44,18 +44,36 @@
     ConfigDict,
     Field,
     ValidationError,
+    ValidationInfo,
     field_validator,
     model_validator,
 )
 from typing_extensions import Self
 
 from framework.exception import ConfigurationError
+from framework.settings import Settings
 from framework.utils import REGEX_FOR_PCI_ADDRESS, StrEnum
 
 if TYPE_CHECKING:
     from framework.test_suite import TestSuiteSpec
 
 
+class ValidationContext(TypedDict):
+    """A context dictionary to use for validation."""
+
+    #: The command line settings.
+    settings: Settings
+
+
+def load_from_settings(data: Any, info: ValidationInfo):
+    """Before field validator that injects values from 
:attr:`ValidationContext.settings`."""
+    context = cast(ValidationContext, info.context)
+    assert info.field_name is not None, "This validator can only be used as a 
field validator."
+    if settings_data := getattr(context["settings"], info.field_name):
+        return settings_data
+    return data
+
+
 class FrozenModel(BaseModel):
     """A pre-configured :class:`~pydantic.BaseModel`."""
 
@@ -317,6 +335,10 @@ class BaseDPDKBuildConfiguration(FrozenModel):
     #: The location of the DPDK tree.
     dpdk_location: DPDKLocation
 
+    dpdk_location_from_settings = field_validator("dpdk_location", 
mode="before")(
+        load_from_settings
+    )
+
 
 class DPDKPrecompiledBuildConfiguration(BaseDPDKBuildConfiguration):
     """DPDK precompiled build configuration."""
@@ -325,6 +347,10 @@ class 
DPDKPrecompiledBuildConfiguration(BaseDPDKBuildConfiguration):
     #: subdirectory of `~dpdk_location.dpdk_tree` or `~dpdk_location.tarball` 
root directory.
     precompiled_build_dir: str = Field(min_length=1)
 
+    build_dir_from_settings = field_validator("precompiled_build_dir", 
mode="before")(
+        load_from_settings
+    )
+
 
 class DPDKBuildOptionsConfiguration(FrozenModel):
     """DPDK build options configuration.
@@ -439,6 +465,10 @@ class TestRunConfiguration(FrozenModel):
     #: The seed to use for pseudo-random generation.
     random_seed: int | None = None
 
+    fields_from_settings = field_validator("test_suites", "random_seed", 
mode="before")(
+        load_from_settings
+    )
+
 
 class TestRunWithNodesConfiguration(NamedTuple):
     """Tuple containing the configuration of the test run and its associated 
nodes."""
@@ -541,7 +571,7 @@ def validate_test_runs_with_nodes(self) -> Self:
         return self
 
 
-def load_config(config_file_path: Path) -> Configuration:
+def load_config(settings: Settings) -> Configuration:
     """Load DTS test run configuration from a file.
 
     Load the YAML test run configuration file, validate it, and create a test 
run configuration
@@ -552,6 +582,7 @@ def load_config(config_file_path: Path) -> Configuration:
 
     Args:
         config_file_path: The path to the YAML test run configuration file.
+        settings: The settings provided by the user on the command line.
 
     Returns:
         The parsed test run configuration.
@@ -559,10 +590,11 @@ def load_config(config_file_path: Path) -> Configuration:
     Raises:
         ConfigurationError: If the supplied configuration file is invalid.
     """
-    with open(config_file_path, "r") as f:
+    with open(settings.config_file_path, "r") as f:
         config_data = yaml.safe_load(f)
 
     try:
-        return Configuration.model_validate(config_data)
+        context = ValidationContext(settings=settings)
+        return Configuration.model_validate(config_data, context=context)
     except ValidationError as e:
         raise ConfigurationError("failed to load the supplied configuration") 
from e
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index 0cdbb07e06..e46a8c1a4f 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -31,7 +31,6 @@
 
 from .config import (
     Configuration,
-    DPDKPrecompiledBuildConfiguration,
     SutNodeConfiguration,
     TestRunConfiguration,
     TestSuiteConfig,
@@ -82,7 +81,7 @@ class DTSRunner:
 
     def __init__(self):
         """Initialize the instance with configuration, logger, result and 
string constants."""
-        self._configuration = load_config(SETTINGS.config_file_path)
+        self._configuration = load_config(SETTINGS)
         self._logger = get_dts_logger()
         if not os.path.exists(SETTINGS.output_dir):
             os.makedirs(SETTINGS.output_dir)
@@ -142,9 +141,7 @@ def run(self) -> None:
                 self._init_random_seed(test_run_config)
                 test_run_result = self._result.add_test_run(test_run_config)
                 # we don't want to modify the original config, so create a copy
-                test_run_test_suites = list(
-                    SETTINGS.test_suites if SETTINGS.test_suites else 
test_run_config.test_suites
-                )
+                test_run_test_suites = test_run_config.test_suites
                 if not test_run_config.skip_smoke_tests:
                     test_run_test_suites[:0] = 
[TestSuiteConfig(test_suite="smoke_tests")]
                 try:
@@ -320,15 +317,6 @@ def _run_test_run(
         test_run_result.sut_info = sut_node.node_info
         try:
             dpdk_build_config = test_run_config.dpdk_config
-            if new_location := SETTINGS.dpdk_location:
-                dpdk_build_config = dpdk_build_config.model_copy(
-                    update={"dpdk_location": new_location}
-                )
-            if dir := SETTINGS.precompiled_build_dir:
-                dpdk_build_config = DPDKPrecompiledBuildConfiguration(
-                    dpdk_location=dpdk_build_config.dpdk_location,
-                    precompiled_build_dir=dir,
-                )
             sut_node.set_up_test_run(test_run_config, dpdk_build_config)
             test_run_result.dpdk_build_info = sut_node.get_dpdk_build_info()
             tg_node.set_up_test_run(test_run_config, dpdk_build_config)
@@ -622,6 +610,6 @@ def _exit_dts(self) -> None:
 
     def _init_random_seed(self, conf: TestRunConfiguration) -> None:
         """Initialize the random seed to use for the test run."""
-        seed = SETTINGS.random_seed or conf.random_seed or 
random.randrange(0xFFFF_FFFF)
+        seed = conf.random_seed or random.randrange(0xFFFF_FFFF)
         self._logger.info(f"Initializing test run with random seed {seed}.")
         random.seed(seed)
-- 
2.43.0

Reply via email to