Skip to content

Commit

Permalink
Let plugin go() method return a value
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
happz committed May 28, 2024
1 parent 7768275 commit 7cf7c3a
Show file tree
Hide file tree
Showing 20 changed files with 87 additions and 63 deletions.
32 changes: 18 additions & 14 deletions tmt/steps/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,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(
Expand Down Expand Up @@ -1154,7 +1157,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,
Expand All @@ -1165,7 +1168,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
Expand Down Expand Up @@ -1334,7 +1337,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
Expand Down Expand Up @@ -1620,27 +1624,27 @@ 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(
self,
*,
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):
Expand Down Expand Up @@ -2135,17 +2139,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)

Expand All @@ -2172,7 +2176,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(
Expand All @@ -2187,7 +2191,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(
Expand Down
7 changes: 6 additions & 1 deletion tmt/steps/discover/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
*,
Expand Down
4 changes: 2 additions & 2 deletions tmt/steps/discover/fmf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
4 changes: 2 additions & 2 deletions tmt/steps/discover/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions tmt/steps/execute/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -876,7 +876,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'))

Expand Down Expand Up @@ -910,7 +910,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:
Expand Down
14 changes: 11 additions & 3 deletions tmt/steps/finish/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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'))

Expand All @@ -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):
Expand Down
18 changes: 6 additions & 12 deletions tmt/steps/prepare/__init__.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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'))

Expand All @@ -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):
Expand Down
9 changes: 7 additions & 2 deletions tmt/steps/provision/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1893,7 +1893,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
Expand Down Expand Up @@ -1939,6 +1939,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 """
Expand Down Expand Up @@ -2291,7 +2296,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'))

Expand Down
4 changes: 2 additions & 2 deletions tmt/steps/provision/artemis.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
4 changes: 2 additions & 2 deletions tmt/steps/provision/connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions tmt/steps/provision/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions tmt/steps/provision/mrack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
4 changes: 2 additions & 2 deletions tmt/steps/provision/podman.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions tmt/steps/provision/testcloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -1010,9 +1010,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()
Expand Down
Loading

0 comments on commit 7cf7c3a

Please sign in to comment.