To/from "state" helpers for tmt state files (#4486)

tmt saves its state info - step data, test data, guest data, etc. - in
YAML format, plent yof `*.yaml` files in a workdir. It's fine, it's
readable by both machines and humans, it works.

It turns out YAML is not necessarily the best format out there, as YAML
can be costly to generate when dealing with large structures and large
collections.

Two things happen in this PR:

* To allow some experimentation, `TMT_STATE_FORMAT` environment variable
tells tmt which format to use when storing its data. As of now, only
`yaml` is supported.
* "to/from JSON" helpers are refactored to align with recent changes to
"to/from YAML" helpers, to provide similar function names, annotations
and so on, and they are made available for selection via
`TMT_STATE_FORMAT`, making JSON the second supported format for tmt
state files.

Moar formats, moar fun!

A set of simple helpers, `to_state` and `from_state` is defined, based
on the aforementioned envvar, and patch changes all places where tmt
stores state on disk to use them instead of `{to,from}_yaml`.
`Common.{read,write}_state()` will help with that, instead of
`Common.{read,write}()` methods.

Now we can play with different format, and even if we learn the format
is not the culprit of ineffective storage of large collections of
results, we will have nice and tidy internal API to work with.

Author: happz

docs/overview.rst

@@ -620,6 +620,17 @@ TMT_POLICY_ROOT
 
 __ https://tmt.readthedocs.io/en/stable/spec/policy.html
 
+TMT_STATE_FORMAT
+    Which format should tmt use for its on-disk storage of various state
+    information like run or step data, or the collection of discovered
+    tests.
+
+    .. note::
+
+        This does not affect user-provided files, like test, plan and
+        story metadata in fmf files, as well as various exports and
+        conversions. It merely affects files tmt uses for its own needs.
+
 .. _step-variables:
 
 Step Variables

spec/results.fmf

@@ -3,6 +3,19 @@ story:
     document the format in which test results are saved on storage.
 
 description: |
+    .. note::
+
+        Name of the file holding results is affected by the choice of
+        the on-disk format of tmt state files. See
+        :ref:`TMT_STATE_FORMAT<command-variables>` environment variable
+        which controls the selection.
+
+        By default, YAML is used, and the file would be called
+        ``results.yaml``. The specification below will stick to this
+        setting, and will describe the format of results in terms of
+        YAML, but the specification is applicable to other formats
+        as well.
+
     The following text defines a YAML file structure tmt uses for storing
     results. tmt itself will use it when saving results of ``execute`` step,
     and custom test results are required to follow it when creating their

tests/execute/result/custom.sh

@@ -77,7 +77,7 @@ rlJournalStart
     testName="/test/wrong-json-results-file"
     rlPhaseStartTest "${testName}"
         rlRun -s "${tmt_command} ${testName} 2>&1 >/dev/null" 2 "Test provides 'results.json' in valid JSON but wrong results format"
-        rlAssertGrep "Expected list in json data, got 'dict'." $rlRun_LOG
+        rlAssertGrep "Expected list in JSON data, got 'dict'." $rlRun_LOG
     rlPhaseEnd
 
     testName="/test/invalid-yaml-results-file"
@@ -89,7 +89,7 @@ rlJournalStart
     testName="/test/invalid-json-results-file"
     rlPhaseStartTest "${testName}"
         rlRun -s "${tmt_command} ${testName} 2>&1 >/dev/null" 2 "Test provides 'results.json' not in JSON format"
-        rlAssertGrep "Invalid json syntax" $rlRun_LOG
+        rlAssertGrep "Invalid JSON syntax." $rlRun_LOG
     rlPhaseEnd
 
     testName="/test/wrong-yaml-content"

tmt/base/core.py

@@ -80,6 +80,7 @@
     HasRunWorkdir,
     Path,
     ShellScript,
+    StateFormat,
     WorkdirArgumentType,
     normalize_shell_script,
     to_yaml,
@@ -2934,6 +2935,74 @@ def run_workdir(self) -> Path:
 
         return self.workdir
 
+    @functools.cached_property
+    def state_format_marker_filepath(self) -> Path:
+        return self.run_workdir / 'state-format'
+
+    @functools.cached_property
+    def state_format(self) -> StateFormat:
+        try:
+            format_name = self.state_format_marker_filepath.read_text().strip()
+
+        except FileNotFoundError:
+            state_format = tmt.utils.get_state_format()
+
+            self.debug(
+                "No state format marker file found,"
+                f" using the default state format '{state_format.name}'."
+            )
+
+            return state_format
+
+        except Exception as exc:
+            raise GeneralError('Failed to read state format marker.') from exc
+
+        state_format = tmt.utils.get_state_format(format=format_name)
+
+        self.debug(
+            f"State format marker file found, using the '{state_format.name}' state format."
+        )
+
+        return state_format
+
+    def read_state(self, filepath: Path) -> Any:
+        """
+        Read a stored state from the given file.
+
+        .. important::
+
+            No deserialization is performed, it is the responsibility of the
+            caller to turn loaded structure, consisting of built-in-like
+            types, into objects of desired classes, e.g. by the power of
+            :py:meth:`tmt.container.SerializableContainer.deserialize`.
+
+        :param filepath: file to read the state from.
+        :returns: stored state as Python data structure.
+        """
+
+        return self.state_format.from_state(
+            self.read_file(Path(f'{filepath}{self.state_format.suffix}'))
+        )
+
+    def write_state(self, filepath: Path, data: Any) -> None:
+        """
+        Write a state into the given file.
+
+        .. important::
+
+            No serialization is performed, it is the responsibility of the
+            caller to turn internal objects into built-in-like Python types,
+            e.g. by the power of
+            :py:meth:`tmt.container.SerializableContainer.serialize`.
+
+        :param filepath: file to write the state into.
+        :param data: state as Python data structure.
+        """
+
+        return self.write_file(
+            Path(f'{filepath}{self.state_format.suffix}'), self.state_format.to_state(data)
+        )
+
     def load_workdir(self, *, with_logfiles: bool = True) -> None:
         """
         Prepare the run workdir and associated.
@@ -3078,16 +3147,21 @@ def save(self) -> None:
         """
         Save list of selected plans and enabled steps
         """
+
+        self.state_format_marker_filepath.unlink(missing_ok=True)
+        self.state_format_marker_filepath.write_text(self.state_format.name)
+
         assert self.tree is not None  # narrow type
         assert self._cli_context_object is not None  # narrow type
+        assert self.workdir is not None  # narrow type
         data = RunData(
             root=str(self.tree.root) if self.tree.root else None,
             plans=[plan.name for plan in self._plans] if self._plans is not None else None,
             steps=list(self._cli_context_object.steps),
             environment=self.environment,
             remove=self.remove,
         )
-        self.write(Path('run.yaml'), to_yaml(data.to_serialized()))
+        self.write_state(self.workdir / 'run', data.to_serialized())
 
     def load_from_workdir(self) -> None:
         """
@@ -3105,10 +3179,12 @@ def load_from_workdir(self) -> None:
         #  which in turn is only called by status and clean, both cases where we do not want
         #  to attach the logfile loggers to.
         self.load_workdir(with_logfiles=False)
+
+        assert self.workdir is not None  # narrow type
+
         try:
-            self.data = RunData.from_serialized(
-                tmt.utils.yaml_to_dict(self.read(Path('run.yaml')))
-            )
+            self.data = RunData.from_serialized(self.read_state(self.workdir / 'run'))
+
         except tmt.utils.FileError:
             self.debug('Run data not found.')
             return
@@ -3137,10 +3213,11 @@ def load(self) -> None:
         """
         from tmt.base.plan import Plan
 
+        assert self.workdir is not None  # narrow type
+
         try:
-            self.data = RunData.from_serialized(
-                tmt.utils.yaml_to_dict(self.read(Path('run.yaml')))
-            )
+            self.data = RunData.from_serialized(self.read_state(self.workdir / 'run'))
+
         except tmt.utils.FileError:
             self.debug('Run data not found.')
             return

tmt/cli/about.py

@@ -1,6 +1,5 @@
 """``tmt about`` implementation"""
 
-import json
 import re
 from typing import Any
 
@@ -73,7 +72,9 @@ def _ls(context: Context, how: str, content: Any) -> None:
         )
 
     elif how in ('json', 'yaml'):
-        context.obj.print(json.dumps(content) if how == 'json' else tmt.utils.to_yaml(content))
+        context.obj.print(
+            tmt.utils.to_json(content) if how == 'json' else tmt.utils.to_yaml(content)
+        )
 
 
 @about.group(invoke_without_command=True, cls=CustomGroup)

tmt/export/_json.py

@@ -1,5 +1,3 @@
-import json
-
 import tmt.base.core
 import tmt.base.plan
 import tmt.export
@@ -12,4 +10,4 @@
 class JSONExporter(tmt.export.TrivialExporter):
     @classmethod
     def _export(cls, data: tmt.export._RawExported) -> str:
-        return json.dumps(data)
+        return tmt.utils.to_json(data)

tmt/frameworks/beakerlib.py

@@ -174,7 +174,7 @@ def extract_results(
         beakerlib_results_filepath = invocation.path / 'TestResults'
 
         try:
-            beakerlib_results = invocation.phase.read(beakerlib_results_filepath, level=3)
+            beakerlib_results = invocation.phase.read(beakerlib_results_filepath, debug_level=3)
         except tmt.utils.FileError:
             logger.debug(f"Unable to read '{beakerlib_results_filepath}'.", level=3)
             note.append('beakerlib: TestResults FileError')

tmt/package_managers/bootc.py

@@ -12,14 +12,7 @@
     PackageManagerEngine,
     provides_package_manager,
 )
-from tmt.utils import (
-    Command,
-    CommandOutput,
-    GeneralError,
-    Path,
-    RunError,
-    ShellScript,
-)
+from tmt.utils import Command, CommandOutput, GeneralError, Path, RunError, ShellScript
 
 LOCALHOST_BOOTC_IMAGE_PREFIX = "localhost/tmt"
 

tmt/steps/__init__.py

@@ -598,7 +598,12 @@ def _preserved_workdir_members(self) -> set[str]:
         A set of members of the step workdir that should not be removed during pruning.
         """
 
-        return {'step.yaml'}
+        members: set[str] = set()
+
+        if self.plan.my_run:
+            members = {*members, f'step{self.plan.my_run.state_format.suffix}'}
+
+        return members
 
     def _check_duplicate_names(self, raw_data: list[_RawStepData]) -> None:
         """
@@ -843,7 +848,9 @@ def load(self) -> None:
         """
 
         try:
-            raw_step_data: dict[Any, Any] = tmt.utils.yaml_to_dict(self.read(Path('step.yaml')))
+            assert self.plan.my_run is not None  # narrow type
+
+            raw_step_data: dict[Any, Any] = self.plan.my_run.read_state(self.step_workdir / 'step')
 
         except tmt.utils.GeneralError:
             self.debug('Step data not found.', level=2)
@@ -871,7 +878,9 @@ def save(self) -> None:
             'status': self.status(),
             'data': [datum.to_serialized() for datum in self.data],
         }
