Simplify Result implementation

* drop `ResultData`, it's no longer needed and `Result` can handle all
  the work;
* add `guest` key as proposed in [1];
* add schema for validation of (custom) `results.yaml`;
* `execute` step saves `results.yaml` as a list of results rather than a
  mapping, to make allow one test being executed multiple times. See [2].

[1] #1614 (comment)
[2] #1614

tests/execute/result/custom/wrong_results.yaml

@@ -1,5 +1,5 @@
 # TODO: Make also this test working - JSON scheme needs to be defined for
-# ResultData to prevent invalid key 'foo'
+# Result to prevent invalid key 'foo'
 #
 #- name: /test/wrong-key
 #  result: pass

tests/unit/test_report_junit.py

@@ -5,7 +5,7 @@
 
 import pytest
 
-from tmt.result import Result, ResultData, ResultOutcome
+from tmt.result import Result, ResultOutcome
 from tmt.steps.report.junit import ReportJUnit, ReportJUnitData
 
 
@@ -127,13 +127,7 @@ def assert_xml(actual_filepath, expected):
 class TestStateMapping:
     def test_pass(self, report_fix):
         report, results, out_file_path = report_fix
-        results.extend(
-            [
-                Result(
-                    data=ResultData(result=ResultOutcome.PASS),
-                    name="/pass")
-                ]
-            )
+        results.append(Result(result=ResultOutcome.PASS, name="/pass"))
 
         report.go()
 
@@ -147,13 +141,7 @@ def test_pass(self, report_fix):
 
     def test_info(self, report_fix):
         report, results, out_file_path = report_fix
