squash: save disabled phases as well, refactoring

Author: therazix

tests/recipe/data/import.yaml

@@ -69,20 +69,59 @@ plans:
             result: respect
     provision:
         enabled: false
-        phases: []
+        phases:
+          - name: default-0
+            how: local
+            order: 50
+            become: false
     prepare:
         enabled: false
-        phases: []
+        phases:
+          - name: enable-latest-copr
+            how: install
+            order: 50
+            summary: Enable copr repo with the latest tmt bits
+            copr:
+              - '@teemtee/latest'
+            missing: fail
+          - name: disable-tag-repo
+            how: shell
+            order: 50
+            summary: Completely disable the tag repository
+            script:
+              - "dnf repolist | grep testing-farm-tag-repository || exit 0\nif command
+                -v dnf5; then\n    dnf config-manager setopt testing-farm-tag-repository.enabled=0\n\
+                else\n    dnf config-manager --set-disabled testing-farm-tag-repository\n\
+                fi\n"
     execute:
         enabled: false
-        phases: []
+        phases:
+          - name: default-0
+            how: tmt
+            order: 50
+            duration: 1h
+            ignore-duration: false
+            exit-first: false
+            interactive: false
+            restraint-compatible: false
+            no-progress-bar: false
         results-path: plans/import/execute/results.yaml
     report:
         enabled: false
-        phases: []
+        phases:
+          - name: default-0
+            how: display
+            order: 50
+            display-guest: auto
     finish:
         enabled: false
-        phases: []
+        phases:
+          - name: default-0
+            how: shell
+            order: 50
     cleanup:
         enabled: false
-        phases: []
+        phases:
+          - name: default-0
+            how: tmt
+            order: 50

tmt/base/core.py

@@ -2553,13 +2553,9 @@ def plans(
 
         # Handle defaults, apply possible command line options
         logger = logger or (run._logger if run is not None else self._logger)
-        if run is not None and run.recipe:
-            names = names or [plan.name for plan in run.recipe.plans]
-            local_plan_keys = []
-        else:
-            names = names or []
-            local_plan_keys = (keys or []) + ['execute']
+        local_plan_keys = (keys or []) + ['execute']
         remote_plan_keys = (keys or []) + ['plan']
+        names = names or []
         filters = filters or []
         conditions = conditions or []
         # FIXME: cast() - typeless "dispatcher" method
@@ -2923,10 +2919,9 @@ def __init__(
 
         self.policies = policies or []
         self.recipe_manager = RecipeManager(logger)
-        self.recipe = self.recipe_manager.load(recipe_path) if recipe_path else None
-        if self.recipe is not None and self._tree is not None:
-            self.recipe_manager.update_tree(self.recipe, self._tree.tree)
-            self.remove = self.remove or self.recipe.run.remove
+        self.recipe = self.recipe_manager.load(
+            self, recipe_path, self._tree.tree if self._tree is not None else None
+        )
 
     @property
     def run_workdir(self) -> Path:
@@ -3037,6 +3032,17 @@ def _environment_from_cli(self) -> Environment:
             logger=self._logger,
         )
 
+    @property
+    def _environment_from_recipe(self) -> Environment:
+        """
+        Environment variables from the recipe.
+        """
+
+        if self.recipe is None:
+            return Environment()
+
+        return self.recipe.run.environment.copy()
+
     @property
     def environment(self) -> Environment:
         """
@@ -3047,21 +3053,15 @@ def environment(self) -> Environment:
 
         * run's environment, saved from the previous runs in the same
           workdir,
+        * run's environment, loaded from the recipe file.
         * run's environment, ``--environment`` and ``--environment-file``
           options.
         """
 
-        if self.recipe is not None:
-            return Environment(
-                {
-                    **self.recipe.run.environment,
-                    **self._environment_from_cli,
-                }
-            )
-
         return Environment(
             {
                 **self._environment_from_workdir,
+                **self._environment_from_recipe,
                 **self._environment_from_cli,
             }
         )

tmt/recipe.py

@@ -1,6 +1,6 @@
 from typing import TYPE_CHECKING, Any, Callable, Optional, TypedDict, cast
 
-import fmf
+from fmf import Tree
 
 import tmt.utils
 from tmt.checks import Check, _RawCheck, normalize_test_checks
@@ -14,7 +14,7 @@
 )
 from tmt.log import Logger
 from tmt.result import ResultInterpret
-from tmt.steps import Step, _RawStepData
+from tmt.steps import STEPS, Step, _RawStepData
 from tmt.steps.discover import Discover, TestOrigin
 from tmt.utils import Common, Environment, FmfContext, NormalizeKeysMixin, Path, ShellScript
 