-        self.write(Path('step.yaml'), tmt.utils.to_yaml(content))
+
+        assert self.plan.my_run is not None  # narrow type
+        self.plan.my_run.write_state(self.step_workdir / 'step', content)
 
     def _load_results(
         self,
@@ -882,8 +891,10 @@ def _load_results(
         Load results of this step from the workdir
         """
 
+        assert self.plan.my_run is not None  # narrow type
+
         try:
-            raw_results: list[Any] = tmt.utils.yaml_to_list(self.read(Path('results.yaml')))
+            raw_results: list[Any] = self.plan.my_run.read_state(self.step_workdir / 'results')
 
             return [result_class.from_serialized(raw_result) for raw_result in raw_results]
 
@@ -902,10 +913,12 @@ def _save_results(self, results: Sequence['BaseResult']) -> None:
         Save results of this step to the workdir
         """
 
+        assert self.plan.my_run is not None  # narrow type
+
         try:
             raw_results = [result.to_serialized() for result in results]
 
-            self.write(Path('results.yaml'), tmt.utils.to_yaml(raw_results))
+            self.plan.my_run.write_state(self.step_workdir / 'results', raw_results)
 
         except Exception as exc:
             raise GeneralError('Cannot save step results.') from exc

tmt/steps/cleanup/__init__.py

@@ -77,7 +77,14 @@ def _preserved_workdir_members(self) -> set[str]:
         A set of members of the step workdir that should not be removed.
         """
 
-        return {*super()._preserved_workdir_members, 'results.yaml'}
+        members = {
+            *super()._preserved_workdir_members,
+        }
+
+        if self.plan.my_run:
+            members = {*members, f'results{self.plan.my_run.state_format.suffix}'}
+
+        return members
 
     def wake(self) -> None:
         """