-        results.extend(
-            [
-                Result(
-                    data=ResultData(result=ResultOutcome.INFO),
-                    name="/info")
-                ]
-            )
+        results.append(Result(result=ResultOutcome.INFO, name="/info"))
         report.go()
 
         assert_xml(out_file_path, """<?xml version="1.0" ?>
@@ -168,13 +156,7 @@ def test_info(self, report_fix):
 
     def test_warn(self, report_fix):
         report, results, out_file_path = report_fix
-        results.extend(
-            [
-                Result(
-                    data=ResultData(result=ResultOutcome.WARN),
-                    name="/warn")
-                ]
-            )
+        results.append(Result(result=ResultOutcome.WARN, name="/warn"))
         report.go()
 
         assert_xml(out_file_path, """<?xml version="1.0" ?>
@@ -189,13 +171,7 @@ def test_warn(self, report_fix):
 
     def test_error(self, report_fix):
         report, results, out_file_path = report_fix
-        results.extend(
-            [
-                Result(
-                    data=ResultData(result=ResultOutcome.ERROR),
-                    name="/error")
-                ]
-            )
+        results.append(Result(result=ResultOutcome.ERROR, name="/error"))
         report.go()
 
         assert_xml(out_file_path, """<?xml version="1.0" ?>
@@ -210,13 +186,7 @@ def test_error(self, report_fix):
 
     def test_fail(self, report_fix):
         report, results, out_file_path = report_fix
-        results.extend(
-            [
-                Result(
-                    data=ResultData(result=ResultOutcome.FAIL),
-                    name="/fail")
-                ]
-            )
+        results.append(Result(result=ResultOutcome.FAIL, name="/fail"))
         report.go()
 
         assert_xml(out_file_path, """<?xml version="1.0" ?>

tmt/result.py

@@ -1,7 +1,7 @@
 import dataclasses
 import enum
 import re
-from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union, cast
+from typing import TYPE_CHECKING, Dict, List, Optional, cast
 
 import click
 import fmf
@@ -64,29 +64,16 @@ def is_result_outcome(cls, value: 'ResultInterpret') -> bool:
 
 
 @dataclasses.dataclass
-class ResultData(tmt.utils.SerializableContainer):
-    """
-    Formal data class containing information about result of the test
-    """
-    result: ResultOutcome = field(
-        default=ResultOutcome.PASS,
-        serialize=lambda result: result.value,
-        unserialize=ResultOutcome.from_spec
-        )
-    log: List[str] = field(default_factory=list)
-    note: Optional[str] = None
-    duration: Optional[str] = None
-    ids: Dict[str, Optional[str]] = field(default_factory=dict)
+class ResultGuestData(tmt.utils.SerializableContainer):
+    """ Describes what tmt knows about a guest the result was produced on """
 
+    name: Optional[str] = None
+    role: Optional[str] = None
 
-@dataclasses.dataclass(init=False)
-class Result(tmt.utils.SerializableContainer):
-    """
-    Test result
 
-    Required parameter 'data' needs to be type of ResultData.
-    Required parameter 'test' or 'name' should contain a test reference.
-    """
+@dataclasses.dataclass
+class Result(tmt.utils.SerializableContainer):
+    """ Describes what tmt knows about a single test result """
 
     name: str
     result: ResultOutcome = field(
@@ -97,75 +84,87 @@ class Result(tmt.utils.SerializableContainer):
     note: Optional[str] = None
     duration: Optional[str] = None
     ids: Dict[str, Optional[str]] = field(default_factory=dict)
-    log: Union[List[Any], Dict[Any, Any]] = field(default_factory=list)
-    # TODO: Check why log can be also a Dictionary.
-    # Check if we can safely get rid of Union.
-    # Should be the same type as in ResultData class.
-    # Check why there is a List[Any] and not a List[str]
-
-    def __init__(
-            self,
-            *,
-            data: ResultData,
-            name: Optional[str] = None,
-            test: Optional['tmt.base.Test'] = None) -> None:
-        """ Initialize test result data """
+    log: List[str] = field(default_factory=list)
+    guest: ResultGuestData = field(
+        default_factory=ResultGuestData,
+        serialize=lambda value: value.to_serialized(),  # type: ignore[attr-defined]
+        unserialize=lambda serialized: ResultGuestData.from_serialized(serialized)
+        )
 
+    @classmethod
+    def from_test(
+            cls,
+            *,
+            test: 'tmt.base.Test',
+            result: ResultOutcome,
+            note: Optional[str] = None,
+            duration: Optional[str] = None,
+            ids: Optional[Dict[str, Optional[str]]] = None,
+            log: Optional[List[str]] = None,
+            guest: Optional[ResultGuestData] = None) -> 'Result':
         from tmt.base import Test
-        super().__init__()
 
-        # Save the test name and optional note
-        if not test and not name:
-            raise tmt.utils.SpecificationError(
-                "Either name or test have to be specified")
-        if test and not isinstance(test, Test):
+        if not isinstance(test, Test):
             raise tmt.utils.SpecificationError(f"Invalid test '{test}'.")
-        if name and not isinstance(name, str):
-            raise tmt.utils.SpecificationError(f"Invalid test name '{name}'.")
-
-        # ignore[union-attr]: either `name` or `test` is set, we just
-        # made sure of it above, but mypy won't realize that.
-        self.name = name or test.name  # type: ignore[union-attr]
-        self.note = data.note
-        self.duration = data.duration
-        if test:
-            # Saving identifiable information for each test case so we can match them
-            # to Polarion/Nitrate/other cases and report run results there
-            # TODO: would an exception be better? Can test.id be None?
-            self.ids = {tmt.identifier.ID_KEY: test.id}
-            for key in EXTRA_RESULT_IDENTIFICATION_KEYS:
-                self.ids[key] = test.node.get(key)
-            interpret = ResultInterpret(test.result) if test.result else ResultInterpret.RESPECT
+
+        # Saving identifiable information for each test case so we can match them
+        # to Polarion/Nitrate/other cases and report run results there
+        # TODO: would an exception be better? Can test.id be None?
+        ids = ids or {}
+        default_ids = {
+            tmt.identifier.ID_KEY: test.id
+            }
+
+        for key in EXTRA_RESULT_IDENTIFICATION_KEYS:
+            default_ids[key] = test.node.get(key)
+
+        default_ids.update(ids)
+        ids = default_ids
+
+        _result = Result(
+            name=test.name,
+            result=result,
+            note=note,
+            duration=duration,
+            ids=ids,
+            log=log or [],
+            guest=guest or ResultGuestData()
+            )
+
+        return _result.interpret_result(
+            ResultInterpret(test.result) if test.result else ResultInterpret.RESPECT)
+
+    def interpret_result(self, interpret: ResultInterpret) -> 'Result':
+        if interpret in (ResultInterpret.RESPECT, ResultInterpret.CUSTOM):
+            return self
+
+        # Extend existing note or set a new one
+        if self.note and isinstance(self.note, str):
+            self.note += f', original result: {self.result.value}'
+
+        elif self.note is None:
+            self.note = f'original result: {self.result.value}'
+
         else:
-            self.ids = data.ids
-            interpret = ResultInterpret.RESPECT
-
-        self.result = data.result
-        self.log = data.log
-
-        # Handle alternative result interpretation
-        if interpret not in (ResultInterpret.RESPECT, ResultInterpret.CUSTOM):
-            # Extend existing note or set a new one
-            if self.note and isinstance(self.note, str):
-                self.note += f', original result: {self.result.value}'
-            elif self.note is None:
-                self.note = f'original result: {self.result.value}'
-            else:
-                raise tmt.utils.SpecificationError(
-                    f"Test result note '{self.note}' must be a string.")
-
-            if interpret == ResultInterpret.XFAIL:
-                # Swap just fail<-->pass, keep the rest as is (info, warn,
-                # error)
-                self.result = {
-                    ResultOutcome.FAIL: ResultOutcome.PASS,
-                    ResultOutcome.PASS: ResultOutcome.FAIL
-                    }.get(self.result, self.result)
-            elif ResultInterpret.is_result_outcome(interpret):
-                self.result = ResultOutcome(interpret.value)
-            else:
-                raise tmt.utils.SpecificationError(
-                    f"Invalid result '{interpret.value}' in test '{self.name}'.")
+            raise tmt.utils.SpecificationError(
+                f"Test result note '{self.note}' must be a string.")
+
+        if interpret == ResultInterpret.XFAIL:
+            # Swap just fail<-->pass, keep the rest as is (info, warn,
+            # error)
+            self.result = {
+                ResultOutcome.FAIL: ResultOutcome.PASS,
+                ResultOutcome.PASS: ResultOutcome.FAIL
+                }.get(self.result, self.result)
+
+        elif ResultInterpret.is_result_outcome(interpret):
+            self.result = ResultOutcome(interpret.value)
+
+        else:
+            raise tmt.utils.SpecificationError(
+                f"Invalid result '{interpret.value}' in test '{self.name}'.")
+
+        return self
 
     @staticmethod
     def total(results: List['Result']) -> Dict[ResultOutcome, int]:
@@ -206,21 +205,6 @@ def show(self) -> str:
         note = f" ({self.note})" if self.note else ''
         return f"{colored} {self.name}{note}"
 
-    @classmethod
-    def from_serialized(cls, serialized: Dict[str, Any]) -> 'Result':
-        # TODO: from_serialized() should trust the input. We should add its
-        # clone to read serialized-but-from-possibly-untrustworthy-source data -
-        # that's the place where the schema validation would happen in the
-        # future.
-
-        # Our special key may or may not be present, depending on who
-        # calls this method.  In any case, it is not needed, because we
-        # already know what class to restore: this one.
-        serialized.pop('__class__', None)
-
-        name = serialized.pop('name')
-        return cls(data=ResultData.from_serialized(serialized), name=name)
-
     @staticmethod
     def failures(log: Optional[str], msg_type: str = 'FAIL') -> str:
         """ Filter stdout and get only messages with certain type """

tmt/schemas/common.yaml

@@ -334,3 +334,26 @@ definitions:
   # https://tmt.readthedocs.io/en/stable/spec/plans.html#multihost
   where:
     $ref: "/schemas/common#/definitions/one_or_more_strings"
+
+  result_outcome:
+    type: string
+    enum:
+      - pass
+      - fail
+      - info
+      - warn
+      - error
+
+  # https://tmt.readthedocs.io/en/stable/spec/tests.html#result
+  result_interpret:
+    type: string
+
+    enum:
+      - respect
+      - xfail
+      - pass
+      - info
+      - warn
+      - error
+      - fail
+      - custom

tmt/schemas/results.yaml

@@ -0,0 +1,57 @@
+---
+
+#
+# JSON Schema definition for tmt `results.yaml` file
+#
+# https://tmt.readthedocs.io/en/stable/spec/plans.html#execute
+#
+
+$id: /schemas/test
+$schema: https://json-schema.org/draft-07/schema
+
+type: array
+items:
+  type: object
+  additionalProperties: false
+
+  properties:
+    guest:
+      type: object
+      additionalProperties: false
+
+      properties:
+        name:
+          type: string
+
+        role:
+          $ref: "/schemas/common#/definitions/role"
+
+      required: []
+      minProperties: 1
+
+    name:
+      type: string
+
+    result:
+      $ref: "/schemas/common#/definitions/result_outcome"
+
+    note:
+      type: string
+
+    duration:
+      $ref: "/schemas/common#/definitions/duration"
+
+    ids:
+      type: object
+      patternProperties:
+        ^.*$:
+          type: string
+
+    log:
+      type: array
+      items:
+        type: string
+
+  required:
+    - name
+    - result