@@ -276,17 +276,18 @@ class _RecipeStep(SpecBasedContainer[_RawRecipeStep, _RawRecipeStep], Serializab
 
     @classmethod
     def from_step(cls, step: 'Step') -> '_RecipeStep':
-        enabled = bool(step.enabled)
         return _RecipeStep(
-            enabled=enabled,
-            phases=[phase.to_minimal_spec() for phase in step.data] if enabled else [],
+            enabled=bool(step.enabled),
+            phases=[phase.to_minimal_spec() for phase in step.data],
         )
 
     # ignore[override]: does not match the signature on purpose, we need to pass logger
     @classmethod
     def from_spec(cls, spec: _RawRecipeStep, logger: Logger) -> '_RecipeStep':  # type: ignore[override]
-        enabled = bool(spec.get('enabled', False))
-        return _RecipeStep(enabled=enabled, phases=spec.get('phases', []) if enabled else [])
+        return _RecipeStep(
+            enabled=bool(spec.get('enabled', False)),
+            phases=spec.get('phases', []),
+        )
 
     def to_spec(self) -> _RawRecipeStep:
         return _RawRecipeStep(enabled=self.enabled, phases=self.phases)
@@ -316,10 +317,9 @@ def to_spec(self) -> _RawRecipeStep:
     # ignore[override]: does not match the signature on purpose, we need to pass logger
     @classmethod
     def from_spec(cls, spec: _RawRecipeStep, logger: Logger) -> '_RecipeDiscoverStep':  # type: ignore[override]
-        enabled = bool(spec.get('enabled', False))
         return _RecipeDiscoverStep(
-            enabled=enabled,
-            phases=spec.get('phases', []) if enabled else [],
+            enabled=bool(spec.get('enabled', False)),
+            phases=spec.get('phases', []),
             tests=[
                 _RecipeTest.from_spec(test, logger)
                 for test in cast(list[_RawRecipeTest], spec.get('tests', []))
@@ -329,10 +329,9 @@ def from_spec(cls, spec: _RawRecipeStep, logger: Logger) -> '_RecipeDiscoverStep
     @classmethod
     def from_step(cls, step: 'Step') -> '_RecipeDiscoverStep':
         assert isinstance(step, Discover)
-        enabled = bool(step.enabled)
         return _RecipeDiscoverStep(
-            enabled=enabled,
-            phases=[phase.to_minimal_spec() for phase in step.data] if enabled else [],
+            enabled=bool(step.enabled),
+            phases=[phase.to_minimal_spec() for phase in step.data],
             tests=[_RecipeTest.from_test_origin(test_origin) for test_origin in step.tests()],
         )
 
@@ -354,20 +353,18 @@ def to_spec(self) -> _RawRecipeStep:
     # ignore[override]: does not match the signature on purpose, we need to pass logger
     @classmethod
     def from_spec(cls, spec: _RawRecipeStep, logger: Logger) -> '_RecipeExecuteStep':  # type: ignore[override]
-        enabled = bool(spec.get('enabled', False))
         results_path = cast(Optional[str], spec.get('results-path', None))
         return _RecipeExecuteStep(
-            enabled=enabled,
-            phases=spec.get('phases', []) if enabled else [],
+            enabled=bool(spec.get('enabled', False)),
+            phases=spec.get('phases', []),
             results_path=Path(results_path) if results_path else None,
         )
 
     @classmethod
     def from_step(cls, step: 'Step') -> '_RecipeExecuteStep':
-        enabled = bool(step.enabled)
         return _RecipeExecuteStep(
-            enabled=enabled,
-            phases=[phase.to_minimal_spec() for phase in step.data] if enabled else [],
+            enabled=bool(step.enabled),
+            phases=[phase.to_minimal_spec() for phase in step.data],
             results_path=(step.step_workdir / 'results.yaml').relative_to(step.run_workdir),
         )
 
@@ -549,23 +546,25 @@ def to_spec(self) -> _RawRecipe:
             'plans': [plan.to_minimal_spec() for plan in self.plans],
         }
 
-    def get_plan_by_name(self, name: str) -> _RecipePlan:
-        plans = [plan for plan in self.plans if plan.name == name]
-        if len(plans) != 1:
-            raise tmt.utils.GeneralError(
-                f"Unable to find the correct plan in the recipe: '{name}'"
-            )
-        return plans[0]
-
 
 class RecipeManager(Common):
     def __init__(self, logger: Logger):
         super().__init__(logger=logger)
 
-    def load(self, path: Path) -> Recipe:
-        return Recipe.from_spec(
-            cast(_RawRecipe, tmt.utils.yaml_to_dict(self.read(path))), self._logger
+    def load(
+        self, run: 'Run', recipe_path: Optional[Path], fmf_tree: Optional[Tree]
+    ) -> Optional[Recipe]:
+        if recipe_path is None or fmf_tree is None:
+            return None
+
+        recipe = Recipe.from_spec(
+            cast(_RawRecipe, tmt.utils.yaml_to_dict(self.read(recipe_path))), self._logger
         )
+        self._update_tree(recipe, fmf_tree)
+        self._update_cli_context(recipe)
+        run.remove = run.remove or recipe.run.remove
+
+        return recipe
 
     def save(self, run: 'Run') -> None:
         recipe = Recipe(
@@ -596,9 +595,21 @@ def tests(self, recipe: Recipe, plan_name: str) -> list[TestOrigin]:
         raise tmt.utils.GeneralError(f"Plan '{plan_name}' not found in the recipe.")
 
     @staticmethod
-    def update_tree(recipe: Recipe, tree: fmf.Tree) -> None:
+    def _update_tree(recipe: Recipe, tree: Tree) -> None:
         """
         Load the plans from the recipe and update the given fmf tree with their specifications.
         """
         tree.children.clear()
         tree.update({plan.name: plan.to_fmf_spec() for plan in recipe.plans})
+
+    def _update_cli_context(self, recipe: Recipe) -> None:
+        """
+        Load the names of enabled steps from the recipe and save them to the CLI context.
+        """
+        if self._cli_context_object is None:
+            return
+        for plan in recipe.plans:
+            for step_name in STEPS:
+                recipe_step = plan.get_step_by_name(step_name)
+                if recipe_step.enabled:
+                    self._cli_context_object.steps.add(step_name)

tmt/steps/__init__.py

@@ -747,11 +747,6 @@ def enabled(self) -> Optional[bool]:
         if self.plan.my_run is None or self.plan.my_run._cli_context_object is None:
             return None
 
-        if self.plan.my_run.recipe:
-            plan = self.plan.my_run.recipe.get_plan_by_name(self.plan.name)
-            step = plan.get_step_by_name(self.step_name)
-            return step.enabled
-
         return self.name in self.plan.my_run._cli_context_object.steps
 
     @functools.cached_property