From ff99daf5ae812fc7f9c6d8f4de0c84215e1e4896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Sun, 26 May 2024 16:09:24 +0200 Subject: [PATCH] Let plugin go() method return a value A preparation patch to introduce parametrized return value to types and annotations. No plugin returns any value yet, only type type changes and minor `go()` refactoring is covered. --- tmt/steps/__init__.py | 32 ++++++++++++++++++-------------- tmt/steps/discover/__init__.py | 7 ++++++- tmt/steps/discover/fmf.py | 4 ++-- tmt/steps/discover/shell.py | 4 ++-- tmt/steps/execute/__init__.py | 8 ++++---- tmt/steps/finish/__init__.py | 14 +++++++++++--- tmt/steps/prepare/__init__.py | 18 ++++++------------ tmt/steps/provision/__init__.py | 9 +++++++-- tmt/steps/provision/artemis.py | 4 ++-- tmt/steps/provision/connect.py | 4 ++-- tmt/steps/provision/local.py | 4 ++-- tmt/steps/provision/mrack.py | 4 ++-- tmt/steps/provision/podman.py | 4 ++-- tmt/steps/provision/testcloud.py | 4 ++-- tmt/steps/report/__init__.py | 7 ++++++- tmt/steps/report/display.py | 6 ++++-- tmt/steps/report/html.py | 5 +++-- tmt/steps/report/junit.py | 4 ++-- tmt/steps/report/polarion.py | 4 ++-- tmt/steps/report/reportportal.py | 4 ++-- 20 files changed, 87 insertions(+), 63 deletions(-) diff --git a/tmt/steps/__init__.py b/tmt/steps/__init__.py index c709251669..68d1f36997 100644 --- a/tmt/steps/__init__.py +++ b/tmt/steps/__init__.py @@ -228,6 +228,9 @@ class _RawStepData(TypedDict, total=False): StepDataT = TypeVar('StepDataT', bound='StepData') +#: A type variable representing a return value of plugin's ``go()`` method. +PluginReturnValueT = TypeVar('PluginReturnValueT') + @dataclasses.dataclass class StepData( @@ -1155,7 +1158,7 @@ def _method(cls: PluginClass) -> PluginClass: # unable to introduce the type var into annotations. Apparently, `cls` # is a more complete type, e.g. `type[ReportJUnit]`, which does not show # space for type var. But it's still something to fix later. - cast('BasePlugin[Any]', cls.__bases__[0])._supported_methods \ + cast('BasePlugin[Any, Any]', cls.__bases__[0])._supported_methods \ .register_plugin( plugin_id=name, plugin=plugin_method, @@ -1166,7 +1169,7 @@ def _method(cls: PluginClass) -> PluginClass: return _method -class BasePlugin(Phase, Generic[StepDataT]): +class BasePlugin(Phase, Generic[StepDataT, PluginReturnValueT]): """ Common parent of all step plugins """ # Deprecated, use @provides_method(...) instead. left for backward @@ -1335,7 +1338,8 @@ def delegate( cls, step: Step, data: Optional[StepDataT] = None, - raw_data: Optional[_RawStepData] = None) -> 'BasePlugin[StepDataT]': + raw_data: Optional[_RawStepData] = None + ) -> 'BasePlugin[StepDataT, PluginReturnValueT]': """ Return plugin instance implementing the data['how'] method @@ -1621,16 +1625,16 @@ def prune(self, logger: tmt.log.Logger) -> None: logger.warn(f"Unable to remove '{self.workdir}': {error}") -class GuestlessPlugin(BasePlugin[StepDataT]): +class GuestlessPlugin(BasePlugin[StepDataT, PluginReturnValueT]): """ Common parent of all step plugins that do not work against a particular guest """ - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> PluginReturnValueT: """ Perform actions shared among plugins when beginning their tasks """ - self.go_prolog(self._logger) + raise NotImplementedError -class Plugin(BasePlugin[StepDataT]): +class Plugin(BasePlugin[StepDataT, PluginReturnValueT]): """ Common parent of all step plugins that do work against a particular guest """ def go( @@ -1638,10 +1642,10 @@ def go( *, guest: 'Guest', environment: Optional[tmt.utils.Environment] = None, - logger: tmt.log.Logger) -> None: + logger: tmt.log.Logger) -> PluginReturnValueT: """ Perform actions shared among plugins when beginning their tasks """ - self.go_prolog(logger) + raise NotImplementedError class Action(Phase, tmt.utils.MultiInvokableCommon): @@ -2137,17 +2141,17 @@ def run(self, logger: tmt.log.Logger) -> None: @dataclasses.dataclass -class PluginTask(tmt.queue.MultiGuestTask[None], Generic[StepDataT]): +class PluginTask(tmt.queue.MultiGuestTask[None], Generic[StepDataT, PluginReturnValueT]): """ A task to run a phase on a given set of guests """ - phase: Plugin[StepDataT] + phase: Plugin[StepDataT, PluginReturnValueT] # Custom yet trivial `__init__` is necessary, see note in `tmt.queue.Task`. def __init__( self, logger: tmt.log.Logger, guests: list['Guest'], - phase: Plugin[StepDataT], + phase: Plugin[StepDataT, PluginReturnValueT], **kwargs: Any) -> None: super().__init__(logger, guests, **kwargs) @@ -2174,7 +2178,7 @@ def run_on_guest(self, guest: 'Guest', logger: tmt.log.Logger) -> None: self.phase.go(guest=guest, logger=logger) -class PhaseQueue(tmt.queue.Queue[Union[ActionTask, PluginTask[StepDataT]]]): +class PhaseQueue(tmt.queue.Queue[Union[ActionTask, PluginTask[StepDataT, PluginReturnValueT]]]): """ Queue class for running phases on guests """ def enqueue_action( @@ -2189,7 +2193,7 @@ def enqueue_action( def enqueue_plugin( self, *, - phase: Plugin[StepDataT], + phase: Plugin[StepDataT, PluginReturnValueT], guests: list['Guest']) -> None: if not guests: raise tmt.utils.MetadataError( diff --git a/tmt/steps/discover/__init__.py b/tmt/steps/discover/__init__.py index 21f3bcac2d..13268a1556 100644 --- a/tmt/steps/discover/__init__.py +++ b/tmt/steps/discover/__init__.py @@ -75,7 +75,7 @@ class DiscoverStepData(tmt.steps.WhereableStepData, tmt.steps.StepData): DiscoverStepDataT = TypeVar('DiscoverStepDataT', bound=DiscoverStepData) -class DiscoverPlugin(tmt.steps.GuestlessPlugin[DiscoverStepDataT]): +class DiscoverPlugin(tmt.steps.GuestlessPlugin[DiscoverStepDataT, None]): """ Common parent of discover plugins """ # ignore[assignment]: as a base class, DiscoverStepData is not included in @@ -109,6 +109,11 @@ def discover(context: 'tmt.cli.Context', **kwargs: Any) -> None: return discover + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: + """ Perform actions shared among plugins when beginning their tasks """ + + self.go_prolog(logger or self._logger) + def tests( self, *, diff --git a/tmt/steps/discover/fmf.py b/tmt/steps/discover/fmf.py index a09c47561d..2346cf00bf 100644 --- a/tmt/steps/discover/fmf.py +++ b/tmt/steps/discover/fmf.py @@ -287,9 +287,9 @@ def get_git_root(self, dir: Path) -> Path: assert output.stdout is not None return Path(output.stdout.strip("\n")) - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Discover available tests """ - super().go() + super().go(logger=logger) # Check url and path, prepare test directory url = self.get('url') diff --git a/tmt/steps/discover/shell.py b/tmt/steps/discover/shell.py index 6fd76f8881..21d721486c 100644 --- a/tmt/steps/discover/shell.py +++ b/tmt/steps/discover/shell.py @@ -319,9 +319,9 @@ def fetch_remote_repository( if not keep_git_metadata: shutil.rmtree(testdir / '.git') - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Discover available tests """ - super().go() + super().go(logger=logger) tests = fmf.Tree({'summary': 'tests'}) assert self.workdir is not None diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index 6c1c60872e..15fd3d2295 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -430,7 +430,7 @@ def terminate_process( self.guest._cleanup_ssh_master_process(signal, logger) -class ExecutePlugin(tmt.steps.Plugin[ExecuteStepDataT]): +class ExecutePlugin(tmt.steps.Plugin[ExecuteStepDataT, None]): """ Common parent of execute plugins """ # ignore[assignment]: as a base class, ExecuteStepData is not included in @@ -491,7 +491,7 @@ def go( guest: 'Guest', environment: Optional[tmt.utils.Environment] = None, logger: tmt.log.Logger) -> None: - super().go(guest=guest, environment=environment, logger=logger) + self.go_prolog(logger) logger.verbose('exit-first', self.data.exit_first, 'green', level=2) @property @@ -896,7 +896,7 @@ def go(self, force: bool = False) -> None: raise tmt.utils.ExecuteError("No guests available for execution.") # Execute the tests, store results - queue: PhaseQueue[ExecuteStepData] = PhaseQueue( + queue: PhaseQueue[ExecuteStepData, None] = PhaseQueue( 'execute', self._logger.descend(logger_name=f'{self}.queue')) @@ -930,7 +930,7 @@ def go(self, force: bool = False) -> None: if discover.enabled_on_guest(guest) ]) - failed_tasks: list[Union[ActionTask, PluginTask[ExecuteStepData]]] = [] + failed_tasks: list[Union[ActionTask, PluginTask[ExecuteStepData, None]]] = [] for outcome in queue.run(): if outcome.exc: diff --git a/tmt/steps/finish/__init__.py b/tmt/steps/finish/__init__.py index 02f109257f..0a60e2c8f8 100644 --- a/tmt/steps/finish/__init__.py +++ b/tmt/steps/finish/__init__.py @@ -32,7 +32,7 @@ class FinishStepData(tmt.steps.WhereableStepData, tmt.steps.StepData): FinishStepDataT = TypeVar('FinishStepDataT', bound=FinishStepData) -class FinishPlugin(tmt.steps.Plugin[FinishStepDataT]): +class FinishPlugin(tmt.steps.Plugin[FinishStepDataT, None]): """ Common parent of finish plugins """ # ignore[assignment]: as a base class, FinishStepData is not included in @@ -66,6 +66,14 @@ def finish(context: 'tmt.cli.Context', **kwargs: Any) -> None: return finish + def go( + self, + *, + guest: 'Guest', + environment: Optional[tmt.utils.Environment] = None, + logger: tmt.log.Logger) -> None: + self.go_prolog(logger) + class Finish(tmt.steps.Step): """ @@ -141,7 +149,7 @@ def go(self, force: bool = False) -> None: guest_copies.append(guest_copy) - queue: PhaseQueue[FinishStepData] = PhaseQueue( + queue: PhaseQueue[FinishStepData, None] = PhaseQueue( 'finish', self._logger.descend(logger_name=f'{self}.queue')) @@ -155,7 +163,7 @@ def go(self, force: bool = False) -> None: guests=[guest for guest in guest_copies if phase.enabled_on_guest(guest)] ) - failed_tasks: list[Union[ActionTask, PluginTask[FinishStepData]]] = [] + failed_tasks: list[Union[ActionTask, PluginTask[FinishStepData, None]]] = [] for outcome in queue.run(): if not isinstance(outcome.phase, FinishPlugin): diff --git a/tmt/steps/prepare/__init__.py b/tmt/steps/prepare/__init__.py index c2bd90f03e..bcae0a100d 100644 --- a/tmt/steps/prepare/__init__.py +++ b/tmt/steps/prepare/__init__.py @@ -1,14 +1,7 @@ import collections import copy import dataclasses -from typing import ( - TYPE_CHECKING, - Any, - Optional, - TypeVar, - Union, - cast, - ) +from typing import TYPE_CHECKING, Any, Optional, TypeVar, Union, cast import click import fmf @@ -58,7 +51,7 @@ class _RawPrepareStepData(tmt.steps._RawStepData, total=False): summary: str -class PreparePlugin(tmt.steps.Plugin[PrepareStepDataT]): +class PreparePlugin(tmt.steps.Plugin[PrepareStepDataT, None]): """ Common parent of prepare plugins """ # ignore[assignment]: as a base class, PrepareStepData is not included in @@ -99,7 +92,8 @@ def go( environment: Optional[tmt.utils.Environment] = None, logger: tmt.log.Logger) -> None: """ Prepare the guest (common actions) """ - super().go(guest=guest, environment=environment, logger=logger) + + self.go_prolog(logger) # Show guest name first in multihost scenarios if self.step.plan.provision.is_multihost: @@ -355,7 +349,7 @@ def as_key(self) -> frozenset['tmt.base.DependencySimple']: # To separate "push" from "prepare" queue visually self.info('') - queue: PhaseQueue[PrepareStepData] = PhaseQueue( + queue: PhaseQueue[PrepareStepData, None] = PhaseQueue( 'prepare', self._logger.descend(logger_name=f'{self}.queue')) @@ -369,7 +363,7 @@ def as_key(self) -> frozenset['tmt.base.DependencySimple']: guests=[ guest for guest in guest_copies if prepare_phase.enabled_on_guest(guest)]) - failed_tasks: list[Union[ActionTask, PluginTask[PrepareStepData]]] = [] + failed_tasks: list[Union[ActionTask, PluginTask[PrepareStepData, None]]] = [] for outcome in queue.run(): if not isinstance(outcome.phase, PreparePlugin): diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index f54c63acaa..6b59fc66b2 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -1960,7 +1960,7 @@ class ProvisionStepData(tmt.steps.StepData): ProvisionStepDataT = TypeVar('ProvisionStepDataT', bound=ProvisionStepData) -class ProvisionPlugin(tmt.steps.GuestlessPlugin[ProvisionStepDataT]): +class ProvisionPlugin(tmt.steps.GuestlessPlugin[ProvisionStepDataT, None]): """ Common parent of provision plugins """ # ignore[assignment]: as a base class, ProvisionStepData is not included in @@ -2006,6 +2006,11 @@ def provision(context: 'tmt.cli.Context', **kwargs: Any) -> None: return provision + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: + """ Perform actions shared among plugins when beginning their tasks """ + + self.go_prolog(logger or self._logger) + # TODO: this might be needed until https://github.com/teemtee/tmt/issues/1696 is resolved def opt(self, option: str, default: Optional[Any] = None) -> Any: """ Get an option from the command line options """ @@ -2358,7 +2363,7 @@ def _run_action_phases(phases: list[Action]) -> tuple[list[ActionTask], list[Act tasks that failed. """ - queue: PhaseQueue[ProvisionStepData] = PhaseQueue( + queue: PhaseQueue[ProvisionStepData, None] = PhaseQueue( 'provision.action', self._logger.descend(logger_name=f'{self}.queue')) diff --git a/tmt/steps/provision/artemis.py b/tmt/steps/provision/artemis.py index fb72e0e9ce..81893f4f25 100644 --- a/tmt/steps/provision/artemis.py +++ b/tmt/steps/provision/artemis.py @@ -729,9 +729,9 @@ class ProvisionArtemis(tmt.steps.provision.ProvisionPlugin[ProvisionArtemisData] # Guest instance _guest = None - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Provision the guest """ - super().go() + super().go(logger=logger) if self.data.api_version not in SUPPORTED_API_VERSIONS: raise ArtemisProvisionError(f"API version '{self.data.api_version}' not supported.") diff --git a/tmt/steps/provision/connect.py b/tmt/steps/provision/connect.py index 9cf3135313..58608e286c 100644 --- a/tmt/steps/provision/connect.py +++ b/tmt/steps/provision/connect.py @@ -197,9 +197,9 @@ class ProvisionConnect(tmt.steps.provision.ProvisionPlugin[ProvisionConnectData] # Guest instance _guest = None - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Prepare the connection """ - super().go() + super().go(logger=logger) # Check guest and auth info if not self.data.guest: diff --git a/tmt/steps/provision/local.py b/tmt/steps/provision/local.py index 3ceb6d53cf..c25cc3e1f4 100644 --- a/tmt/steps/provision/local.py +++ b/tmt/steps/provision/local.py @@ -173,9 +173,9 @@ class ProvisionLocal(tmt.steps.provision.ProvisionPlugin[ProvisionLocalData]): # Guest instance _guest = None - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Provision the container """ - super().go() + super().go(logger=logger) # Create a GuestLocal instance data = tmt.steps.provision.GuestData.from_plugin(self) diff --git a/tmt/steps/provision/mrack.py b/tmt/steps/provision/mrack.py index 1559777c36..348bd3973c 100644 --- a/tmt/steps/provision/mrack.py +++ b/tmt/steps/provision/mrack.py @@ -1011,9 +1011,9 @@ def wake(self, data: Optional[BeakerGuestData] = None) -> None: # type: ignore[ logger=self._logger, ) - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Provision the guest """ - super().go() + super().go(logger=logger) data = BeakerGuestData.from_plugin(self) diff --git a/tmt/steps/provision/podman.py b/tmt/steps/provision/podman.py index e0c3102641..acb409bb60 100644 --- a/tmt/steps/provision/podman.py +++ b/tmt/steps/provision/podman.py @@ -449,9 +449,9 @@ def default(self, option: str, default: Any = None) -> Any: return super().default(option, default=default) - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Provision the container """ - super().go() + super().go(logger=logger) # Prepare data for the guest instance data = PodmanGuestData.from_plugin(self) diff --git a/tmt/steps/provision/testcloud.py b/tmt/steps/provision/testcloud.py index 9681c649bf..7ebf4c7804 100644 --- a/tmt/steps/provision/testcloud.py +++ b/tmt/steps/provision/testcloud.py @@ -1064,9 +1064,9 @@ class ProvisionTestcloud(tmt.steps.provision.ProvisionPlugin[ProvisionTestcloudD # Guest instance _guest = None - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Provision the testcloud instance """ - super().go() + super().go(logger=logger) if self.data.list_local_images: self._print_local_images() diff --git a/tmt/steps/report/__init__.py b/tmt/steps/report/__init__.py index b1bc67c6cb..813ce3bb0c 100644 --- a/tmt/steps/report/__init__.py +++ b/tmt/steps/report/__init__.py @@ -22,7 +22,7 @@ class ReportStepData(tmt.steps.StepData): ReportStepDataT = TypeVar('ReportStepDataT', bound=ReportStepData) -class ReportPlugin(tmt.steps.GuestlessPlugin[ReportStepDataT]): +class ReportPlugin(tmt.steps.GuestlessPlugin[ReportStepDataT, None]): """ Common parent of report plugins """ # ignore[assignment]: as a base class, ReportStepData is not included in @@ -59,6 +59,11 @@ def report(context: 'tmt.cli.Context', **kwargs: Any) -> None: return report + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: + """ Perform actions shared among plugins when beginning their tasks """ + + self.go_prolog(logger or self._logger) + class Report(tmt.steps.Step): """ Provide test results overview and send reports. """ diff --git a/tmt/steps/report/display.py b/tmt/steps/report/display.py index 3f38c23eb4..9e1d9d4e76 100644 --- a/tmt/steps/report/display.py +++ b/tmt/steps/report/display.py @@ -1,6 +1,8 @@ import dataclasses +from typing import Optional import tmt +import tmt.log import tmt.steps import tmt.steps.report from tmt.result import BaseResult, CheckResult, Result @@ -131,9 +133,9 @@ def display_log_content(result: BaseResult) -> None: elif verbosity > 1: display_log_info(check_result) - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Discover available tests """ - super().go() + super().go(logger=logger) # Show individual test results only in verbose mode if not self.verbosity_level: return diff --git a/tmt/steps/report/html.py b/tmt/steps/report/html.py index dcb138c86d..3bda9208da 100644 --- a/tmt/steps/report/html.py +++ b/tmt/steps/report/html.py @@ -1,5 +1,6 @@ import dataclasses import webbrowser +from typing import Optional import tmt import tmt.log @@ -59,9 +60,9 @@ def prune(self, logger: tmt.log.Logger) -> None: """ Do not prune generated html report """ pass - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Process results """ - super().go() + super().go(logger=logger) # Prepare the template environment = tmt.utils.default_template_environment() diff --git a/tmt/steps/report/junit.py b/tmt/steps/report/junit.py index d302d79827..13e72aa4d3 100644 --- a/tmt/steps/report/junit.py +++ b/tmt/steps/report/junit.py @@ -113,9 +113,9 @@ def prune(self, logger: tmt.log.Logger) -> None: """ Do not prune generated junit report """ pass - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Read executed tests and write junit """ - super().go() + super().go(logger=logger) junit_xml = import_junit_xml() suite = make_junit_xml(self) diff --git a/tmt/steps/report/polarion.py b/tmt/steps/report/polarion.py index d2bc4e1dc5..3a6dd1b7c5 100644 --- a/tmt/steps/report/polarion.py +++ b/tmt/steps/report/polarion.py @@ -191,9 +191,9 @@ def prune(self, logger: tmt.log.Logger) -> None: """ Do not prune generated xunit report """ pass - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Go through executed tests and report into Polarion """ - super().go() + super().go(logger=logger) from tmt.export.polarion import find_polarion_case_ids, import_polarion import_polarion() diff --git a/tmt/steps/report/reportportal.py b/tmt/steps/report/reportportal.py index c2154a85c6..6b08daf364 100644 --- a/tmt/steps/report/reportportal.py +++ b/tmt/steps/report/reportportal.py @@ -311,7 +311,7 @@ def append_description(self, curr_description: str) -> str: curr_description = self.data.launch_description return curr_description - def go(self) -> None: + def go(self, *, logger: Optional[tmt.log.Logger] = None) -> None: """ Report test results to the endpoint @@ -319,7 +319,7 @@ def go(self) -> None: fill it with all parts needed and report the logs. """ - super().go() + super().go(logger=logger) if not self.data.url: raise tmt.utils.ReportError("No ReportPortal endpoint url provided.")