fixups and changes recommended by happz

docs/releases.rst

@@ -17,12 +17,13 @@ still has an impact on the parent test result. See also
 The beakerlib test framework tests now generate tmt subresults. The behavior is
 very similar to the shell test framework with ``tmt-report-result`` command
 calls (see above). The ``tmt-report-result`` now gets called with every
-``rlPhaseEnd`` macro and the tmt subresult gets created. The
-difference is that the subresults outcomes do not influence the parent result
-outcome in any way - the parent result outcome is always set to the outcome
-reported by the Beaker. The subresults are always assigned under the parent tmt
-result and can be easily showed e.g. by :ref:`/plugins/report/display` plugin
-when verbose mode is enabled.
+``rlPhaseEnd`` macro and the tmt subresult gets created. The difference is that
+the subresults outcomes are not evaluated by tmt. The tmt only captures them
+and then relies on a beakerlib and its result reporting, which does take
+the outcomes of phases into account to determine the final test outcome. The
+subresults are always assigned under the parent tmt result and can be easily
+showed e.g. by :ref:`/plugins/report/display` plugin when verbose mode is
+enabled.
 
 
 tmt-1.37.0

stories/features/report-result.fmf

@@ -16,9 +16,9 @@ description: |
     result will be the most severe rating. Available values ordered by severity
     are SKIP, PASS, WARN and FAIL.
 
-    When ``tmt-report-result`` is called, the return value of the test itself
-    is ignored, and only the results saved by ``tmt-report-result`` calls are
-    consumed by tmt.
+    Once the ``tmt-report-result`` command is called for the first time, the
+    return value of the test itself no longer matters, and only the results
+    saved by ``tmt-report-result`` command calls are consumed by tmt.
 
     __ https://restraint.readthedocs.io/en/latest/commands.html#rstrnt-report-result
 

tmt/frameworks/__init__.py

@@ -118,8 +118,7 @@ def extract_results(
         Extract test results.
 
         :param invocation: test invocation to which the check belongs to.
-        :param results: list of results which will be optionally saved as subresults by a
-            framework.
+        :param results: current list of results as reported by a test
         :param logger: to use for logging.
         :returns: list of results produced by the given test.
         """

tmt/frameworks/beakerlib.py

@@ -67,7 +67,7 @@ def extract_results(
         """ Check result of a beakerlib test """
 
         # The outcome of a parent tmt result must be never modified based on subresults outcomes.
-        # The parent result outcome will be always set to outcome reported by a Beaker. The
+        # The parent result outcome will be always set to outcome reported by a beakerlib. The
         # subresults are there just to provide more detail.
         subresults = [r.to_subresult(invocation) for r in results]
 
@@ -82,7 +82,7 @@ def extract_results(
         beakerlib_results_filepath = invocation.path / 'TestResults'
 
         try:
-            bkr_results = invocation.phase.read(beakerlib_results_filepath, level=3)
+            beakerlib_results = invocation.phase.read(beakerlib_results_filepath, level=3)
         except tmt.utils.FileError:
             logger.debug(f"Unable to read '{beakerlib_results_filepath}'.", level=3)
             note = 'beakerlib: TestResults FileError'
@@ -94,10 +94,10 @@ def extract_results(
                 log=log,
                 subresult=subresults)]
 
-        search_result = re.search('TESTRESULT_RESULT_STRING=(.*)', bkr_results)
+        search_result = re.search('TESTRESULT_RESULT_STRING=(.*)', beakerlib_results)
         # States are: started, incomplete and complete
         # FIXME In quotes until beakerlib/beakerlib/pull/92 is merged
-        search_state = re.search(r'TESTRESULT_STATE="?(\w+)"?', bkr_results)
+        search_state = re.search(r'TESTRESULT_STATE="?(\w+)"?', beakerlib_results)
 
         if search_result is None or search_state is None:
             # Same outcome but make it easier to debug

tmt/frameworks/shell.py

@@ -111,28 +111,28 @@ def extract_results(
         assert invocation.return_code is not None
         note = None
 
+        # Handle the `tmt-report-result` command results as a single test with assigned tmt
+        # subresults.
+        if results:
+            return cls._process_results_reduce(invocation, results)
+
         # If no extra results were passed (e.g. `tmt-report-result` was not called during the
         # test), just process the exit code of a shell test and return the result.
-        if not results:
-            try:
-                # Process the exit code and prepare the log path
-                result = {0: ResultOutcome.PASS, 1: ResultOutcome.FAIL}[invocation.return_code]
-            except KeyError:
-                result = ResultOutcome.ERROR
-                # Add note about the exceeded duration
-                if invocation.return_code == tmt.utils.ProcessExitCodes.TIMEOUT:
-                    note = 'timeout'
-                    invocation.phase.timeout_hint(invocation)
-
-                elif tmt.utils.ProcessExitCodes.is_pidfile(invocation.return_code):
-                    note = 'pidfile locking'
-
-            return [tmt.Result.from_test_invocation(
-                invocation=invocation,
-                result=result,
-                log=[invocation.relative_path / tmt.steps.execute.TEST_OUTPUT_FILENAME],
-                note=note)]
+        try:
+            # Process the exit code and prepare the log path
+            result = {0: ResultOutcome.PASS, 1: ResultOutcome.FAIL}[invocation.return_code]
+        except KeyError:
+            result = ResultOutcome.ERROR
+            # Add note about the exceeded duration
+            if invocation.return_code == tmt.utils.ProcessExitCodes.TIMEOUT:
+                note = 'timeout'
+                invocation.phase.timeout_hint(invocation)
+
+            elif tmt.utils.ProcessExitCodes.is_pidfile(invocation.return_code):
+                note = 'pidfile locking'
 
-        # Handle the `tmt-report-result` command results as a single test with assigned tmt
-        # subresults.
-        return cls._process_results_reduce(invocation, results)
+        return [tmt.Result.from_test_invocation(
+            invocation=invocation,
+            result=result,
+            log=[invocation.relative_path / tmt.steps.execute.TEST_OUTPUT_FILENAME],
+            note=note)]

tmt/steps/execute/__init__.py

@@ -763,11 +763,7 @@ def extract_custom_results(self, invocation: TestInvocation) -> list["tmt.Result
         return self._process_results_partials(invocation, collection.results)
 
     def extract_tmt_report_results(self, invocation: TestInvocation) -> list["tmt.Result"]:
-        """
-        Extract results from the file generated by ``tmt-report-result`` script.
-
-        All recorded results are reduced to one result eventually.
-        """
+        """ Extract results from a file generated by ``tmt-report-result`` script """
 
         collection = self._load_tmt_report_results_file(invocation)
 
@@ -837,16 +833,13 @@ def extract_results(
                 default_log=invocation.relative_path / TEST_OUTPUT_FILENAME)
 
         # Load the results from the `tmt-report-results.yaml` if a file was generated.
-        tmt_report_results = []
+        results = []
         if self._tmt_report_results_filepath(invocation).exists():
-            tmt_report_results = self.extract_tmt_report_results(invocation)
+            results = self.extract_tmt_report_results(invocation)
 
-        # Propagate loaded `tmt_report_results` to test framework, which will handle these results
-        # accordingly (e.g. saves them as a tmt subresults).
-        return invocation.test.test_framework.extract_results(
-            invocation=invocation,
-            results=tmt_report_results,
-            logger=logger)
+        # Propagate loaded `results` to test framework, which will handle these results accordingly
+        # (e.g. saves them as a tmt subresults).
+        return invocation.test.test_framework.extract_results(invocation, results, logger)
 
     def check_abort_file(self, invocation: TestInvocation) -> bool:
         """