From cd0f0a3d2e307b057b00a85ea924e510865939c9 Mon Sep 17 00:00:00 2001 From: Hank Wikle Date: Wed, 3 Dec 2025 14:48:02 -0700 Subject: [PATCH 1/8] Initial bisect command --- lib/pavilion/commands/__init__.py | 1 + lib/pavilion/commands/bisect.py | 120 ++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 lib/pavilion/commands/bisect.py diff --git a/lib/pavilion/commands/__init__.py b/lib/pavilion/commands/__init__.py index 7c76e2815..ea0236204 100644 --- a/lib/pavilion/commands/__init__.py +++ b/lib/pavilion/commands/__init__.py @@ -16,6 +16,7 @@ '_log_results': ('_log_results', 'LogResults'), '_run': ('_run', '_RunCommand'), '_series': ('_series', 'AutoSeries'), + 'bisect': ('bisect', 'BisectCommand'), 'build': ('build', 'BuildCommand'), 'cancel': ('cancel', 'CancelCommand'), 'cat': ('cat', 'CatCommand'), diff --git a/lib/pavilion/commands/bisect.py b/lib/pavilion/commands/bisect.py new file mode 100644 index 000000000..597eb43fc --- /dev/null +++ b/lib/pavilion/commands/bisect.py @@ -0,0 +1,120 @@ +from argparse import ArgumentParser + +from pavilion import output +from pavilion.errors import TestSeriesError +from pavilion.config import PavConfig +from pavilion.series.series import TestSeries +from pavilion.schedulers.config import parse_node_range +from .base_classes import Command + + +class BisectCommand(Command): + """Identify bad nodes by using a test to perform a binary search.""" + + def __init__(self): + super().__init__( + 'bisect', + 'Perform a bisection search on a given set of nodes using a particular test.', + short_help="Perform a bisection search." + ) + + def _setup_arguments(self, parser: ArgumentParser) -> None: + """Set up the parser arguments.""" + + parser.add_argument("test_name", type=str, + help="The name of the test to use to bisect the nodes.") + parser.add_argument("nodes", type=str, default="", + help="The set of nodes with which to start the search.") + parser.add_argument( + '-p', '--platform', action='store', + help='The platform to configure this test for. If not ' + 'specified, the current platform as denoted by the sys ' + 'plugin \'platform\' is used.') + parser.add_argument( + '-H', '--host', action='store', + help='The host to configure this test for. If not specified, the ' + 'current host as denoted by the sys plugin \'sys_host\' is ' + 'used. Host configurations are overlayed on operating system ' + 'configurations.') + parser.add_argument( + '-n', '--name', action='store', default='' + ) + parser.add_argument( + '-m', '--mode', action='append', dest='modes', default=[], + help='Mode configurations to overlay on the host configuration for ' + 'each test. These are overlayed in the order given.') + parser.add_argument( + '-c', dest='overrides', action='append', default=[], + help='Overrides for specific configuration options. These are ' + 'gathered used as a final set of overrides before the ' + 'configs are resolved. They should take the form ' + '\'key=value\', where key is the dot separated key name, ' + 'and value is a json object. Example: `-c schedule.nodes=23`') + + def run(pav_cfg: PavConfig, args: Namespace) -> int: + """Run the bisection search.""" + + try: + nodes = self._parse_nodes(args.nodes) + except ValueError: + output.fprint(self.errfile, f"Error parsing node list {args.nodes}.") + + return 1 + + # 1. Split the set of nodes in half. + num_nodes = len(nodes) + first_half = nodes[:num_nodes] + second_half = nodes[num_nodes:] + + # 2. Run the test on each set of nodes + series_cfg = generate_series_config( + name="bisect", + modes=args.modes, + platform=args.platform, + host=args.host, + overrides=args.overrides, + ignore_errors=args.ignore_errors, + ) + + series_obj = TestSeries(pav_cfg, series_cfg=series_cfg, outfile=self.outfile) + testset_name = cmd_utils.get_testset_name(pav_cfg, [args.test_name], []) + + series_obj.add_test_set_config( + testset_name, + [args.test_name], + modes=args.modes, + ) + + self.last_series = series_obj + + try: + series_obj.run( + rebuild=args.rebuild, + local_builds_only=local_builds_only, + log_results=log_results) + self.last_tests = list(series_obj.tests.values()) + except TestSeriesError as err: + self.last_tests = list(series_obj.tests.values()) + output.fprint(self.errfile, err, color=output.RED) + + return 2 + + # 3. Wait for tests to finish + series_obj.wait() + + # 4. Choose test with failed nodes and repeat + + return 0 + + @staticmethod + def _parse_nodes(nodes: str) -> List[str]: + """Parse a list of nodes into individual nodes.""" + + nodes = [] + + ranges = nodes.split(",") + + for rng in ranges: + nodes.append(parse_node_range(rng)) + + return nodes \ No newline at end of file From 10592ec04e66467bb50aedb08fcd465539501e21 Mon Sep 17 00:00:00 2001 From: Hank Wikle Date: Thu, 4 Dec 2025 14:20:43 -0700 Subject: [PATCH 2/8] Simplify overrides logic --- lib/pavilion/commands/run.py | 2 +- lib/pavilion/resolver/resolver.py | 229 ++++++++++--------------- lib/pavilion/series/series.py | 26 ++- lib/pavilion/series/test_set.py | 44 +++-- lib/pavilion/series_config/__init__.py | 18 +- lib/pavilion/utils.py | 17 ++ 6 files changed, 159 insertions(+), 177 deletions(-) diff --git a/lib/pavilion/commands/run.py b/lib/pavilion/commands/run.py index a1fee2f1f..95a3f0064 100644 --- a/lib/pavilion/commands/run.py +++ b/lib/pavilion/commands/run.py @@ -154,7 +154,7 @@ def run(self, pav_cfg, args, log_results: bool = True): platform=args.platform, host=args.host, repeat=getattr(args, 'repeat', None), - overrides=args.overrides, + overrides=TestConfigResolver.config_from_overrides(args.overrides), ignore_errors=args.ignore_errors, ) diff --git a/lib/pavilion/resolver/resolver.py b/lib/pavilion/resolver/resolver.py index 7b57fd12a..b9f34fe34 100644 --- a/lib/pavilion/resolver/resolver.py +++ b/lib/pavilion/resolver/resolver.py @@ -23,6 +23,7 @@ import similarity import yc_yaml import yaml_config as yc +from pavilion.config import PavConfig from pavilion.enums import Verbose from pavilion import output, variables from pavilion import pavilion_variables @@ -36,7 +37,7 @@ from pavilion.test_config.file_format import (TEST_NAME_RE, KEY_NAME_RE) from pavilion.test_config.file_format import TestConfigLoader, TestSuiteLoader -from pavilion.utils import union_dictionary +from pavilion.utils import union_dictionary, recursive_update from pavilion.micro import first, listmap from pavilion.path_utils import append_to_path, exists from yaml_config import RequiredError, YamlConfigLoader @@ -53,7 +54,7 @@ TEST_VERS_RE = re.compile(r'^\d+(\.\d+){0,2}$') -TestConfig = Dict +TestConfig = Dict[str, Any] class ConfigInfo: @@ -70,18 +71,22 @@ def __init__(self, name: str, type: str, path: Path, label: str = None, class TestOptions: """Test options from the command line or series configs.""" - def __init__(self, modes: List[str], overrides: List[str], conditions: Dict): - self.modes = modes if modes is not None else [] - self.overrides = overrides if overrides is not None else [] - self.conditions = conditions if conditions is not None else {} + def __init__(self, modes: List[str], overrides: TestConfig, conditions: Dict): + self.modes = modes or [] + self.overrides = overrides or {} + self.conditions = conditions or {} class TestConfigResolver: """Converts raw test configurations into their final, fully resolved form.""" - def __init__(self, pav_cfg, platform: str = None, host: str = None, - outfile: TextIO = None, verbosity: int = Verbose.QUIET): + def __init__(self, + pav_cfg: PavConfig, + platform: Optional[str] = None, + host: Optional[str] = None, + outfile: Optional[TextIO] = None, + verbosity: int = Verbose.QUIET): """Initialize the resolver. :param platform: The platform to configure tests for. @@ -382,8 +387,12 @@ def find_all_configs(self, conf_type: str): PROGRESS_PERIOD = 0.5 - def load_iter(self, tests: List[str], modes: List[str] = None, overrides: List[str] = None, - conditions=None, batch_size=None) -> Iterator[List[ProtoTest]]: + def load_iter(self, + tests: List[str], + modes: Optional[List[str]] = None, + overrides: Optional[TestConfig] = None, + conditions: Optional[Dict] = None, + batch_size: Optional[int] = None) -> Iterator[List[ProtoTest]]: """Load and fully resolve the requested tests. This returns an iterator of ProtoTest objects, which can be used to create the final test objects. Test resolution is delayed as long as possible, to keep in sync with system @@ -500,9 +509,12 @@ def load_iter(self, tests: List[str], modes: List[str] = None, overrides: List[s "Is `permute_on` set for that test?" .format(request.request, request.permutation))) - def load(self, tests: List[str], - modes: List[str] = None, overrides: List[str] = None, - conditions=None, throw_errors: bool = True) -> List[ProtoTest]: + def load(self, + tests: List[str], + modes: Optional[List[str]] = None, + overrides: Optional[TestConfig] = None, + conditions: Optional[Dict] = None, + throw_errors: bool = True) -> List[ProtoTest]: """As per ``load_iter`` except just return a list of all generated tests without any batching. This method is entirely meant for testing - the primary code should always use the iterator. @@ -602,6 +614,48 @@ def _resolve_escapes(self, ptests: ProtoTest) -> List[ProtoTest]: return multiplied_tests + @staticmethod + def config_from_overrides(overrides: List[str]) -> TestConfig: + """Parse a list of override strings and convert them into a test config.""" + + cfg = {} + + for ovr in overrides: + if '=' not in ovr: + raise ValueError( + "Invalid override value. Must be in the form: " + "=. Ex. -c run.modules=['gcc'] ") + + key, value = ovr.split('=', 1) + key = key.strip() + + if not key: + raise ValueError("Override '{}' given a blank key.".format(ovr)) + + key = key.split('.') + + for part in key: + if ' ' in part: + raise ValueError("Override '{}' has whitespace in its key.".format(ovr)) + if not part: + raise ValueError("Override '{}' has an empty key part.".format(ovr)) + + ovr_dict = {} + sub_cfg = ovr_dict + + for part in key[-1]: + sub_cfg[part] = {} + sub_cfg = sub_cfg.get(part) + + sub_cfg[key[-1]] = value + + try: + recursive_update(cfg, ovr_dict) + except ValueError as err: + raise TestConfigError("Error parsing override {ovr}.") + + return TestConfigLoader().normalize(cfg) + @staticmethod def _safe_load_config(cfg: ConfigInfo, loader: yc.YamlConfigLoader) -> TestConfig: """Given a path to a config, load the config, and raise an appropriate @@ -677,9 +731,7 @@ def _load_raw_configs(self, request: TestRequest, options: TestOptions) -> List[ as guaranteeing that they have 'variables' and 'permutations' sections. :param request: A test request to load tests for. - :param modes: A list (possibly empty) of modes to layer onto the test. - :param conditions: A list (possibly empty) of conditions to apply to each test config. - :param overrides: A list of overrides to apply to each test config. + :param options: A set of test options, including modes and overrides. :return: A list of RawProtoTests. """ @@ -1040,7 +1092,10 @@ def apply_platform(self, test_cfg: TestConfig, platform: str, raise TestConfigError( "Error merging configuration for platform '{}'".format(platform)) - def apply_modes(self, test_cfg, modes: List[str], suite_name: str = None): + def apply_modes(self, + test_cfg: TestConfig, + modes: List[str], + suite_name: Optional[str] = None) -> TestConfig: """Apply each of the mode files to the given test config. :param test_cfg: The raw test configuration. @@ -1082,6 +1137,22 @@ def apply_modes(self, test_cfg, modes: List[str], suite_name: str = None): return test_cfg + + def apply_overrides(test_cfg: TestConfig, overrides: TestConfig) -> TestConfig: + """Apply the dictionary of overrides to the given test config.""" + + for key in overrides.keys(): + if key in self.NOT_OVERRIDABLE: + raise KeyError("You can't override the '{}' key in a test config" + .format(key)) + + try: + return self._loader.merge(test_cfg, overrides) + except (KeyError, ValueError) as err: + raise TestConfigError( + "Error applying overrides to test config.\n{err}") + + def resolve_inheritance(self, suite_cfg, suite_path) \ -> Dict[str, dict]: """Resolve inheritance between tests in a test suite. There's potential @@ -1187,10 +1258,11 @@ def resolve_inheritance(self, suite_cfg, suite_path) \ return suite_tests - NOT_OVERRIDABLE = ['name', 'suite', 'suite_path', - 'base_name', 'host', 'platform', 'modes'] + NOT_OVERRIDABLE = ['name', 'suite', 'suite_path', 'base_name', 'host', 'platform', 'modes'] - def apply_overrides(self, test_cfg, overrides) -> Dict: + def apply_overrides(self, + test_cfg: TestConfig, + overrides: TestConfig) -> TestConfig: """Apply overrides to this test. :param dict test_cfg: The test configuration. @@ -1223,119 +1295,4 @@ def apply_overrides(self, test_cfg, overrides) -> Dict: try: return config_loader.normalize(test_cfg, root_name='overrides') except TypeError as err: - raise TestConfigError("Invalid override", prior_error=err) - - def _apply_override(self, test_cfg, key, value): - """Set the given key to the given value in test_cfg. - - :param dict test_cfg: The test configuration. - :param [str] key: A list of key components, like - ``[`slurm', 'num_nodes']`` - :param str value: The value to assign. If this looks like a json - structure, it will be decoded and treated as one. - """ - - cfg = test_cfg - - disp_key = '.'.join(key) - - if key[0] in self.NOT_OVERRIDABLE: - raise KeyError("You can't override the '{}' key in a test config" - .format(key[0])) - - key_copy = list(key) - last_cfg = None - last_key = None - - # Normalize simple variable values. - if key[0] == 'variables' and len(key) in (2, 3): - is_var_value = True - else: - is_var_value = False - - # Validate the key by walking the config according to the key - while key_copy: - part = key_copy.pop(0) - - if isinstance(cfg, list): - try: - idx = int(part) - except ValueError: - raise KeyError("Trying to override list item with a " - "non-integer '{}' in key '{}'." - .format(part, disp_key)) - - try: - last_cfg = cfg - last_key = idx - cfg = cfg[idx] - except IndexError: - raise KeyError( - "Trying to override index '{}' from key '{}' " - "but the index is out of range." - .format(part, disp_key)) - elif isinstance(cfg, dict): - - if part not in cfg and key_copy: - raise KeyError("Trying to override '{}' from key '{}', " - "but there is no such key." - .format(part, disp_key)) - - # It's ok to override a key that doesn't exist if it's the - # last key component. We'll validate everything anyway. - last_cfg = cfg - last_key = part - cfg = cfg.get(part, None) - else: - raise KeyError("Tried, to override key '{}', but '{}' isn't " - "a dict or list." - .format(disp_key, part)) - - if last_cfg is None: - # Should never happen. - raise RuntimeError( - "Trying to override an empty key: {}".format(key)) - - # We should be at the final place where the value should go. - try: - dummy_file = io.StringIO(value) - value = yc_yaml.safe_load(dummy_file) - except (yc_yaml.YAMLError, ValueError, KeyError) as err: - raise TestConfigError("Invalid value '{}' for key '{}' in overrides" - .format(value, disp_key), prior_error=err) - - last_cfg[last_key] = self.normalize_override_value(value, is_var_value) - - def normalize_override_value(self, value, is_var_value=False): - """Normalize a value to one compatible with Pavilion configs. It can - be any structure of dicts and lists, as long as the leaf values are - strings. - - :param value: The value to normalize. - :param is_var_value: True if the value will be used to set a variable value. - :returns: A string or a structure of dicts/lists whose leaves are - strings. - """ - - if isinstance(value, (int, float, bool, bytes)): - value = str(value) - - if isinstance(value, str): - if is_var_value: - # Normalize a simple value into the standard variable format. - return [{None: value}] - else: - return value - elif isinstance(value, (list, tuple)): - return [self.normalize_override_value(v) for v in value] - elif isinstance(value, dict): - dict_val = {str(k): self.normalize_override_value(v) - for k, v in value.items()} - - if is_var_value: - # Normalize a single dict item into a list of them for variables. - return [dict_val] - else: - return dict_val - else: - raise ValueError("Invalid type in override value: {}".format(value)) + raise TestConfigError("Invalid override", prior_error=err) \ No newline at end of file diff --git a/lib/pavilion/series/series.py b/lib/pavilion/series/series.py index 25214d1d5..e4a9b5719 100644 --- a/lib/pavilion/series/series.py +++ b/lib/pavilion/series/series.py @@ -11,6 +11,7 @@ import sys import subprocess import time +import copy from collections import defaultdict, OrderedDict from pathlib import Path from operator import attrgetter @@ -273,6 +274,10 @@ def _create_test_sets(self, iteration=0): else: _simultaneous = self.simultaneous + # Update overrides with set-specific overrides + overrides = copy.deepcopy(self.config.get("overrides", {})) + utils.recursive_update(overrides, set_info.get("overrides", {})) + set_obj = TestSet( pav_cfg=self.pav_cfg, name=set_name, @@ -281,10 +286,10 @@ def _create_test_sets(self, iteration=0): modes=universal_modes + set_info['modes'], platform=self.config.get('platform'), host=self.config.get('host'), + overrides=overrides, only_if=set_info['only_if'], not_if=set_info['not_if'], parents_must_pass=set_info['depends_pass'], - overrides=self.config.get('overrides', []), status=self.status, simultaneous= _simultaneous, outfile=self.outfile, @@ -674,13 +679,17 @@ def pgid(self) -> Optional[int]: return self._pgid - def add_test_set_config( - self, name, test_names: List[str], modes: List[str] = None, - only_if: Dict[str, List[str]] = None, - not_if: Dict[str, List[str]] = None, - simultaneous: int = None, - save: bool = True, - _depends_on: List[str] = None, _depends_pass: bool = False): + def add_test_set_config(self, + name: str, + test_names: List[str], + modes: Optional[List[str]] = None, + overrides: Optional[Dict[str, Any]] = None, + only_if: Optional[Dict[str, List[str]]] = None, + not_if: Optional[Dict[str, List[str]]] = None, + simultaneous: Optional[int] = None, + save: bool = True, + _depends_on: Optional[List[str]] = None, + _depends_pass: bool = False): """Manually add a test set to this series. The set will be added to the series config, and created when we create all sets for the series. After adding all set configs, call save_config to update the saved config. @@ -708,6 +717,7 @@ def add_test_set_config( 'depends_pass': _depends_pass, 'depends_on': _depends_on or [], 'modes': modes or [], + 'overrides': overrides or {}, 'only_if': only_if or {}, 'not_if': not_if or {}, 'simultaneous': simultaneous, diff --git a/lib/pavilion/series/test_set.py b/lib/pavilion/series/test_set.py index b36f362e7..cebe83767 100644 --- a/lib/pavilion/series/test_set.py +++ b/lib/pavilion/series/test_set.py @@ -8,16 +8,17 @@ import math from collections import defaultdict from io import StringIO -from typing import List, Dict, TextIO, Union, Set, Iterator, Tuple +from typing import List, Dict, TextIO, Union, Set, Iterator, Tuple, Optional import pavilion.errors +from pavilion.config import PavConfig from pavilion import output, result, schedulers, cancel_utils from pavilion.build_tracker import MultiBuildTracker from pavilion.errors import TestRunError, TestConfigError, TestSetError, ResultError from pavilion.resolver import TestConfigResolver from pavilion.status_file import SeriesStatusFile, STATES, SERIES_STATES from pavilion.test_run import TestRun, mass_status_update -from pavilion.utils import str_bool +from pavilion.utils import str_bool, recursive_update from pavilion.enums import Verbose from pavilion.jobs import Job from pavilion.micro import set_default @@ -38,19 +39,19 @@ class TestSet: # need info like # modes, only/not_ifs, next, prev def __init__(self, - pav_cfg, + pav_cfg: PavConfig, name: str, test_names: List[str], iteration: int = 0, - status: SeriesStatusFile = None, - modes: List[str] = None, - host: str = None, - platform: str = None, - only_if: Dict[str, List[str]] = None, - not_if: Dict[str, List[str]] = None, - overrides: List = None, + status: Optional[SeriesStatusFile] = None, + modes: Optional[List[str]] = None, + host: Optional[str] = None, + platform: Optional[str] = None, + overrides: Optional[Dict[str, Any]] = None, + only_if: Optional[Dict[str, List[str]]] = None, + not_if: Optional[Dict[str, List[str]]] = None, parents_must_pass: bool = False, - simultaneous: Union[int, None] = None, + simultaneous: Optional[int] = None, ignore_errors: bool = False, outfile: TextIO = StringIO(), verbosity=Verbose.QUIET): @@ -94,7 +95,7 @@ def __init__(self, self.only_if = only_if or {} self.not_if = not_if or {} self.pav_cfg = pav_cfg - self.overrides = overrides or [] + self.overrides = overrides or {} self.parent_sets = set() # type: Set[TestSet] self.child_sets = set() # type: Set[TestSet] @@ -116,14 +117,14 @@ def __init__(self, self.status.set(S_STATES.SET_CREATED, "Created test set {}.".format(self.name)) - def add_parents(self, *parents: 'TestSet'): + def add_parents(self, *parents: 'TestSet') -> None: """Add the given TestSets as a parent to this one.""" for parent in parents: self.parent_sets.add(parent) parent.child_sets.add(self) - def remove_parent(self, parent: 'TestSet'): + def remove_parent(self, parent: 'TestSet') -> None: """Remove the given parent from this test set.""" try: @@ -189,10 +190,12 @@ def __ordered_split(self) -> List['TestSet']: return test_sets - def make_iter(self, build_only=False, rebuild=False, local_builds_only=False) \ - -> Iterator[List[TestRun]]: + def make_iter(self, + build_only: bool = False, + rebuild: bool = False, + local_builds_only: bool = False) -> Iterator[List[TestRun]]: """Resolve the given tests names and options into actual test run objects, and print - the test creation status. This returns an iterator over batches tests, respecting the + the test creation status. This returns an iterator over batches of tests, respecting the batch_size (half the simultanious limit). """ @@ -204,12 +207,7 @@ def make_iter(self, build_only=False, rebuild=False, local_builds_only=False) \ } if build_only: - for override in self.overrides: - if override.startswith('schedule.nodes'): - self.overrides.remove(override) - break - - self.overrides.append('schedule.nodes=1') + recursive_update(self.overrides, {"schedule": {"nodes": 1}}) cfg_resolver = TestConfigResolver( self.pav_cfg, diff --git a/lib/pavilion/series_config/__init__.py b/lib/pavilion/series_config/__init__.py index 64127bba8..3526ed90c 100644 --- a/lib/pavilion/series_config/__init__.py +++ b/lib/pavilion/series_config/__init__.py @@ -1,5 +1,5 @@ import os -from typing import List +from typing import List, Dict, Optional import yc_yaml import yaml_config @@ -135,15 +135,15 @@ def verify_configs(pav_cfg, series_name: str, platform: str = None, def generate_series_config( name: str, - platform: str = None, - host: str = None, - modes: List[str] = None, - ordered: bool = None, - overrides: List[str] = None, - repeat: int = None, - simultaneous: int = None, + platform: Optional[str] = None, + host: Optional[str] = None, + modes: Optional[List[str]] = None, + ordered: Optional[bool] = None, + overrides: Optional[Dict[str, Any]] = None, + repeat: Optional[int] = None, + simultaneous: Optional[int] = None, ignore_errors: bool = False, - ) -> dict: + ) -> Dict[str, Any]: """Generates series config given global series settings. To add test sets, create a series with this config and use the add_test_set_config() method.""" diff --git a/lib/pavilion/utils.py b/lib/pavilion/utils.py index 6b03ad37d..d332d8a53 100644 --- a/lib/pavilion/utils.py +++ b/lib/pavilion/utils.py @@ -13,6 +13,7 @@ import textwrap import zipfile from pathlib import Path +from collections.abc import Mapping from typing import Iterator, Union, TextIO, List, Dict, Optional, Iterable @@ -643,6 +644,22 @@ def flatten_dictionary(nested_dict: Dict) -> Dict: return flat_dict +def recursive_update(dict1: Dict, dict2: Dict) -> Dict: + """Recursively update a nested dictionary with another dictionary, in place.""" + + for k, v in dict2.items(): + if isinstance(v, Mapping): + sub_dict = dict1.get(k, {}) + + if not isinstance(sub_dict, Mapping): + raise ValueError( + f"Expected Mapping for dict1[{k}], but found {type(sub_dict).__name__}.") + + dict1[k] = recursive_update(sub_dict, dict2.get(k)) + else: + dict1[k] = v + + return dict1 def auto_type_convert(value): """Try to convert 'value' to a int, float, or bool. Otherwise leave From 7350f753738db8e2e8e8127897f2bb69821d531d Mon Sep 17 00:00:00 2001 From: Hank Wikle Date: Thu, 4 Dec 2025 15:07:48 -0700 Subject: [PATCH 3/8] Add timeout for scheduler unit tests --- test/tests/sched_tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/tests/sched_tests.py b/test/tests/sched_tests.py index 4cd0d0217..f0ec44416 100644 --- a/test/tests/sched_tests.py +++ b/test/tests/sched_tests.py @@ -589,8 +589,8 @@ def test_task_based(self): test = self._quick_test(test_cfg, finalize=False) test2 = self._quick_test(test_cfg, finalize=False) dummy.schedule_tests(self.pav_cfg, [test, test2]) - test.wait() - test2.wait() + test.wait(timeout=10) + test2.wait(timeout=10) self.assertIn("tasks: 21", (test.path/'run.log').open().read()) self.assertIn("tasks: 21", (test.path/'run.log').open().read()) @@ -614,7 +614,7 @@ def test_wrapper(self): dummy = pavilion.schedulers.get_plugin('dummy') dummy.schedule_tests(self.pav_cfg, [test]) # Wait few seconds for the test to be scheduled to run. - test.wait() + test.wait(timeout=10) # Check if it actually echoed to log with (test.path/'run.log').open('r') as runlog: From dc99995a90c4d9303ef3c08a787ecbfc3722f2ec Mon Sep 17 00:00:00 2001 From: Hank Wikle Date: Thu, 4 Dec 2025 16:29:09 -0700 Subject: [PATCH 4/8] Miscellaneous changes --- lib/pavilion/commands/bisect.py | 5 +-- lib/pavilion/commands/run.py | 1 + lib/pavilion/resolver/resolver.py | 45 ++------------------------ lib/pavilion/series/series.py | 2 +- lib/pavilion/series/test_set.py | 2 +- lib/pavilion/series_config/__init__.py | 2 +- 6 files changed, 10 insertions(+), 47 deletions(-) diff --git a/lib/pavilion/commands/bisect.py b/lib/pavilion/commands/bisect.py index 597eb43fc..b03adb5a0 100644 --- a/lib/pavilion/commands/bisect.py +++ b/lib/pavilion/commands/bisect.py @@ -1,4 +1,5 @@ -from argparse import ArgumentParser +from argparse import ArgumentParser, Namespace +from typing import List from pavilion import output from pavilion.errors import TestSeriesError @@ -25,7 +26,7 @@ def _setup_arguments(self, parser: ArgumentParser) -> None: help="The name of the test to use to bisect the nodes.") parser.add_argument("nodes", type=str, default="", help="The set of nodes with which to start the search.") - parser.add_argument( + parser.add_argument( '-p', '--platform', action='store', help='The platform to configure this test for. If not ' 'specified, the current platform as denoted by the sys ' diff --git a/lib/pavilion/commands/run.py b/lib/pavilion/commands/run.py index 95a3f0064..40ee2b727 100644 --- a/lib/pavilion/commands/run.py +++ b/lib/pavilion/commands/run.py @@ -16,6 +16,7 @@ from pavilion.series_config import generate_series_config from pavilion.status_utils import print_from_tests from pavilion.test_ids import GroupID +from pavilion.resolver import TestConfigResolver from .base_classes import Command diff --git a/lib/pavilion/resolver/resolver.py b/lib/pavilion/resolver/resolver.py index b9f34fe34..82a2a0c25 100644 --- a/lib/pavilion/resolver/resolver.py +++ b/lib/pavilion/resolver/resolver.py @@ -623,7 +623,7 @@ def config_from_overrides(overrides: List[str]) -> TestConfig: for ovr in overrides: if '=' not in ovr: raise ValueError( - "Invalid override value. Must be in the form: " + f"Invalid override value {ovr}. Must be in the form: " "=. Ex. -c run.modules=['gcc'] ") key, value = ovr.split('=', 1) @@ -1137,8 +1137,9 @@ def apply_modes(self, return test_cfg + NOT_OVERRIDABLE = ['name', 'suite', 'suite_path', 'base_name', 'host', 'platform', 'modes'] - def apply_overrides(test_cfg: TestConfig, overrides: TestConfig) -> TestConfig: + def apply_overrides(self, test_cfg: TestConfig, overrides: TestConfig) -> TestConfig: """Apply the dictionary of overrides to the given test config.""" for key in overrides.keys(): @@ -1256,43 +1257,3 @@ def resolve_inheritance(self, suite_cfg, suite_path) \ del suite_tests['__base__'] return suite_tests - - - NOT_OVERRIDABLE = ['name', 'suite', 'suite_path', 'base_name', 'host', 'platform', 'modes'] - - def apply_overrides(self, - test_cfg: TestConfig, - overrides: TestConfig) -> TestConfig: - """Apply overrides to this test. - - :param dict test_cfg: The test configuration. - :param list overrides: A list of raw overrides in a.b.c=value form. - :raises: (ValueError,KeyError, TestConfigError) - """ - - config_loader = self._loader - - for ovr in overrides: - if '=' not in ovr: - raise ValueError( - "Invalid override value. Must be in the form: " - "=. Ex. -c run.modules=['gcc'] ") - - key, value = ovr.split('=', 1) - key = key.strip() - if not key: - raise ValueError("Override '{}' given a blank key.".format(ovr)) - - key = key.split('.') - for part in key: - if ' ' in part: - raise ValueError("Override '{}' has whitespace in its key.".format(ovr)) - if not part: - raise ValueError("Override '{}' has an empty key part.".format(ovr)) - - self._apply_override(test_cfg, key, value) - - try: - return config_loader.normalize(test_cfg, root_name='overrides') - except TypeError as err: - raise TestConfigError("Invalid override", prior_error=err) \ No newline at end of file diff --git a/lib/pavilion/series/series.py b/lib/pavilion/series/series.py index e4a9b5719..9ddaa88b2 100644 --- a/lib/pavilion/series/series.py +++ b/lib/pavilion/series/series.py @@ -16,7 +16,7 @@ from pathlib import Path from operator import attrgetter from itertools import product -from typing import List, Dict, Set, Union, TextIO, Iterator, Optional +from typing import List, Dict, Set, Union, TextIO, Iterator, Optional, Any import pavilion from pavilion.config import PavConfig diff --git a/lib/pavilion/series/test_set.py b/lib/pavilion/series/test_set.py index cebe83767..0ed4f02dc 100644 --- a/lib/pavilion/series/test_set.py +++ b/lib/pavilion/series/test_set.py @@ -8,7 +8,7 @@ import math from collections import defaultdict from io import StringIO -from typing import List, Dict, TextIO, Union, Set, Iterator, Tuple, Optional +from typing import List, Dict, TextIO, Union, Set, Iterator, Tuple, Optional, Any import pavilion.errors from pavilion.config import PavConfig diff --git a/lib/pavilion/series_config/__init__.py b/lib/pavilion/series_config/__init__.py index 3526ed90c..fd5dcc0c0 100644 --- a/lib/pavilion/series_config/__init__.py +++ b/lib/pavilion/series_config/__init__.py @@ -1,5 +1,5 @@ import os -from typing import List, Dict, Optional +from typing import List, Dict, Optional, Any import yc_yaml import yaml_config From 68241fa4c49d8e470edaa716d20baaad6405e97a Mon Sep 17 00:00:00 2001 From: Hank Wikle Date: Fri, 5 Dec 2025 14:24:30 -0700 Subject: [PATCH 5/8] Overhaul auxiliary config resolution --- lib/pavilion/resolver/resolver.py | 423 ++++++++++++++++-------------- 1 file changed, 224 insertions(+), 199 deletions(-) diff --git a/lib/pavilion/resolver/resolver.py b/lib/pavilion/resolver/resolver.py index 82a2a0c25..8202d159f 100644 --- a/lib/pavilion/resolver/resolver.py +++ b/lib/pavilion/resolver/resolver.py @@ -38,7 +38,7 @@ KEY_NAME_RE) from pavilion.test_config.file_format import TestConfigLoader, TestSuiteLoader from pavilion.utils import union_dictionary, recursive_update -from pavilion.micro import first, listmap +from pavilion.micro import first, listmap, listfilter from pavilion.path_utils import append_to_path, exists from yaml_config import RequiredError, YamlConfigLoader @@ -58,7 +58,7 @@ class ConfigInfo: - def __init__(self, name: str, type: str, path: Path, label: str = None, + def __init__(self, name: str, type: str, path: Path, label: Optional[str] = None, from_suite: bool = False): self.name = name @@ -71,7 +71,12 @@ def __init__(self, name: str, type: str, path: Path, label: str = None, class TestOptions: """Test options from the command line or series configs.""" - def __init__(self, modes: List[str], overrides: TestConfig, conditions: Dict): + def __init__(self + platform: str, + host: str, + modes: Optional[List[str]], + overrides: Optional[TestConfig], + conditions: Optional[Dict]): self.modes = modes or [] self.overrides = overrides or {} self.conditions = conditions or {} @@ -81,6 +86,8 @@ class TestConfigResolver: """Converts raw test configurations into their final, fully resolved form.""" + CONFIG_TYPES = ("platform", "host", "mode") + def __init__(self, pav_cfg: PavConfig, platform: Optional[str] = None, @@ -146,8 +153,8 @@ def _get_config_dirname(cfg_type: str, use_suites_dir: bool = False) -> str: @staticmethod def _get_config_fname(cfg_type: str) -> str: - """Given a config type, returns the name of the file in the - suites directory corresponding to that type.""" + """Given a config type, returns the name of the file in the suite directory corresponding to + that type.""" fname = cfg_type.lower() @@ -171,47 +178,74 @@ def config_labels(self) -> Iterator[str]: """Return an iterator over all config labels.""" return self.pav_cfg.configs.keys() - def _get_test_config_path(self, cfg_name: str, cfg_type: str) -> Tuple[str, Optional[Path]]: + def _get_test_config_path(self, + cfg_name: str, + cfg_type: str) -> Tuple[Optional[str], Optional[Path]]: """Given a config name and type, find the path to that config, if it exists, excluding configs in the suites directory. If no such config exists, return None.""" cfg_dir = self._get_config_dirname(cfg_type) paths = map(append_to_path(f"{cfg_dir}/{cfg_name}.yaml"), self.config_paths) + pairs = zip(self.config_labels, paths) + paris = listfilter(lambda p: p[1].exists(), pairs) + + if len(pairs) > 1: + raise TestConfigError(f"Could not unambiguously find config with name {cfg_name}: " + f"Found {len(pairs)} in the following locations: " + f"{listmap(lambda p: p[1], pairs)}.") + elif len(pairs) == 0: + return None, None + else: + return pairs[0] - res = first(lambda x: x[1].exists(), pairs) + def _get_suite_path(self, suite_name: str) -> Tuple[Optional[str], Optional[Path]]: + """Get the path to the suite with the given name, if it exists, along with its corresponding + config label. Returns None if no suite is found.""" - if res is None: - return '', None + suite_paths = listmap(append_to_path(f"{suite_name}.yaml"), self.suites_dirs) + suite_paths.extend(map(append_to_path(f"{suite_name}"), self.suites_dirs)) - return res + pairs = zip(suite_paths, list(self.config_labels) * 2) + pairs = listfilter(lambda p: p[1].exists(), pairs) - def _config_path_from_suite(self, suite_name: str, - conf_type: str) -> Tuple[str, Optional[Path]]: - """Given a suite name, return the path to the config file of the specified - type, if one exists. If the file does not exist in any known suites directory, - returns None.""" + if len(pairs) > 1: + raise TestConfigError(f"Could not unambiguously find suite with name {suite_name}: " + f"Found {len(pairs)} in the following locations: " + f"{listmap(lambda p: p[1], pairs)}.") + elif len(pairs) == 0: + return None, None + else: + return suite_paths[0] - paths = [] - labels = list(self.config_labels) + def _config_path_from_suite(self, suite_name: Optional[str], + conf_type: str) -> Tuple[Optional[str], Optional[Path]]: + """Given a suite name, return the path to the config file of the specified type, if one + exists, along with its corresponding config label. If the file does not exist in any known + suites directory, returns None.""" - cfg_fname = self._get_config_fname(conf_type) + if suite_name is None: + return None, None - if conf_type == "suite": - paths.extend(listmap(append_to_path(f"{suite_name}.yaml"), self.suites_dirs)) - labels *= 2 + label, suite_path = self._get_suite_path(suite_name) - paths.extend(listmap(append_to_path(f"{suite_name}/{cfg_fname}"), self.suites_dirs)) + if suite_path is None: + return None, None - pairs = zip(labels, paths) + cfg_fname = self._get_config_fname(conf_type) - res = first(lambda x: x[1].exists(), pairs) + if suite_path.is_dir(): + cfg_path = suite_path / cfg_fname + elif cfg_type == "suite": + cfg_path = suite_path + else: + return None, None - if res is None: - return '', None + if cfg_path.exists(): + return label, cfg_path - return res + return None, None def find_config(self, cfg_type: str, cfg_name: str, suite_name: str = None) -> ConfigInfo: """Search all of the known configuration directories for a config of the @@ -689,35 +723,18 @@ def _safe_load_config(cfg: ConfigInfo, loader: yc.YamlConfigLoader) -> TestConfi return raw_cfg - def _load_raw_config(self, cfg_info: ConfigInfo, loader: yc.YamlConfigLoader, - optional: bool = False) -> Optional[TestConfig]: + def _load_raw_config(self, + cfg_info: ConfigInfo, + loader: yc.YamlConfigLoader) -> TestConfig: """Given a path to a config file and a loader, attempt to load the config, handle errors appropriately.""" - if cfg_info.path is None and optional: - return None - - if cfg_info.path is None and not optional: - similar = self.find_similar_configs(cfg_info.type, cfg_info.name) - - if similar: - raise TestConfigError( - "Could not find {} config {}.yaml.\n" - "Did you mean one of these? {}" - .format(cfg_info.type, cfg_info.name, ', '.join(similar))) - else: - raise TestConfigError( - "Could not find {0} config file '{1}.yaml' in any of the " - "Pavilion config directories.\n" - "Run `pav show {2}` to get a list of available {0} files." - .format(cfg_info.type, cfg_info.name, cfg_info.type)) - raw_cfg = self._safe_load_config(cfg_info, loader) if cfg_info.from_suite and cfg_info.type != "suite": raw_cfg = raw_cfg.get(cfg_info.name) - if raw_cfg is None and not optional: + if raw_cfg is None: raise TestConfigError( f"Could not find {cfg_info.type} config with name {cfg_info.name}" f" in file {cfg_info.path}.") @@ -794,9 +811,163 @@ def _load_raw_configs(self, request: TestRequest, options: TestOptions) -> List[ return test_configs + def apply_aux_configs(self, test_cfg: TestConfig, options: TestOptions) -> TestConfig: + """Apply the sequence of auxiliary configs to the test config.""" + + suite_name = test_cfg.get("suite") + + aux_configs = self._load_aux_configs(options, suite_name) + + for cfg_info, cfg in aux_cfgs: + try: + test_cfg = self._loader.merge(test_cfg, cfg) + except (KeyError, ValueError) as err: + if cfg_info.type == "overrides": + msg = "Error merging overrides configuration." + else: + msg = f"Error merging {cfg_info.type} configuration for {cfg_info.type} " + "'{cfg_info.name}'" + raise TestConfigError(msg) + + if cfg_info.type == "mode": + test_cfg = resolve.cmd_inheritance(test_cfg) + + return test_cfg + + def _load_aux_configs(self, + options: TestOptions, + suite_name: Optional[str] = None) -> Tuple[str, TestConfig]: + """Load platform, host, and mode configs, and construct the override configs, + returning them in the order in which they will be applied.""" + + configs = [] + + aux_paths = self._get_aux_config_paths(options, suite_name) + + for cfg_info in aux_paths: + if cfg_info.from_suite: + loader = self._suite_loader + else: + loader = self._loader + + raw_cfg = self._load_raw_config(cfg_info, loader) + + try: + cfg = self._loader.normalize( + raw_cfg, + root_name=f"the top level of the {cfg_info.type} file.") + except (KeyError, ValueError) as err: + raise TestConfigError( + f"Error loading {cfg_info.type}} config '{cfg_info.name}' from file " + f"'{cfg_info.path}'.") + + configs.append((cfg_info, cfg)) + + if overrides is not None: + overrides = self._validate_overrides(options.overrides) + overrides = self._loader.normalize(overrides) + + cfg_info = ConfigInfo( + name=None, + type="overrides", + label=None, + path=None, + from_suite=False) + + configs.append((cfg_info, overrides)) + + return configs + + NOT_OVERRIDABLE = ['name', 'suite', 'suite_path', 'base_name', 'host', 'platform', 'modes'] + + def _validate_overrides(self, overrides: TestConfig) -> TestConfig: + """Validate and normalize the overrides config.""" + + for key in overrides.keys(): + if key in self.NOT_OVERRIDABLE: + raise KeyError("You can't override the '{}' key in a test config".format(key)) + + return self._loader.normalize(overrides, root_name=f"the top level of the overrides config") + + def _get_aux_config_paths(self, + options: TestOptions, + suite_name: Optional[str] = None) -> List[ConfigInfo]: + """Get a list of auxiliary config paths in the order in which they will be applied.""" + + cfg_names = {"platform": options.platform, "host": options.host} + + paths = [] + + for cfg_type in ("platform", "host"): + # If a config exists in both the suite directory and the config-type specific + # directory, we'll just stack them, with the config from the suite directory taking + # higher precedence. + label, path = self._get_test_config_path(cfg_names.get(cfg_type), cfg_type) + + if path is not None: + paths.append(ConfigInfo( + name=cfg_names.get(cfg_type), + type=cfg_type, + label=label, + path=path, + from_suite=False)) + + label, path = self._config_path_from_suite(suite_name, cfg_type) + + if path is not None: + paths.append(ConfigInfo( + name=cfg_names.get(cfg_type), + type=cfg_type, + label=label, + path=path, + from_suite=True)) + + for mode in options.modes: + global_mode_label, global_mode_path = self._get_test_config_path(mode, "mode") + suite_mode_label, suite_mode_path = self._config_path_from_suite(mode, "mode") + + if suite_mode_path is not None: + if global_mode_path is not None: + raise TestConfigError(f"Found multiple mode files with name {mode} in the " + "following locations: " + f"{[global_mode_path, suite_mode_path]}") + else: + path = suite_mode_path + label = suite_mode_label + from_suite = True + else: + if global_mode_path is None: + similar = self.find_similar_configs("mode", mode) + + if len(similar) > 0: + raise TestConfigError( + "Could not find {} config {}.yaml.\n" + "Did you mean one of these? {}" + .format(cfg_info.type, cfg_info.name, ', '.join(similar))) + else: + raise TestConfigError( + "Could not find {0} config file '{1}.yaml' in any of the " + "Pavilion config directories.\n" + "Run `pav show {2}` to get a list of available {0} files." + .format(cfg_info.type, cfg_info.name, cfg_info.type)) + else: + path = global_mode_path + label = global_mode_label + from_suite = False + + paths.append(ConfigInfo( + name=mode, + type="mode", + label=label, + path=path, + from_suite=from_suite)) - def _apply_test_options(self, raw_test: Dict, options: TestOptions, request: TestRequest) \ - -> Optional[Dict]: + return paths + + def _apply_test_options(self, + raw_test: TestConfig, + options: TestOptions, + request: TestRequest) -> Optional[Dict]: test_cfg = copy.deepcopy(raw_test) @@ -814,9 +985,7 @@ def _apply_test_options(self, raw_test: Dict, options: TestOptions, request: Tes # Apply downstream configs. try: - test_cfg = self.apply_platform(test_cfg, self._platform, suite_name) - test_cfg = self.apply_host(test_cfg, self._host, suite_name) - test_cfg = self.apply_modes(test_cfg, options.modes, suite_name) + test_cfg = self.apply_aux_configs(test_cfg, options) except TestConfigError as err: err.request = request self.errors.append(err) @@ -825,21 +994,6 @@ def _apply_test_options(self, raw_test: Dict, options: TestOptions, request: Tes # Save the overrides as part of the test config test_cfg['overrides'] = options.overrides - # Apply overrides - if options.overrides: - try: - test_cfg = self.apply_overrides(test_cfg, options.overrides) - except TestConfigError as err: - err.request = request - self.errors.append(err) - return None - except (KeyError, ValueError) as err: - self.errors.append(TestConfigError( - 'Error applying overrides to test {} from suite {} at:\n{}' \ - .format(test_cfg['name'], test_cfg['suite'], test_cfg['suite_path']), - request, err)) - return None - # Result evaluations can be added to all tests at the root pavilion config level. result_evals = test_cfg['result_evaluate'] for key, const in self.pav_cfg.default_results.items(): @@ -894,9 +1048,9 @@ def _load_base_config(self, platform: str, host: str) -> TestConfig: # Get the base, empty config, then apply the host config on top of it. base_config = self._loader.load_empty() - base_config = self.apply_platform(base_config, platform) + options = TestOptions(platform=platform, host=host) - return self.apply_host(base_config, host) + return self.apply_aux_configs(base_config, options) def _load_suite_tests(self, request: TestRequest) -> Dict[str, Dict]: """Load the suite config, with standard info applied to """ @@ -1025,135 +1179,6 @@ def check_version_compatibility(self, test_cfg): "Incompatible with pavilion version '{}', compatible versions " "'{}'.".format(PavVars()['version'], comp_versions)) - def apply_host(self, test_cfg: TestConfig, hostname: str, suite_name: str = None) -> TestConfig: - """Apply the host configuration to the given config.""" - - if suite_name is not None: - from_suite = True - label, host_cfg_path = self._config_path_from_suite(suite_name, "host") - loader = self._suite_loader - else: - from_suite = False - label, host_cfg_path = self._get_test_config_path(hostname, "host") - loader = self._loader - - cfg_info = ConfigInfo(hostname, "host", host_cfg_path, label, from_suite) - - raw_host_cfg = self._load_raw_config(cfg_info, loader, optional=True) - - if raw_host_cfg is None: - return test_cfg - - try: - host_cfg = self._loader.normalize( - raw_host_cfg, - root_name=f"the top level of the host file.") - except (KeyError, ValueError) as err: - raise TestConfigError( - f"Error loading host config '{hostname}' from file '{host_cfg_path}'.") - - try: - return self._loader.merge(test_cfg, host_cfg) - except (KeyError, ValueError) as err: - raise TestConfigError( - "Error merging host configuration for host '{}'".format(hostname)) - - def apply_platform(self, test_cfg: TestConfig, platform: str, - suite_name: str = None) -> TestConfig: - """Apply the platform configuration to the given config.""" - - if suite_name is not None: - from_suite = True - label, platform_cfg_path = self._config_path_from_suite(suite_name, "platform") - loader = self._suite_loader - else: - from_suite = False - label, platform_cfg_path = self._get_test_config_path(platform, "platform") - loader = self._loader - - cfg_info = ConfigInfo(platform, "platform", platform_cfg_path, label, from_suite) - - raw_platform_cfg = self._load_raw_config(cfg_info, loader, optional=True) - - if raw_platform_cfg is None: - return test_cfg - - try: - platform_cfg = self._loader.normalize( - raw_platform_cfg, - root_name=f"the top level of the platform file.") - except (KeyError, ValueError) as err: - raise TestConfigError( - f"Error loading host config '{platform}' from file '{platform_cfg_path}'") - - try: - return self._loader.merge(test_cfg, platform_cfg) - except (KeyError, ValueError) as err: - raise TestConfigError( - "Error merging configuration for platform '{}'".format(platform)) - - def apply_modes(self, - test_cfg: TestConfig, - modes: List[str], - suite_name: Optional[str] = None) -> TestConfig: - """Apply each of the mode files to the given test config. - - :param test_cfg: The raw test configuration. - :param modes: A list of mode names. - """ - - for mode in modes: - mode_cfg_path = None - - if suite_name is not None: - label, mode_cfg_path = self._config_path_from_suite(suite_name, "mode") - if mode_cfg_path is None: - from_suite = False - label, mode_cfg_path = self._get_test_config_path(mode, "mode") - loader = self._loader - else: - from_suite = True - loader = self._suite_loader - - cfg_info = ConfigInfo(mode, "mode", mode_cfg_path, label, from_suite) - - raw_mode_cfg = self._load_raw_config(cfg_info, loader) - - try: - mode_cfg = self._loader.normalize( - raw_mode_cfg, - root_name=f"the top level of the OS file.") - except (KeyError, ValueError) as err: - raise TestConfigError( - f"Error loading host config '{mode}' from file '{mode_cfg_path}'.") - - try: - test_cfg = self._loader.merge(test_cfg, mode_cfg) - except (KeyError, ValueError) as err: - raise TestConfigError( - "Error merging mode configuration for mode '{}'".format(mode)) - - test_cfg = resolve.cmd_inheritance(test_cfg) - - return test_cfg - - NOT_OVERRIDABLE = ['name', 'suite', 'suite_path', 'base_name', 'host', 'platform', 'modes'] - - def apply_overrides(self, test_cfg: TestConfig, overrides: TestConfig) -> TestConfig: - """Apply the dictionary of overrides to the given test config.""" - - for key in overrides.keys(): - if key in self.NOT_OVERRIDABLE: - raise KeyError("You can't override the '{}' key in a test config" - .format(key)) - - try: - return self._loader.merge(test_cfg, overrides) - except (KeyError, ValueError) as err: - raise TestConfigError( - "Error applying overrides to test config.\n{err}") - - def resolve_inheritance(self, suite_cfg, suite_path) \ -> Dict[str, dict]: """Resolve inheritance between tests in a test suite. There's potential From db79073547c68b449264071f565a07d5e3256ceb Mon Sep 17 00:00:00 2001 From: Hank Wikle Date: Fri, 5 Dec 2025 14:34:57 -0700 Subject: [PATCH 6/8] Fix some things --- lib/pavilion/resolver/resolver.py | 40 ++++++++++++++++--------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/lib/pavilion/resolver/resolver.py b/lib/pavilion/resolver/resolver.py index 8202d159f..b9e10725c 100644 --- a/lib/pavilion/resolver/resolver.py +++ b/lib/pavilion/resolver/resolver.py @@ -71,12 +71,14 @@ def __init__(self, name: str, type: str, path: Path, label: Optional[str] = None class TestOptions: """Test options from the command line or series configs.""" - def __init__(self + def __init__(self, platform: str, host: str, - modes: Optional[List[str]], - overrides: Optional[TestConfig], - conditions: Optional[Dict]): + modes: Optional[List[str]] = None, + overrides: Optional[TestConfig] = None, + conditions: Optional[Dict] = None): + self.platform = platform + self.host = host self.modes = modes or [] self.overrides = overrides or {} self.conditions = conditions or {} @@ -189,7 +191,7 @@ def _get_test_config_path(self, paths = map(append_to_path(f"{cfg_dir}/{cfg_name}.yaml"), self.config_paths) pairs = zip(self.config_labels, paths) - paris = listfilter(lambda p: p[1].exists(), pairs) + pairs = listfilter(lambda p: p[1].exists(), pairs) if len(pairs) > 1: raise TestConfigError(f"Could not unambiguously find config with name {cfg_name}: " @@ -207,7 +209,7 @@ def _get_suite_path(self, suite_name: str) -> Tuple[Optional[str], Optional[Path suite_paths = listmap(append_to_path(f"{suite_name}.yaml"), self.suites_dirs) suite_paths.extend(map(append_to_path(f"{suite_name}"), self.suites_dirs)) - pairs = zip(suite_paths, list(self.config_labels) * 2) + pairs = zip(list(self.config_labels) * 2, suite_paths) pairs = listfilter(lambda p: p[1].exists(), pairs) if len(pairs) > 1: @@ -217,10 +219,10 @@ def _get_suite_path(self, suite_name: str) -> Tuple[Optional[str], Optional[Path elif len(pairs) == 0: return None, None else: - return suite_paths[0] + return pairs[0] def _config_path_from_suite(self, suite_name: Optional[str], - conf_type: str) -> Tuple[Optional[str], Optional[Path]]: + cfg_type: str) -> Tuple[Optional[str], Optional[Path]]: """Given a suite name, return the path to the config file of the specified type, if one exists, along with its corresponding config label. If the file does not exist in any known suites directory, returns None.""" @@ -233,7 +235,7 @@ def _config_path_from_suite(self, suite_name: Optional[str], if suite_path is None: return None, None - cfg_fname = self._get_config_fname(conf_type) + cfg_fname = self._get_config_fname(cfg_type) if suite_path.is_dir(): cfg_path = suite_path / cfg_fname @@ -816,7 +818,7 @@ def apply_aux_configs(self, test_cfg: TestConfig, options: TestOptions) -> TestC suite_name = test_cfg.get("suite") - aux_configs = self._load_aux_configs(options, suite_name) + aux_cfgs = self._load_aux_configs(options, suite_name) for cfg_info, cfg in aux_cfgs: try: @@ -825,8 +827,8 @@ def apply_aux_configs(self, test_cfg: TestConfig, options: TestOptions) -> TestC if cfg_info.type == "overrides": msg = "Error merging overrides configuration." else: - msg = f"Error merging {cfg_info.type} configuration for {cfg_info.type} " - "'{cfg_info.name}'" + msg = (f"Error merging {cfg_info.type} configuration for {cfg_info.type} " + "'{cfg_info.name}'") raise TestConfigError(msg) if cfg_info.type == "mode": @@ -858,12 +860,12 @@ def _load_aux_configs(self, root_name=f"the top level of the {cfg_info.type} file.") except (KeyError, ValueError) as err: raise TestConfigError( - f"Error loading {cfg_info.type}} config '{cfg_info.name}' from file " + f"Error loading {cfg_info.type} config '{cfg_info.name}' from file " f"'{cfg_info.path}'.") configs.append((cfg_info, cfg)) - if overrides is not None: + if options.overrides is not None: overrides = self._validate_overrides(options.overrides) overrides = self._loader.normalize(overrides) @@ -941,15 +943,15 @@ def _get_aux_config_paths(self, if len(similar) > 0: raise TestConfigError( - "Could not find {} config {}.yaml.\n" + "Could not find mode config {}.yaml.\n" "Did you mean one of these? {}" - .format(cfg_info.type, cfg_info.name, ', '.join(similar))) + .format(mode, ', '.join(similar))) else: raise TestConfigError( - "Could not find {0} config file '{1}.yaml' in any of the " + "Could not find mode config file '{}.yaml' in any of the " "Pavilion config directories.\n" - "Run `pav show {2}` to get a list of available {0} files." - .format(cfg_info.type, cfg_info.name, cfg_info.type)) + "Run `pav show mode` to get a list of available mode files." + .format(mode)) else: path = global_mode_path label = global_mode_label From bc9db9b839519953102880a3ab2e581340dc894f Mon Sep 17 00:00:00 2001 From: Hank Wikle Date: Fri, 5 Dec 2025 14:41:17 -0700 Subject: [PATCH 7/8] Fix test options --- lib/pavilion/resolver/resolver.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/pavilion/resolver/resolver.py b/lib/pavilion/resolver/resolver.py index b9e10725c..c7c807497 100644 --- a/lib/pavilion/resolver/resolver.py +++ b/lib/pavilion/resolver/resolver.py @@ -451,7 +451,11 @@ def load_iter(self, batch_size = 2**32 if batch_size is None else batch_size - options = TestOptions(modes, overrides, conditions) + options = TestOptions(platform=self._platform, + host=self._host, + modes=modes, + overrides=overrides, + conditions=conditions) requests = [TestRequest(req) for req in tests] From 31d5c343cea9df9176aa79cc6a687ae366caad67 Mon Sep 17 00:00:00 2001 From: Hank Wikle Date: Fri, 5 Dec 2025 16:56:41 -0700 Subject: [PATCH 8/8] Progress towards better overrides --- lib/pavilion/commands/series.py | 4 +++- lib/pavilion/resolver/resolver.py | 14 +------------- lib/pavilion/series_config/__init__.py | 12 +++++++----- lib/pavilion/series_config/file_format.py | 22 +++++++++++++++------- 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/lib/pavilion/commands/series.py b/lib/pavilion/commands/series.py index fe58b9fdb..31eb494c8 100644 --- a/lib/pavilion/commands/series.py +++ b/lib/pavilion/commands/series.py @@ -20,6 +20,7 @@ from pavilion.test_ids import SeriesID, resolve_mixed_ids from pavilion.errors import TestSeriesError, TestSeriesWarning from pavilion.config import PavConfig +from pavilion.resolver import TestConfigResolver from .base_classes import Command, sub_cmd @@ -189,13 +190,14 @@ def _run_cmd(self, pav_cfg, args): else: # load series and test files try: + overrides = TestConfigResolver.config_from_overrides(args.overrides) # Pre-verify that all the series, tests, platform, modes, and hosts exist. series_cfg = series_config.verify_configs(pav_cfg, args.series_name, platform=args.platform, host=args.host, modes=args.modes, - overrides=args.overrides) + overrides=overrides) except series_config.SeriesConfigError as err: output.fprint(self.errfile, err.pformat(), color=output.RED) return errno.EINVAL diff --git a/lib/pavilion/resolver/resolver.py b/lib/pavilion/resolver/resolver.py index c7c807497..af60ce088 100644 --- a/lib/pavilion/resolver/resolver.py +++ b/lib/pavilion/resolver/resolver.py @@ -870,8 +870,7 @@ def _load_aux_configs(self, configs.append((cfg_info, cfg)) if options.overrides is not None: - overrides = self._validate_overrides(options.overrides) - overrides = self._loader.normalize(overrides) + overrides = self._loader.normalize(options.overrides) cfg_info = ConfigInfo( name=None, @@ -884,17 +883,6 @@ def _load_aux_configs(self, return configs - NOT_OVERRIDABLE = ['name', 'suite', 'suite_path', 'base_name', 'host', 'platform', 'modes'] - - def _validate_overrides(self, overrides: TestConfig) -> TestConfig: - """Validate and normalize the overrides config.""" - - for key in overrides.keys(): - if key in self.NOT_OVERRIDABLE: - raise KeyError("You can't override the '{}' key in a test config".format(key)) - - return self._loader.normalize(overrides, root_name=f"the top level of the overrides config") - def _get_aux_config_paths(self, options: TestOptions, suite_name: Optional[str] = None) -> List[ConfigInfo]: diff --git a/lib/pavilion/series_config/__init__.py b/lib/pavilion/series_config/__init__.py index fd5dcc0c0..762c7fd5d 100644 --- a/lib/pavilion/series_config/__init__.py +++ b/lib/pavilion/series_config/__init__.py @@ -3,7 +3,9 @@ import yc_yaml import yaml_config +from pavilion.config import PavConfig from pavilion.resolver import TestConfigResolver +from pavilion.utils import recursive_update from ..errors import TestConfigError, SeriesConfigError from .file_format import SeriesConfigLoader @@ -99,14 +101,14 @@ def load_series_config(pav_cfg, series_name: str) -> dict: prior_error=err) -def verify_configs(pav_cfg, series_name: str, platform: str = None, - host: str = None, modes: List[str] = None, - overrides: List[str] = None) -> dict: +def verify_configs(pav_cfg: PavConfig, series_name: str, platform: Optional[str] = None, + host: Optional[str] = None, modes: Optional[List[str]] = None, + overrides: Optional[Dict[str, Any]] = None) -> Dict: """Loads series config and checks that all tests can be loaded with all modes and host (if any). """ modes = modes or [] - overrides = overrides or [] + overrides = overrides or {} series_cfg = load_series_config(pav_cfg, series_name) resolver = TestConfigResolver(pav_cfg, host=host) @@ -115,7 +117,7 @@ def verify_configs(pav_cfg, series_name: str, platform: str = None, series_cfg['name'] = series_name series_cfg['modes'] += modes - series_cfg['overrides'] += overrides + recursive_update(series_cfg['overrides'], overrides) try: for set_name, set_dict in series_cfg['test_sets'].items(): diff --git a/lib/pavilion/series_config/file_format.py b/lib/pavilion/series_config/file_format.py index 20a8360f8..d92ac8b65 100644 --- a/lib/pavilion/series_config/file_format.py +++ b/lib/pavilion/series_config/file_format.py @@ -3,13 +3,20 @@ import yaml_config as yc from pavilion.config import make_invalidator -from pavilion.test_config.file_format import \ - CondCategoryElem, EnvCatElem, TestCatElem +from pavilion.test_config import TestConfigLoader +from pavilion.test_config.file_format import CondCategoryElem, EnvCatElem, TestCatElem + + +NOT_OVERRIDABLE = ['name', 'suite', 'suite_path', 'base_name', 'host', 'platform', 'modes', + 'overrides'] class SeriesConfigLoader(yc.YamlConfigLoader): """This class describes a series file.""" + OVERRIDE_ELEMS = [elem for elem in TestConfigLoader.ELEMENTS \ + if elem.name not in NOT_OVERRIDABLE] + ELEMENTS = [ TestCatElem( 'test_sets', sub_elem=yc.KeyedElem( @@ -18,6 +25,7 @@ class SeriesConfigLoader(yc.YamlConfigLoader): yc.BoolElem('depends_pass', default=False), yc.ListElem('depends_on', sub_elem=yc.StrElem()), yc.ListElem('modes', sub_elem=yc.StrElem()), + yc.KeyedElem('overrides', elements=OVERRIDE_ELEMS), yc.IntElem('simultaneous', default=None), CondCategoryElem( 'only_if', sub_elem=yc.ListElem(sub_elem=yc.StrElem()), @@ -48,8 +56,8 @@ class SeriesConfigLoader(yc.YamlConfigLoader): help_text="The host this series will be run on. This is not " "configured, but dynamically added to the config." ), - yc.ListElem( - 'overrides', sub_elem=yc.StrElem(), hidden=True, + yc.KeyedElem( + 'overrides', elements=OVERRIDE_ELEMS, hidden=True, help_text="Command line overrides to apply to this series. This is only " "used when ad-hoc series are created from the command line." ), @@ -57,8 +65,8 @@ class SeriesConfigLoader(yc.YamlConfigLoader): 'modes', sub_elem=yc.StrElem(), help_text="Modes to run all tests in this series under." ), - yc.ListElem( - 'overrides', sub_elem=yc.StrElem(), + yc.KeyedElem( + 'overrides', elements=OVERRIDE_ELEMS, help_text="Overrides to apply to all tests in this series." ), yc.IntElem( @@ -91,4 +99,4 @@ class SeriesConfigLoader(yc.YamlConfigLoader): help_text="The series config option 'restart' has been replaced with 'repeat'." ), ] - """Describes elements in Series Config Loader.""" + """Describes elements in Series Config Loader.""" \ No newline at end of file