Auto-verify artifact packages against require/recommend by vaibhavdaren · Pull Request #4726 · teemtee/tmt

vaibhavdaren · 2026-03-20T12:08:57Z

When the artifact plugin is present with verify=True (default), a verify-installation phase is automatically injected at order=79. It intersects require/recommend packages for the current guest with packages listed in artifacts.yaml, and verifies that the overlapping packages were installed from the tmt-artifact-shared repository.

Related to #4646

When the artifact plugin is present with verify=True (default), a verify-installation phase is automatically injected at order=79. It intersects require/recommend packages for the current guest with packages listed in artifacts.yaml, and verifies that the overlapping packages were installed from the tmt-artifact-shared repository.

gemini-code-assist

Code Review

The pull request introduces an automatic verification phase for artifact packages, ensuring that packages listed in require/recommend are installed from the tmt-artifact-shared repository when the artifact plugin is enabled with verify=True. This is a valuable addition for ensuring test environment integrity. The implementation correctly handles the intersection of artifact metadata and test requirements, and prioritizes explicit verify-installation configurations over auto-detected ones. The use of Final for constants in tmt/steps/prepare/artifact/__init__.py is a good practice.

gemini-code-assist · 2026-03-20T12:13:00Z

tmt/steps/prepare/__init__.py

+                    {dest for phase in artifact_phases_with_verify for dest in phase.data.where}
+                )
+            verify_data = PrepareVerifyInstallationData(
+                name='verify-artifact-packages',


Define name as a constant. This improves maintainability by centralizing the string literal, making it easier to update and ensuring consistency if referenced elsewhere.

gemini-code-assist · 2026-03-20T12:13:00Z

tmt/steps/prepare/__init__.py

+            verify_data = PrepareVerifyInstallationData(
+                name='verify-artifact-packages',
+                how='verify-installation',
+                summary='Verify packages were installed from artifact repositories',


Define summary as a constant. This improves maintainability by centralizing the string literal, making it easier to update and ensuring consistency if referenced elsewhere.

LecrisUT · 2026-03-20T12:17:55Z

tmt/steps/prepare/__init__.py

+        if artifact_phases_with_verify and not has_verify_phase:
+            # Mirror the where= scope of the artifact phases: if any artifact
+            # phase runs on all guests (empty where), verify should too;
+            # otherwise restrict to the union of their guest/role targets.
+            if any(not phase.data.where for phase in artifact_phases_with_verify):
+                verify_where: list[str] = []
+            else:
+                verify_where = list(
+                    {dest for phase in artifact_phases_with_verify for dest in phase.data.where}
+                )
+            verify_data = PrepareVerifyInstallationData(
+                name='verify-artifact-packages',
+                how='verify-installation',
+                summary='Verify packages were installed from artifact repositories',
+                order=tmt.steps.PHASE_ORDER_PREPARE_VERIFY_INSTALLATION,
+                auto=True,
+                where=verify_where,
+            )
+            self._phases.append(PreparePlugin.delegate(self, data=verify_data))


Uph, please not like this. Would rather it be created inside the artifact plugin instead, where we would also control how it is created. The questionable thing is if can be created while in the prepare.go phase, but that needs to be solved regardless because we do not know the packages and their origins until PrepareArtifact (probably all of them) complete.

For reference, see Discover dist-git in how it inserts the phase dynamically

It does, but it's a discover phase adding to prepare step that haven't started yet. prepare phase adding to prepare step, after the queue has been established and started is not supported. Right now, I would do it in the prepare step, just as test requirements are sorted out there. It's not ideal, but it's available today, and can be improved independently later.

Uph. At the minimum, encapsulate it in a function, and document to get rid of it as soon as possible.

On second thought, I still don't like it. What does it take to inject the phase in the queue properly and improperly?

Changing how queues are populated in steps, adding API to support dynamic additions while the queue is already running, which most likely requires reordering of the queued tasks as tasks were enqueued according to phases' order keys. I don't mind working on that, already some needed work landed, it just won't be ready today.

And a unit test, to make sure it works because this would be rather a rare use case. If there's a bug in how queue works, we'd see tmt explode everywhere; if there's a bug in how tasks are enqueued from another tasks of the same step, only a handful of tests would fail, so this calls for having a test as well.

We did create the interface in preparation for it at least

tmt/tmt/steps/__init__.py

Lines 1322 to 1330 in b32fa85

def add_phase(self, phase: Phase) -> None:

"""

Add a phase dynamically to the current step.

:param phase: The phase to add.

"""

if not isinstance(phase, self._plugin_base_class):

raise GeneralError(f"The phase '{phase}' is not part of step '{self}'.")

self._phases.append(phase)

LecrisUT · 2026-03-20T13:37:13Z

tmt/steps/prepare/verify_installation.py

+    def _build_auto_verify(self, guest: Guest) -> dict[str, str]:
+        """
+        Build a verify mapping from artifact metadata and test requirements.
+
+        Reads ``artifacts.yaml`` from the plan workdir, collects all
+        package names provided by artifact providers, and intersects them
+        with packages listed in ``require``/``recommend`` of tests enabled
+        on this guest.  Returns a mapping of intersecting package names to
+        the artifact shared repository name.
+        """


Well, the second issue with not being able to inject the phase is having this magic method. Didn't like the branching in discover, and this just adds another flavor of it. This wouldn't even work if you have one artifact provider with verify and another without since it cannot discriminate between them, unless there is even more introspection between these phases

vaibhavdaren self-assigned this Mar 20, 2026

vaibhavdaren added step | prepare Stuff related to the prepare step ci | full test Pull request is ready for the full test execution plugin | artifact Related to the `prepare/artifact` plugin. labels Mar 20, 2026

vaibhavdaren added this to planning Mar 20, 2026

github-project-automation bot moved this to backlog in planning Mar 20, 2026

vaibhavdaren linked an issue Mar 20, 2026 that may be closed by this pull request

Link the verify install plugin to the artifact plugin #4646

Open

vaibhavdaren mentioned this pull request Mar 20, 2026

Link the verify install plugin to the artifact plugin #4646

Open

gemini-code-assist bot reviewed Mar 20, 2026

View reviewed changes

LecrisUT reviewed Mar 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto-verify artifact packages against require/recommend#4726

Auto-verify artifact packages against require/recommend#4726
vaibhavdaren wants to merge 1 commit intomainfrom
vaibhav-verify-artifact-plugin

vaibhavdaren commented Mar 20, 2026 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Mar 20, 2026

Uh oh!

gemini-code-assist bot Mar 20, 2026

Uh oh!

LecrisUT Mar 20, 2026

Uh oh!

LecrisUT Mar 20, 2026

Uh oh!

happz Mar 20, 2026

Uh oh!

LecrisUT Mar 20, 2026

Uh oh!

LecrisUT Mar 20, 2026

Uh oh!

happz Mar 20, 2026

Uh oh!

happz Mar 20, 2026

Uh oh!

LecrisUT Mar 20, 2026

Uh oh!

LecrisUT Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	def add_phase(self, phase: Phase) -> None:
	"""
	Add a phase dynamically to the current step.

	:param phase: The phase to add.
	"""
	if not isinstance(phase, self._plugin_base_class):
	raise GeneralError(f"The phase '{phase}' is not part of step '{self}'.")
	self._phases.append(phase)

Conversation

vaibhavdaren commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vaibhavdaren commented Mar 20, 2026 •

edited

Loading