Skip to content

Commit

Permalink
Move hints to their own place, with API and unicorns
Browse files Browse the repository at this point in the history
I wanted to change a couple of things about "hints" tmt shows to users
when when a package is missing and functionality is limited:

* Static, stored in a single function. That made them detached from
  their origin, and 3rd party plugins would have no chance to add
  their own hints.
* A bit confined text of hints did not cover all possible venues. PyPI
  installation was ignored by some, other hints spoke just about using
  `pip install`.
* Hints are interesting and useful, but visible only when error strikes.

The patch turns the function into a "registry", a simple dictionary
storing them. Plugins and tmt code in general can "register" their
hints, and a nice tools are available for showing them.

Hints wow have IDs, and there are dedicated IDs for step-specific
(``report``, ...) and plugin-specific (``report/foo``) hints, allowing
tmt core to print them when step or plugins crashes on import.

Plugin-specific hints are now rendered both in their CLI and HTML
documentation.

In the future, I would like to provide hints not just as "a package foo
is missing, install it" guide, but also explaining various errors and
issues tmt would report, e.g. pairing them with exceptions, "To learn
more, run `tmt about E1234`".
  • Loading branch information
happz committed Feb 3, 2025
1 parent 5bf4198 commit a010e3b
Show file tree
Hide file tree
Showing 14 changed files with 352 additions and 103 deletions.
8 changes: 8 additions & 0 deletions docs/_static/tmt-custom.css
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,11 @@
.logo {
padding: 10px 50px !important;
}

.rst-content .note .admonition-title {
display: block !important;
}

.rst-content .warning .admonition-title {
display: block !important;
}
2 changes: 2 additions & 0 deletions docs/scripts/generate-plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import tmt.steps.provision
import tmt.steps.report
import tmt.utils
import tmt.utils.hints
from tmt.utils import ContainerClass, Path
from tmt.utils.templates import render_template_file

Expand Down Expand Up @@ -195,6 +196,7 @@ def main() -> None:
STEP=step_name,
PLUGINS=plugin_generator,
REVIEWED_PLUGINS=REVIEWED_PLUGINS,
HINTS=tmt.utils.hints.HINTS,
is_enum=is_enum,
container_fields=tmt.utils.container_fields,
container_field=tmt.utils.container_field,
Expand Down
6 changes: 6 additions & 0 deletions docs/templates/plugins.rst.j2
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ The following keys are accepted by all plugins of the ``{{ STEP }}`` step.
{{ PLUGIN.__doc__ | dedent | trim }}
{% endif %}

{% if plugin_full_id in HINTS %}
.. note::

{{ HINTS[plugin_full_id] | indent(3, first=true) }}
{% endif %}

{% set intrinsic_fields = container_intrinsic_fields(PLUGIN_DATA_CLASS) | sort %}

{% if intrinsic_fields %}
Expand Down
3 changes: 1 addition & 2 deletions tmt/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3993,8 +3993,7 @@ def images(self) -> bool:
self.info('images', color='blue')
successful = True
for method in tmt.steps.provision.ProvisionPlugin.methods():
# FIXME: ignore[union-attr]: https://github.com/teemtee/tmt/issues/1599
if not method.class_.clean_images(self, self.is_dry_run): # type: ignore[union-attr]
if not method.class_.clean_images(self, self.is_dry_run): # type: ignore[attr-defined]
successful = False
return successful

Expand Down
63 changes: 12 additions & 51 deletions tmt/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,56 +427,6 @@ def common_decorator(fn: FC) -> FC:
return common_decorator


def show_step_method_hints(
step_name: str,
how: str,
logger: tmt.log.Logger) -> None:
"""
Show hints about available step methods' installation
The logger will be used to output the hints to the terminal, hence
it must be an instance of a subclass of tmt.utils.Common (info method
must be available).
"""

if how == 'ansible':
logger.info(
'hint', "Install 'ansible-core' to prepare "
"guests using ansible playbooks.", color='blue')
elif step_name == 'provision':
if how == 'virtual':
logger.info(
'hint', "Install 'tmt+provision-virtual' "
"to run tests in a virtual machine.", color='blue')
if how == 'container':
logger.info(
'hint', "Install 'tmt+provision-container' "
"to run tests in a container.", color='blue')
if how == 'minute':
logger.info(
'hint', "Install 'tmt-redhat-provision-minute' "
"to run tests in 1minutetip OpenStack backend. "
"(Available only from the internal COPR repository.)",
color='blue')
logger.info(
'hint', "Use the 'local' method to execute tests "
"directly on your localhost.", color='blue')
logger.info(
'hint', "See 'tmt run provision --help' for all "
"available provision options.", color='blue')
elif step_name == 'report':
if how == 'junit':
logger.info(
'hint', "Install 'tmt+report-junit' to write results "
"in JUnit format.", color='blue')
logger.info(
'hint', "Use the 'display' method to show test results "
"on the terminal.", color='blue')
logger.info(
'hint', "See 'tmt run report --help' for all "
"available report options.", color='blue')


