diff --git a/tests/run/max/data/.fmf/version b/tests/run/max/data/.fmf/version new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/run/max/data/.fmf/version @@ -0,0 +1 @@ +1 diff --git a/tests/run/max/data/main.fmf b/tests/run/max/data/main.fmf new file mode 100644 index 0000000000..eec2d409d2 --- /dev/null +++ b/tests/run/max/data/main.fmf @@ -0,0 +1,18 @@ +/plan: + discover: + how: shell + tests: + - name: Test 1 + test: echo "1" + - name: Test 2 + test: echo "2" + - name: Test 3 + test: echo "3" + - name: Test 4 + test: echo "4" + - name: Test 5 + test: echo "5" + provision: + how: local + execute: + how: tmt diff --git a/tests/run/max/main.fmf b/tests/run/max/main.fmf new file mode 100644 index 0000000000..c294cb4e63 --- /dev/null +++ b/tests/run/max/main.fmf @@ -0,0 +1 @@ +summary: Verify --max option splits plans into smaller plans diff --git a/tests/run/max/test.sh b/tests/run/max/test.sh new file mode 100755 index 0000000000..ec34e2f881 --- /dev/null +++ b/tests/run/max/test.sh @@ -0,0 +1,23 @@ +#!/bin/bash +. /usr/share/beakerlib/beakerlib.sh || exit 1 + +rlJournalStart + rlPhaseStartSetup + rlRun "run=\$(mktemp -d)" 0 "Create run directory" + rlRun "pushd data" + rlPhaseEnd + + rlPhaseStartTest + rlRun -s "tmt run -vv --id $run --max 3" + rlAssertGrep "Splitting plan to batches of 3 tests." $rlRun_LOG + rlAssertGrep "3 tests selected" $rlRun_LOG + rlAssertGrep "summary: 3 tests passed" $rlRun_LOG + rlAssertGrep "2 tests selected" $rlRun_LOG + rlAssertGrep "summary: 2 tests passed" $rlRun_LOG + rlPhaseEnd + + rlPhaseStartCleanup + rlRun "popd" + rlRun "rm -r $run" 0 "Remove run directory" + rlPhaseEnd +rlJournalEnd diff --git a/tmt/base.py b/tmt/base.py index 21999409e2..556b241596 100644 --- a/tmt/base.py +++ b/tmt/base.py @@ -43,6 +43,7 @@ import tmt.lint import tmt.log import tmt.plugins +import tmt.plugins.plan_shapers import tmt.result import tmt.steps import tmt.steps.discover @@ -78,6 +79,7 @@ if TYPE_CHECKING: import tmt.cli + import tmt.steps.discover import tmt.steps.provision.local @@ -713,9 +715,10 @@ def __init__(self, tree: Optional['Tree'] = None, parent: Optional[tmt.utils.Common] = None, logger: tmt.log.Logger, + name: Optional[str] = None, **kwargs: Any) -> None: """ Initialize the node """ - super().__init__(node=node, logger=logger, parent=parent, name=node.name, **kwargs) + super().__init__(node=node, logger=logger, parent=parent, name=name or node.name, **kwargs) self.node = node self.tree = tree @@ -1670,11 +1673,17 @@ class Plan( # Optional Login instance attached to the plan for easy login in tmt try login: Optional[tmt.steps.Login] = None - # When fetching remote plans we store links between the original - # plan with the fmf id and the imported plan with the content. - _imported_plan: Optional['Plan'] = field(default=None, internal=True) + # When fetching remote plans or splitting plans, we store links + # between the original plan with the fmf id and the imported or + # derived plans with the content. _original_plan: Optional['Plan'] = field(default=None, internal=True) - _remote_plan_fmf_id: Optional[FmfId] = field(default=None, internal=True) + _original_plan_fmf_id: Optional[FmfId] = field(default=None, internal=True) + + _imported_plan_fmf_id: Optional[FmfId] = field(default=None, internal=True) + _imported_plan: Optional['Plan'] = field(default=None, internal=True) + + _derived_plans: list['Plan'] = field(default_factory=list, internal=True) + derived_id: Optional[int] = field(default=None, internal=True) #: Used by steps to mark invocations that have been already applied to #: this plan's phases. Needed to avoid the second evaluation in @@ -1716,11 +1725,12 @@ def __init__( # set, incorrect default value is generated, and the field ends up being # set to `None`. See https://github.com/teemtee/tmt/issues/2630. self._applied_cli_invocations = [] + self._derived_plans = [] # Check for possible remote plan reference first reference = self.node.get(['plan', 'import']) if reference is not None: - self._remote_plan_fmf_id = FmfId.from_spec(reference) + self._imported_plan_fmf_id = FmfId.from_spec(reference) # Save the run, prepare worktree and plan data directory self.my_run = run @@ -2137,13 +2147,14 @@ def show(self) -> None: self._show_additional_keys() # Show fmf id of the remote plan in verbose mode - if (self._original_plan or self._remote_plan_fmf_id) and self.verbosity_level: + if (self._original_plan or self._imported_plan_fmf_id) and self.verbosity_level: # Pick fmf id from the original plan by default, use the # current plan in shallow mode when no plans are fetched. + if self._original_plan is not None: - fmf_id = self._original_plan._remote_plan_fmf_id + fmf_id = self._original_plan._imported_plan_fmf_id else: - fmf_id = self._remote_plan_fmf_id + fmf_id = self._imported_plan_fmf_id echo(tmt.utils.format('import', '', key_color='blue')) assert fmf_id is not None # narrow type @@ -2413,14 +2424,21 @@ def go(self) -> None: try: for step in self.steps(skip=['finish']): step.go() - # Finish plan if no tests found (except dry mode) - if (isinstance(step, tmt.steps.discover.Discover) and not step.tests() - and not self.is_dry_run and not step.extract_tests_later): - step.info( - 'warning', 'No tests found, finishing plan.', - color='yellow', shift=1) - abort = True - return + + if isinstance(step, tmt.steps.discover.Discover): + tests = step.tests() + + # Finish plan if no tests found (except dry mode) + if not tests and not self.is_dry_run and not step.extract_tests_later: + step.info( + 'warning', 'No tests found, finishing plan.', + color='yellow', shift=1) + abort = True + return + + if self.my_run and self.reshape(tests): + return + # Source the plan environment file after prepare and execute step if isinstance(step, (tmt.steps.prepare.Prepare, tmt.steps.execute.Execute)): self._source_plan_environment_file() @@ -2456,7 +2474,7 @@ def _export( @property def is_remote_plan_reference(self) -> bool: """ Check whether the plan is a remote plan reference """ - return self._remote_plan_fmf_id is not None + return self._imported_plan_fmf_id is not None def import_plan(self) -> Optional['Plan']: """ Import plan from a remote repository, return a Plan instance """ @@ -2466,8 +2484,8 @@ def import_plan(self) -> Optional['Plan']: if self._imported_plan: return self._imported_plan - assert self._remote_plan_fmf_id is not None # narrow type - plan_id = self._remote_plan_fmf_id + assert self._imported_plan_fmf_id is not None # narrow type + plan_id = self._imported_plan_fmf_id self.debug(f"Import remote plan '{plan_id.name}' from '{plan_id.url}'.", level=3) # Clone the whole git repository if executing tests (run is attached) @@ -2559,12 +2577,37 @@ def import_plan(self) -> Optional['Plan']: # Create the plan object, save links between both plans self._imported_plan = Plan(node=node, run=self.my_run, logger=self._logger) self._imported_plan._original_plan = self + self._imported_plan._original_plan_fmf_id = self.fmf_id with self.environment.as_environ(): expand_node_data(node.data, self._fmf_context) return self._imported_plan + def derive_plan(self, derived_id: int, tests: dict[str, list[Test]]) -> 'Plan': + derived_plan = Plan( + node=self.node, + run=self.my_run, + logger=self._logger, + name=f'{self.name}.{derived_id}') + + derived_plan._original_plan = self + derived_plan._original_plan_fmf_id = self.fmf_id + self._derived_plans.append(derived_plan) + + derived_plan.discover._tests = tests + derived_plan.discover.status('done') + + assert self.discover.workdir is not None + assert derived_plan.discover.workdir is not None + + shutil.copytree(self.discover.workdir, derived_plan.discover.workdir, dirs_exist_ok=True) + + for step_name in tmt.steps.STEPS: + getattr(derived_plan, step_name).save() + + return derived_plan + def prune(self) -> None: """ Remove all uninteresting files from the plan workdir """ @@ -2582,6 +2625,23 @@ def prune(self) -> None: for step in self.steps(enabled_only=False): step.prune(logger=step._logger) + def reshape(self, tests: list['tmt.steps.discover.TestAddress']) -> bool: + for shaper_id in tmt.plugins.plan_shapers._PLAN_SHAPER_PLUGIN_REGISTRY.iter_plugin_ids(): + shaper = tmt.plugins.plan_shapers._PLAN_SHAPER_PLUGIN_REGISTRY.get_plugin(shaper_id) + + assert shaper is not None # narrow type + + if not shaper.check(self, tests): + self.debug(f"Plan shaper '{shaper_id}' not applicable.") + continue + + if self.my_run: + self.my_run.swap_plans(self, *shaper.apply(self, tests)) + + return True + + return False + class StoryPriority(enum.Enum): MUST_HAVE = 'must have' @@ -3549,14 +3609,45 @@ def load(self) -> None: self.remove = self.remove or data.remove self.debug(f"Remove workdir when finished: {self.remove}", level=3) - @property - def plans(self) -> list[Plan]: + @functools.cached_property + def plans(self) -> Sequence[Plan]: """ Test plans for execution """ if self._plans is None: assert self.tree is not None # narrow type self._plans = self.tree.plans(run=self, filters=['enabled:true']) return self._plans + @functools.cached_property + def plan_queue(self) -> Sequence[Plan]: + """ + A list of plans remaining to be executed. + + It is being populated via :py:attr:`plans`, but eventually, + :py:meth:`go` will remove plans from it as they get processed. + :py:attr:`plans` will remain untouched and will represent all + plans collected. + """ + + return self.plans[:] + + def swap_plans(self, plan: Plan, *others: Plan) -> None: + """ + Replace given plan with one or more plans. + + :param plan: a plan to remove. + :param others: plans to put into the queue instead of ``plans``. + """ + + plans = cast(list[Plan], self.plans) + plan_queue = cast(list[Plan], self.plan_queue) + + if plan in plan_queue: + plan_queue.remove(plan) + plans.remove(plan) + + plan_queue.extend(others) + plans.extend(others) + def finish(self) -> None: """ Check overall results, return appropriate exit code """ # We get interesting results only if execute or prepare step is enabled @@ -3720,7 +3811,9 @@ def go(self) -> None: # Iterate over plans crashed_plans: list[tuple[Plan, Exception]] = [] - for plan in self.plans: + while self.plan_queue: + plan = cast(list[Plan], self.plan_queue).pop(0) + try: plan.go() diff --git a/tmt/cli.py b/tmt/cli.py index a9591c0bb4..cf85398e76 100644 --- a/tmt/cli.py +++ b/tmt/cli.py @@ -23,6 +23,7 @@ import tmt.log import tmt.options import tmt.plugins +import tmt.plugins.plan_shapers import tmt.steps import tmt.templates import tmt.trying @@ -461,6 +462,10 @@ def run(context: Context, id_: Optional[str], **kwargs: Any) -> None: context.obj.run = run +for plugin_class in tmt.plugins.plan_shapers._PLAN_SHAPER_PLUGIN_REGISTRY.iter_plugins(): + run = create_options_decorator(plugin_class.run_options())(run) + + # Steps options run.add_command(tmt.steps.discover.DiscoverPlugin.command()) run.add_command(tmt.steps.provision.ProvisionPlugin.command()) diff --git a/tmt/plugins/__init__.py b/tmt/plugins/__init__.py index fd7e42fa51..129832f7cf 100644 --- a/tmt/plugins/__init__.py +++ b/tmt/plugins/__init__.py @@ -73,6 +73,7 @@ def _discover_packages() -> list[tuple[str, Path]]: ('tmt.frameworks', Path('frameworks')), ('tmt.checks', Path('checks')), ('tmt.package_managers', Path('package_managers')), + ('tmt.plugins.plan_shapers', Path('plugins/plan_shapers')), ] diff --git a/tmt/plugins/plan_shapers/__init__.py b/tmt/plugins/plan_shapers/__init__.py new file mode 100644 index 0000000000..cdcfca1159 --- /dev/null +++ b/tmt/plugins/plan_shapers/__init__.py @@ -0,0 +1,67 @@ +from collections.abc import Iterator +from typing import TYPE_CHECKING, Any, Callable + +import tmt.log +import tmt.utils +from tmt.plugins import PluginRegistry + +if TYPE_CHECKING: + from tmt.base import Plan, Test + from tmt.options import ClickOptionDecoratorType + + +PlanShaperClass = type['PlanShaper'] + + +_PLAN_SHAPER_PLUGIN_REGISTRY: PluginRegistry[PlanShaperClass] = PluginRegistry() + + +def provides_plan_shaper( + shaper: str) -> Callable[[PlanShaperClass], PlanShaperClass]: + """ + A decorator for registering plan shaper plugins. + + Decorate a plan shaper plugin class to register a plan shaper. + """ + + def _provides_plan_shaper(plan_shaper_cls: PlanShaperClass) -> PlanShaperClass: + _PLAN_SHAPER_PLUGIN_REGISTRY.register_plugin( + plugin_id=shaper, + plugin=plan_shaper_cls, + logger=tmt.log.Logger.get_bootstrap_logger()) + + return plan_shaper_cls + + return _provides_plan_shaper + + +class PlanShaper(tmt.utils.Common): + """ A base class for plan shaper plugins """ + + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + + @classmethod + def run_options(cls) -> list['ClickOptionDecoratorType']: + """ Return additional options for ``tmt run`` """ + + raise NotImplementedError + + @classmethod + def check(cls, plan: 'Plan', tests: list[tuple[str, 'Test']]) -> bool: + """ Check whether this shaper should be applied to the given plan """ + + raise NotImplementedError + + @classmethod + def apply( + cls, + plan: 'Plan', + tests: list[tuple[str, 'Test']]) -> Iterator['Plan']: + """ + Apply the shaper to a given plan and a set of tests. + + :returns: a sequence of plans replacing the original plan. + """ + + raise NotImplementedError diff --git a/tmt/plugins/plan_shapers/max_tests.py b/tmt/plugins/plan_shapers/max_tests.py new file mode 100644 index 0000000000..6e52b875ad --- /dev/null +++ b/tmt/plugins/plan_shapers/max_tests.py @@ -0,0 +1,74 @@ +import itertools +from collections.abc import Iterator +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from tmt.base import Plan, Test + from tmt.options import ClickOptionDecoratorType + +from tmt.plugins.plan_shapers import PlanShaper, provides_plan_shaper + + +@provides_plan_shaper('max-tests') +class MaxTestsPlanShaper(PlanShaper): + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + + @classmethod + def run_options(cls) -> list['ClickOptionDecoratorType']: + from tmt.cli import option + + return [ + option( + '--max', + metavar='N', + help='Split plans to include N tests at max.', + type=int, + default=-1) + ] + + @classmethod + def check(cls, plan: 'Plan', tests: list[tuple[str, 'Test']]) -> bool: + if not plan.my_run: + return False + + max_test_count = plan.my_run.opt('max') + + if max_test_count <= 0: + return False + + if len(tests) <= max_test_count: + return False + + return True + + @classmethod + def apply( + cls, + plan: 'Plan', + tests: list[tuple[str, 'Test']]) -> Iterator['Plan']: + assert plan.my_run is not None + + max_test_per_batch = plan.my_run.opt('max') + + plan.info(f'Splitting plan to batches of {max_test_per_batch} tests.') + + for batch_id in itertools.count(1): + if not tests: + break + + batch: dict[str, list[Test]] = {} + + for _ in range(max_test_per_batch): + if not tests: + break + + phase_name, test = tests.pop(0) + + if phase_name not in batch: + batch[phase_name] = [test] + + else: + batch[phase_name].append(test) + + yield plan.derive_plan(batch_id, batch) diff --git a/tmt/steps/discover/__init__.py b/tmt/steps/discover/__init__.py index ccc416fbc6..c7691806aa 100644 --- a/tmt/steps/discover/__init__.py +++ b/tmt/steps/discover/__init__.py @@ -11,6 +11,9 @@ import tmt.cli import tmt.options import tmt.steps + from tmt._compat.typing import TypeAlias + + TestAddress: TypeAlias = tuple[str, 'tmt.Test'] import tmt.base import tmt.steps @@ -118,13 +121,32 @@ def tests( self, *, phase_name: Optional[str] = None, - enabled: Optional[bool] = None) -> list['tmt.Test']: + enabled: Optional[bool] = None) -> list['TestAddress']: """ - Return discovered tests - - Each DiscoverPlugin has to implement this method. - Should return a list of Test() objects. + Return discovered tests. + + :param phase_name: if set, return only tests discovered by the + phase of this name. Otherwise, all tests discovered by the + phase are returned. + + .. note:: + + This parameter exists to present unified interface with + :py:meth:`tmt.steps.discover.Discover.tests` API, but it + has no interesting effect in case of individual phases: + + * left unset, all tests discovered by the phase are + returned, + * set to a phase name, tests discovered by that phase + should be returned. But a phase does not have access to + other phases' tests, therefore setting it to anything + but this phase name would produce an empty list. + :param enabled: if set, return only tests that are enabled + (``enabled=True``) or disabled (``enabled=False``). Otherwise, + all tests are returned. + :returns: a list of phase name and test pairs. """ + raise NotImplementedError def download_distgit_source( @@ -149,8 +171,8 @@ def log_import_plan_details(self) -> None: """ Log details about the imported plan """ parent = cast(Optional[tmt.steps.discover.Discover], self.parent) if parent and parent.plan._original_plan and \ - parent.plan._original_plan._remote_plan_fmf_id: - remote_plan_id = parent.plan._original_plan._remote_plan_fmf_id + parent.plan._original_plan._imported_plan_fmf_id: + remote_plan_id = parent.plan._original_plan._imported_plan_fmf_id # FIXME: cast() - https://github.com/python/mypy/issues/7981 # Note the missing Optional for values - to_minimal_dict() would # not include unset keys, therefore all values should be valid. @@ -326,7 +348,7 @@ def summary(self) -> None: text = listed(len(self.tests(enabled=True)), 'test') + ' selected' self.info('summary', text, 'green', shift=1) # Test list in verbose mode - for test in self.tests(enabled=True): + for _, test in self.tests(enabled=True): self.verbose(test.name, color='red', shift=2) def go(self, force: bool = False) -> None: @@ -354,7 +376,7 @@ def go(self, force: bool = False) -> None: # Prefix test name only if multiple plugins configured prefix = f'/{phase.name}' if len(self.phases()) > 1 else '' # Check discovered tests, modify test name/path - for test in phase.tests(enabled=True): + for _, test in phase.tests(enabled=True): test.name = f"{prefix}{test.name}" test.path = Path(f"/{phase.safe_name}{test.path}") # Update test environment with plan environment @@ -364,7 +386,7 @@ def go(self, force: bool = False) -> None: else: raise GeneralError(f'Unexpected phase in discover step: {phase}') - for test in self.tests(): + for _, test in self.tests(): test.serial_number = self.plan.draw_test_serial_number(test) # Show fmf identifiers for tests discovered in plan @@ -373,7 +395,7 @@ def go(self, force: bool = False) -> None: if self.tests(enabled=True): export_fmf_ids: list[str] = [] - for test in self.tests(enabled=True): + for _, test in self.tests(enabled=True): fmf_id = test.fmf_id if not fmf_id.url: @@ -420,21 +442,36 @@ def tests( self, *, phase_name: Optional[str] = None, - enabled: Optional[bool] = None) -> list['tmt.Test']: - def _iter_all_tests() -> Iterator['tmt.Test']: - tests = self._failed_tests if self._failed_tests else self._tests - for phase_tests in tests.values(): - yield from phase_tests + enabled: Optional[bool] = None) -> list['TestAddress']: + """ + Return discovered tests. + + :param phase_name: if set, return only tests discovered by the + phase of this name. Otherwise, tests discovered by all + phases are returned. + :param enabled: if set, return only tests that are enabled + (``enabled=True``) or disabled (``enabled=False``). Otherwise, + all tests are returned. + :returns: a list of phase name and test pairs. + """ - def _iter_phase_tests() -> Iterator['tmt.Test']: - assert phase_name is not None - tests = self._failed_tests if self._failed_tests else self._tests + suitable_tests = self._failed_tests if self._failed_tests else self._tests + suitable_phases = [phase_name] if phase_name is not None else list(self._tests.keys()) - yield from tests[phase_name] + def _iter_tests() -> Iterator['TestAddress']: + # PLR1704: this redefinition of `phase_name` is acceptable, the original + # value is not longer needed as it has been turned into `suitable_phases`. + for phase_name, phase_tests in suitable_tests.items(): # noqa: PLR1704 + if phase_name not in suitable_phases: + continue - iterator = _iter_all_tests if phase_name is None else _iter_phase_tests + for test in phase_tests: + yield phase_name, test if enabled is None: - return list(iterator()) + return list(_iter_tests()) - return [test for test in iterator() if test.enabled is enabled] + return [ + (phase_name, test) + for phase_name, test in _iter_tests() + if test.enabled is enabled] diff --git a/tmt/steps/discover/fmf.py b/tmt/steps/discover/fmf.py index 2c4f5d6e12..a1e10fadbb 100644 --- a/tmt/steps/discover/fmf.py +++ b/tmt/steps/discover/fmf.py @@ -759,7 +759,7 @@ def post_dist_git(self, created_content: list[Path]) -> None: # Prefix test name only if multiple plugins configured prefix = f'/{self.name}' if len(self.step.phases()) > 1 else '' # Check discovered tests, modify test name/path - for test in self.tests(enabled=True): + for _, test in self.tests(enabled=True): test.name = f"{prefix}{test.name}" test.path = Path(f"/{self.safe_name}{test.path}") # Update test environment with plan environment @@ -773,13 +773,12 @@ def tests( self, *, phase_name: Optional[str] = None, - enabled: Optional[bool] = None) -> list['tmt.Test']: - """ Return all discovered tests """ + enabled: Optional[bool] = None) -> list[tuple[str, 'tmt.Test']]: if phase_name is not None and phase_name != self.name: return [] if enabled is None: - return self._tests + return [(self.name, test) for test in self._tests] - return [test for test in self._tests if test.enabled is enabled] + return [(self.name, test) for test in self._tests if test.enabled is enabled] diff --git a/tmt/steps/discover/shell.py b/tmt/steps/discover/shell.py index 986f0cd51c..2fbcb8c5ed 100644 --- a/tmt/steps/discover/shell.py +++ b/tmt/steps/discover/shell.py @@ -450,12 +450,12 @@ def tests( self, *, phase_name: Optional[str] = None, - enabled: Optional[bool] = None) -> list['tmt.Test']: + enabled: Optional[bool] = None) -> list[tuple[str, 'tmt.Test']]: if phase_name is not None and phase_name != self.name: return [] if enabled is None: - return self._tests + return [(self.name, test) for test in self._tests] - return [test for test in self._tests if test.enabled is enabled] + return [(self.name, test) for test in self._tests if test.enabled is enabled] diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index 407b4ac096..987085ddb4 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -565,7 +565,7 @@ def prepare_tests(self, guest: Guest, logger: tmt.log.Logger) -> list[TestInvoca """ invocations: list[TestInvocation] = [] - for test in self.discover.tests(phase_name=self.discover_phase, enabled=True): + for _, test in self.discover.tests(phase_name=self.discover_phase, enabled=True): invocation = TestInvocation(phase=self, test=test, guest=guest, logger=logger) invocations.append(invocation) @@ -1169,8 +1169,8 @@ def results(self) -> list["tmt.result.Result"]: def results_for_tests( self, - tests: list['tmt.base.Test'] - ) -> list[tuple[Optional[Result], Optional['tmt.base.Test']]]: + tests: list['tmt.steps.discover.TestAddress'] + ) -> list[tuple[Optional[Result], Optional['tmt.steps.discover.TestAddress']]]: """ Collect results and corresponding tests. @@ -1183,14 +1183,17 @@ def results_for_tests( ``(None, test)``. """ - known_serial_numbers = {test.serial_number: test for test in tests} + known_serial_numbers = { + test_address[1].serial_number: test_address + for test_address in tests + } referenced_serial_numbers = {result.serial_number for result in self._results} return [ (result, known_serial_numbers.get(result.serial_number)) for result in self._results ] + [ - (None, test) - for test in tests - if test.serial_number not in referenced_serial_numbers + (None, test_address) + for test_address in tests + if test_address[1].serial_number not in referenced_serial_numbers ] diff --git a/tmt/steps/execute/upgrade.py b/tmt/steps/execute/upgrade.py index 801ad11957..c9bea3ef98 100644 --- a/tmt/steps/execute/upgrade.py +++ b/tmt/steps/execute/upgrade.py @@ -337,7 +337,7 @@ def _perform_upgrade( required_packages: list[tmt.base.DependencySimple] = [] recommended_packages: list[tmt.base.DependencySimple] = [] - for test in self._discover_upgrade.tests(enabled=True): + for _, test in self._discover_upgrade.tests(enabled=True): test.name = f'/{DURING_UPGRADE_PREFIX}/{test.name.lstrip("/")}' # Gathering dependencies for upgrade tasks @@ -386,7 +386,7 @@ def _run_test_phase( The prefix is also set as IN_PLACE_UPGRADE environment variable. """ names_backup = [] - for test in self.discover.tests(enabled=True): + for _, test in self.discover.tests(enabled=True): names_backup.append(test.name) test.name = f'/{prefix}/{test.name.lstrip("/")}' @@ -395,6 +395,5 @@ def _run_test_phase( extra_environment=Environment({STATUS_VARIABLE: EnvVarValue(prefix)}), logger=logger) - tests = self.discover.tests(enabled=True) - for i, test in enumerate(tests): + for i, (_, test) in enumerate(self.discover.tests(enabled=True)): test.name = names_backup[i] diff --git a/tmt/steps/prepare/__init__.py b/tmt/steps/prepare/__init__.py index 4b86b59277..bc34725909 100644 --- a/tmt/steps/prepare/__init__.py +++ b/tmt/steps/prepare/__init__.py @@ -240,7 +240,7 @@ def as_key(self) -> frozenset['tmt.base.DependencySimple']: # use what the step loads from its storage, `tests.yaml`. Which # means, there probably would be no phases to inspect from time to # time, therefore going after the step itself. - for test in self.plan.discover.tests(enabled=True): + for _, test in self.plan.discover.tests(enabled=True): if not test.enabled_on_guest(guest): continue diff --git a/tmt/steps/prepare/distgit.py b/tmt/steps/prepare/distgit.py index 4c9881b406..9042653f1e 100644 --- a/tmt/steps/prepare/distgit.py +++ b/tmt/steps/prepare/distgit.py @@ -290,7 +290,8 @@ def go( for g in self.step.plan.provision.guests(): collected_requires: list[tmt.base.DependencySimple] = [] collected_recommends: list[tmt.base.DependencySimple] = [] - for test in self.step.plan.discover.tests(enabled=True): + + for _, test in self.step.plan.discover.tests(enabled=True): if not test.enabled_on_guest(g): continue diff --git a/tmt/steps/report/reportportal.py b/tmt/steps/report/reportportal.py index 91d0c2f5d6..265df6b7b8 100644 --- a/tmt/steps/report/reportportal.py +++ b/tmt/steps/report/reportportal.py @@ -511,8 +511,11 @@ def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: self.verbose("uuid", suite_uuid, "yellow", shift=1) self.data.suite_uuid = suite_uuid - for result, test in self.step.plan.execute.results_for_tests( + # For each test + for result, test_address in self.step.plan.execute.results_for_tests( self.step.plan.discover.tests()): + test = test_address[1] if test_address else None + test_time = self.time() test_name = None test_description = ''