def create_method_class(methods: MethodDictType) -> type[click.Command]:
"""
Create special class to handle different options for each method
Expand Down Expand Up @@ -599,10 +549,21 @@ def _find_how(args: list[str]) -> Optional[str]:
break

if how and self._method is None:
from tmt.utils.hints import print_hint

# Use run for logging, steps may not be initialized yet
assert context.obj.run is not None # narrow type
assert self.name is not None # narrow type
show_step_method_hints(self.name, how, context.obj.run._logger)

print_hint(
id_=f'{self.name}/{how}',
ignore_missing=True,
logger=context.obj.run._logger)
print_hint(
id_=self.name,
ignore_missing=True,
logger=context.obj.run._logger)

raise tmt.utils.SpecificationError(
f"Unsupported {self.name} method '{how}'.")

Expand Down
19 changes: 14 additions & 5 deletions tmt/plugins/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,8 @@ def _import_or_raise(
*,
module: str,
exc_class: type[BaseException],
exc_message: str,
exc_message: Optional[str] = None,
hint_id: Optional[str] = None,
logger: Logger) -> ModuleT: # type: ignore[type-var,misc]
"""
Import a module, or raise an exception.
Expand All @@ -247,7 +248,15 @@ def _import_or_raise(
return _import(module=module, logger=logger)

except tmt.utils.GeneralError as exc:
raise exc_class(exc_message) from exc
if hint_id is not None:
from tmt.utils.hints import print_hint

print_hint(id_=hint_id, logger=logger)

if exc_message is not None:
raise exc_class(exc_message) from exc

raise exc_class(f"Failed to import the '{module}' module.") from exc


# ignore[type-var,misc]: the actual type is provided by caller - the
Expand Down Expand Up @@ -385,10 +394,10 @@ def __init__(
self,
module: str,
exc_class: type[Exception],
exc_message: str) -> None:
hint_id: str) -> None:
self._module_name = module
self._exc_class = exc_class
self._exc_message = exc_message
self._hint_id = hint_id

self._module: Optional[ModuleT] = None

Expand All @@ -397,7 +406,7 @@ def __call__(self, logger: Logger) -> ModuleT:
self._module = _import_or_raise(
module=self._module_name,
exc_class=self._exc_class,
exc_message=self._exc_message,
hint_id=self._hint_id,
logger=logger)

assert self._module # narrow type
Expand Down
53 changes: 42 additions & 11 deletions tmt/steps/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import tmt.queue
import tmt.utils
import tmt.utils.rest
from tmt.options import option, show_step_method_hints
from tmt.options import option
from tmt.utils import (
DEFAULT_NAME,
Environment,
Expand Down Expand Up @@ -69,6 +69,8 @@

DEFAULT_ALLOWED_HOW_PATTERN: Pattern[str] = re.compile(r'.*')

_PLUGIN_CLASS_NAME_TO_STEP_PATTERN = re.compile(r'tmt.steps.([a-z]+)')

#
# Following are default and predefined order values for various special phases
# recognized by tmt. When adding new special phase, add its order below, and
Expand Down Expand Up @@ -1190,9 +1192,10 @@ class Method:
def __init__(
self,
name: str,
class_: Optional[PluginClass] = None,
class_: PluginClass,
doc: Optional[str] = None,
order: int = PHASE_ORDER_DEFAULT
order: int = PHASE_ORDER_DEFAULT,
installation_hint: Optional[str] = None
) -> None:
""" Store method data """

Expand All @@ -1204,6 +1207,10 @@ def __init__(

raise tmt.utils.GeneralError(f"Plugin method '{name}' provides no docstring.")

if installation_hint is not None:
doc = doc + '\n\n.. note::\n\n' + \
textwrap.indent(textwrap.dedent(installation_hint), ' ')

self.name = name
self.class_ = class_
self.doc = tmt.utils.rest.render_rst(doc, tmt.log.Logger.get_bootstrap_logger()) \
Expand Down Expand Up @@ -1236,7 +1243,8 @@ def usage(self) -> str:
def provides_method(
name: str,
doc: Optional[str] = None,
order: int = PHASE_ORDER_DEFAULT) -> Callable[[PluginClass], PluginClass]:
order: int = PHASE_ORDER_DEFAULT,
installation_hint: Optional[str] = None) -> Callable[[PluginClass], PluginClass]:
"""
A plugin class decorator to register plugin's method with tmt steps.
Expand All @@ -1256,18 +1264,29 @@ class SomePlugin(tmt.steps.discover.DicoverPlugin):
"""

def _method(cls: PluginClass) -> PluginClass:
plugin_method = Method(name, class_=cls, doc=doc, order=order)
plugin_method = Method(
name,
cls,
doc=doc,
order=order,
installation_hint=installation_hint)

# FIXME: make sure cls.__bases__[0] is really BasePlugin class
# TODO: BasePlugin[Any]: it's tempting to use StepDataT, but I was
# 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, Any]', cls.__bases__[0])._supported_methods \
.register_plugin(
plugin_id=name,
plugin=plugin_method,
logger=tmt.log.Logger.get_bootstrap_logger())
base_class = cast('BasePlugin[Any, Any]', cls.__bases__[0])

base_class._supported_methods.register_plugin(
plugin_id=name,
plugin=plugin_method,
logger=tmt.log.Logger.get_bootstrap_logger())

if installation_hint is not None:
from tmt.utils.hints import register_hint

register_hint(f'{base_class.get_step_name()}/{name}', installation_hint)

return cls

Expand Down Expand Up @@ -1309,6 +1328,14 @@ def get_data_class(cls) -> type[StepDataT]:

data: StepDataT

@classmethod
def get_step_name(cls) -> str:
match = _PLUGIN_CLASS_NAME_TO_STEP_PATTERN.match(cls.__module__)

assert match is not None # must be

return match.group(1)

# TODO: do we need this list? Can whatever code is using it use _data_class directly?
# List of supported keys
# (used for import/export to/from attributes during load and save)
Expand Down Expand Up @@ -1523,7 +1550,11 @@ def delegate(
assert isinstance(plugin, BasePlugin)
return plugin

show_step_method_hints(step.name, how, step._logger)
from tmt.utils.hints import print_hint

print_hint(id_=f'{step.name}/{how}', ignore_missing=True, logger=step._logger)
print_hint(id_=step.name, ignore_missing=True, logger=step._logger)

# Report invalid method
if step.plan is None:
raise tmt.utils.GeneralError(f"Plan for {step.name} is not set.")
Expand Down
6 changes: 4 additions & 2 deletions tmt/steps/provision/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
import tmt.steps.provision
import tmt.utils
from tmt.log import Logger
from tmt.options import option, show_step_method_hints
from tmt.options import option
from tmt.package_managers import FileSystemPath, Package, PackageManagerClass
from tmt.plugins import PluginRegistry
from tmt.steps import Action, ActionTask, PhaseQueue
Expand Down Expand Up @@ -1942,7 +1942,9 @@ def _run_ansible(
log=log)
except tmt.utils.RunError as exc:
if "File 'ansible-playbook' not found." in exc.message:
show_step_method_hints('plugin', 'ansible', self._logger)
from tmt.utils.hints import print_hint

print_hint(id_='ansible-not-available', logger=self._logger)
raise exc

@property
Expand Down
5 changes: 3 additions & 2 deletions tmt/steps/provision/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import tmt.steps
import tmt.steps.provision
import tmt.utils
from tmt.options import show_step_method_hints
from tmt.utils import Command, OnProcessStartCallback, Path, ShellScript


Expand Down Expand Up @@ -72,7 +71,9 @@ def _run_ansible(
silent=silent)
except tmt.utils.RunError as exc:
if exc.stderr and 'ansible-playbook: command not found' in exc.stderr:
show_step_method_hints('plugin', 'ansible', self._logger)
from tmt.utils.hints import print_hint

print_hint(id_='ansible-not-available', logger=self._logger)
raise exc

def execute(self,
Expand Down
21 changes: 18 additions & 3 deletions tmt/steps/provision/podman.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import tmt.steps
import tmt.steps.provision
import tmt.utils
from tmt.options import show_step_method_hints
from tmt.steps.provision import GuestCapability
from tmt.utils import Command, OnProcessStartCallback, Path, ShellScript, field, retry

Expand Down Expand Up @@ -286,7 +285,9 @@ def _run_ansible(
silent=silent)
except tmt.utils.RunError as exc:
if "File 'ansible-playbook' not found." in exc.message:
show_step_method_hints('plugin', 'ansible', self._logger)
from tmt.utils.hints import print_hint

print_hint(id_='ansible-not-available', logger=self._logger)
raise exc

def podman(
Expand Down Expand Up @@ -438,7 +439,21 @@ def remove(self) -> None:
raise err


@tmt.steps.provides_method('container')
@tmt.steps.provides_method(
'container',
installation_hint="""
Make sure ``podman`` is installed and configured, it is required for container-backed
guests provided by ``provision/container`` plugin.
To quickly test ``podman`` functionality, you can try running ``podman images`` or
``podman run --rm -it fedora:latest``.
* Users who installed tmt from system repositories should install
``tmt+provision-container`` package.
* Users who installed tmt from PyPI should also install ``tmt+provision-container``
package, as it will install required system dependencies. After doing so, they should
install ``tmt[provision-container]`` extra.
""")
class ProvisionPodman(tmt.steps.provision.ProvisionPlugin[ProvisionPodmanData]):
"""
Create a new container using ``podman``.
Expand Down
Loading

0 comments on commit a010e3b

Please sign in to comment.