Clear test invocation data path use and derived paths

There was a "data path" tied to a test invocation, then there was a
"test data path" which was the "data path" plus one more `/data`. The
patch turns `TestInvocation.data_path()` into `TestInvocation.path`,
adds caching and defines the test data path. Together, they should
provide clearer view on paths related to a test and its invocations,
simplifying the related code.

tmt/frameworks/beakerlib.py

@@ -23,7 +23,7 @@ def get_environment_variables(
             logger: tmt.log.Logger) -> tmt.utils.EnvironmentType:
 
         return {
-            'BEAKERLIB_DIR': str(invocation.data_path(full=True)),
+            'BEAKERLIB_DIR': str(invocation.path),
             'BEAKERLIB_COMMAND_SUBMIT_LOG': f'bash {tmt.steps.execute.TMT_FILE_SUBMIT_SCRIPT.path}'
             }
 
@@ -34,7 +34,7 @@ def get_pull_options(
             logger: tmt.log.Logger) -> list[str]:
         return [
             '--exclude',
-            str(invocation.data_path("backup*", full=True))
+            str(invocation.path / "backup*")
             ]
 
     @classmethod
@@ -47,11 +47,11 @@ def extract_results(
         note: Optional[str] = None
         log: list[Path] = []
         for filename in [tmt.steps.execute.TEST_OUTPUT_FILENAME, 'journal.txt']:
-            if invocation.data_path(filename, full=True).is_file():
-                log.append(invocation.data_path(filename))
+            if (invocation.path / filename).is_file():
+                log.append(invocation.path / filename)
 
         # Check beakerlib log for the result
-        beakerlib_results_filepath = invocation.data_path('TestResults', full=True)
+        beakerlib_results_filepath = invocation.path / 'TestResults'
 
         try:
             results = invocation.phase.read(beakerlib_results_filepath, level=3)

tmt/frameworks/shell.py

@@ -48,5 +48,5 @@ def extract_results(
         return [tmt.Result.from_test_invocation(
             invocation=invocation,
             result=result,
-            log=[invocation.data_path(tmt.steps.execute.TEST_OUTPUT_FILENAME)],
+            log=[invocation.relative_path / tmt.steps.execute.TEST_OUTPUT_FILENAME],
             note=note)]

tmt/result.py

@@ -232,7 +232,7 @@ def from_test_invocation(
             ids=ids,
             log=log or [],
             guest=guest_data,
-            data_path=invocation.data_path('data'))
+            data_path=invocation.relative_test_data_path)
 
         return _result.interpret_result(ResultInterpret(
             invocation.test.result) if invocation.test.result else ResultInterpret.RESPECT)

tmt/steps/execute/__init__.py

@@ -23,7 +23,7 @@
 from tmt.steps import Action, PhaseQueue, QueuedPhase, Step
 from tmt.steps.discover import Discover, DiscoverPlugin, DiscoverStepData
 from tmt.steps.provision import Guest
-from tmt.utils import Path, ShellScript, Stopwatch, field
+from tmt.utils import Path, ShellScript, Stopwatch, cached_property, field
 
 if TYPE_CHECKING:
     import tmt.cli
@@ -159,44 +159,58 @@ class TestInvocation:
 
     _reboot_count: int = 0
 
-    def data_path(
-            self,
-            filename: Optional[str] = None,
-            full: bool = False,
-            create: bool = False) -> Path:
-        """
-        Prepare full/relative test data directory/file path.
+    @cached_property
+    def path(self) -> Path:
+        """ Absolute path to invocation directory """
 
-        Construct test data directory path for given test, create it
-        if requested and return the full or relative path to it (if
-        filename not provided) or to the given data file otherwise.
-        """
-        # Prepare directory path, create if requested
         assert self.phase.step.workdir is not None  # narrow type
-        directory = self.phase.step.workdir \
+
+        path = self.phase.step.workdir \
             / TEST_DATA \
             / 'guest' \
             / self.guest.safe_name \
             / f'{self.test.safe_name.lstrip("/") or "default"}-{self.test.serial_number}'
-        if create and not directory.is_dir():
-            directory.joinpath(TEST_DATA).mkdir(parents=True)
-        if not filename:
-            return directory
-        path = directory / filename
-        return path if full else path.relative_to(self.phase.step.workdir)
+
+        path.mkdir(parents=True, exist_ok=True)
+
+        # Pre-create also the test data path - cannot use `self.test_data_path`,
+        # that would be an endless recursion.
+        (path / TEST_DATA).mkdir(parents=True, exist_ok=True)
+
+        return path
+
+    @cached_property
+    def relative_path(self) -> Path:
+        """ Invocation directory path relative to step workdir """
+
+        assert self.phase.step.workdir is not None  # narrow type
+
+        return self.path.relative_to(self.phase.step.workdir)
+
+    @cached_property
+    def test_data_path(self) -> Path:
+        """ Absolute path to test data directory """
+
+        return self.path / TEST_DATA
+
+    @cached_property
+    def relative_test_data_path(self) -> Path:
+        """ Test data path relative to step workdir """
+
+        return self.relative_path / TEST_DATA
 
     @tmt.utils.cached_property
     def check_files_path(self) -> Path:
         """ Construct a directory path for check files needed by tmt """
-        path = self.data_path(create=True, full=True) / "checks"
+
+        path = self.path / "checks"
         path.mkdir(exist_ok=True)
         return path
 
     @tmt.utils.cached_property
     def reboot_request_path(self) -> Path:
         """ A path to the reboot request file """
-        return self.data_path(full=True) \
-            / TEST_DATA \
+        return self.test_data_path \
             / TMT_REBOOT_SCRIPT.created_file
 
     @property
@@ -224,8 +238,6 @@ def handle_reboot(self) -> bool:
         self.logger.debug(
             f"Reboot during test '{self.test}' with reboot count {self._reboot_count}.")
 
-        test_data = self.data_path(full=True) / TEST_DATA
-
         with open(self.reboot_request_path) as reboot_file:
             reboot_data = json.loads(reboot_file.read())
 
@@ -241,7 +253,7 @@ def handle_reboot(self) -> bool:
 
         # Reset the file
         os.remove(self.reboot_request_path)
-        self.guest.push(test_data)
+        self.guest.push(self.test_data_path)
 
         rebooted = False
 
@@ -355,10 +367,9 @@ def prepare_tests(self, guest: Guest, logger: tmt.log.Logger) -> list[TestInvoca
             invocation = TestInvocation(phase=self, test=test, guest=guest, logger=logger)
             invocations.append(invocation)
 
-            metadata_filename = invocation.data_path(
-                filename=TEST_METADATA_FILENAME, full=True, create=True)
             self.write(
-                metadata_filename, tmt.utils.dict_to_yaml(test._metadata))
+                invocation.path / TEST_METADATA_FILENAME,
+                tmt.utils.dict_to_yaml(test._metadata))
 
         return invocations
 
@@ -380,8 +391,7 @@ def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None:
     def _tmt_report_results_filepath(self, invocation: TestInvocation) -> Path:
         """ Create path to test's ``tmt-report-result`` file """
 
-        return invocation.data_path(full=True) \
-            / TEST_DATA \
+        return invocation.test_data_path \
             / TMT_REPORT_RESULT_SCRIPT.created_file
 
     def load_tmt_report_results(self, invocation: TestInvocation) -> list["tmt.Result"]:
@@ -424,7 +434,7 @@ def load_tmt_report_results(self, invocation: TestInvocation) -> list["tmt.Resul
         return [tmt.Result.from_test_invocation(
             invocation=invocation,
             result=actual_result,
-            log=[invocation.data_path(TEST_OUTPUT_FILENAME)],
+            log=[invocation.relative_path / TEST_OUTPUT_FILENAME],
             note=note)]
 
     def load_custom_results(self, invocation: TestInvocation) -> list["tmt.Result"]:
@@ -433,10 +443,8 @@ def load_custom_results(self, invocation: TestInvocation) -> list["tmt.Result"]:
         """
         test, guest = invocation.test, invocation.guest
 
-        test_data_path = invocation.data_path(full=True) / TEST_DATA
-
-        custom_results_path_yaml = test_data_path / 'results.yaml'
-        custom_results_path_json = test_data_path / 'results.json'
+        custom_results_path_yaml = invocation.test_data_path / 'results.yaml'
+        custom_results_path_json = invocation.test_data_path / 'results.json'
 
         if custom_results_path_yaml.exists():
             with open(custom_results_path_yaml) as results_file:
@@ -449,7 +457,7 @@ def load_custom_results(self, invocation: TestInvocation) -> list["tmt.Result"]:
         else:
             return [tmt.Result.from_test_invocation(
                 invocation=invocation,
-                note=f"custom results file not found in '{test_data_path}'",
+                note=f"custom results file not found in '{invocation.test_data_path}'",
                 result=ResultOutcome.ERROR)]
 
         if not results:
@@ -477,8 +485,8 @@ def load_custom_results(self, invocation: TestInvocation) -> list["tmt.Result"]:
 
             # Fix log paths as user provides relative path to TMT_TEST_DATA
             # but Result has to point relative to the execute workdir
-            log_path_base = invocation.data_path(full=False, filename=TEST_DATA)
-            partial_result.log = [log_path_base / log for log in partial_result.log]
+            partial_result.log = [
+                invocation.relative_test_data_path / log for log in partial_result.log]
 
             # TODO: this might need more care: the test has been assigned a serial
             # number, which is now part of its data directory path. Now, the test
@@ -532,8 +540,7 @@ def check_abort_file(self, invocation: TestInvocation) -> bool:
 
         Returns whether an abort file is present (i.e. abort occurred).
         """
-        return (invocation.data_path(full=True)
-                / TEST_DATA
+        return (invocation.test_data_path
                 / TMT_ABORT_SCRIPT.created_file).exists()
 
     @staticmethod
@@ -556,7 +563,7 @@ def format_duration(duration: datetime.timedelta) -> str:
 
     def timeout_hint(self, invocation: TestInvocation) -> None:
         """ Append a duration increase hint to the test output """
-        output = invocation.data_path(TEST_OUTPUT_FILENAME, full=True)
+        output = invocation.path / TEST_OUTPUT_FILENAME
         self.write(
             output,
             f"\nMaximum test time '{invocation.test.duration}' exceeded.\n"

tmt/steps/execute/internal.py

@@ -223,8 +223,6 @@ def _test_environment(
 
         extra_environment = extra_environment or {}
 
-        data_directory = invocation.data_path(full=True, create=True)
-
         environment = extra_environment.copy()
         environment.update(invocation.test.environment)
         assert self.parent is not None
@@ -235,12 +233,12 @@ def _test_environment(
         environment['TMT_TEST_PIDFILE_LOCK'] = str(
             effective_pidfile_root() / TEST_PIDFILE_LOCK_FILENAME)
         environment["TMT_TEST_NAME"] = invocation.test.name
-        environment["TMT_TEST_DATA"] = str(data_directory / tmt.steps.execute.TEST_DATA)
+        environment["TMT_TEST_DATA"] = str(invocation.test_data_path)
         environment['TMT_TEST_SERIAL_NUMBER'] = str(invocation.test.serial_number)
         environment["TMT_TEST_METADATA"] = str(
-            data_directory / tmt.steps.execute.TEST_METADATA_FILENAME)
+            invocation.path / tmt.steps.execute.TEST_METADATA_FILENAME)
         environment["TMT_REBOOT_REQUEST"] = str(
-            data_directory / tmt.steps.execute.TEST_DATA / TMT_REBOOT_SCRIPT.created_file)
+            invocation.test_data_path / TMT_REBOOT_SCRIPT.created_file)
         # Set all supported reboot variables
         for reboot_variable in TMT_REBOOT_SCRIPT.related_variables:
             environment[reboot_variable] = str(invocation._reboot_count)
@@ -315,7 +313,7 @@ def execute(
         topology.guest = tmt.steps.GuestTopology(guest)
 
         environment.update(topology.push(
-            dirpath=invocation.data_path(full=True),
+            dirpath=invocation.path,
             guest=guest,
             logger=logger))
 
@@ -386,7 +384,7 @@ def _test_output_logger(
         invocation.real_duration = self.format_duration(timer.duration)
 
         self.write(
-            invocation.data_path(TEST_OUTPUT_FILENAME, full=True),
+            invocation.path / TEST_OUTPUT_FILENAME,
             stdout or '', mode='a', level=3)
 
         # TODO: do we want timestamps? Yes, we do, leaving that for refactoring later,
@@ -452,7 +450,7 @@ def _run_tests(
                     logger=logger)
 
                 guest.pull(
-                    source=invocation.data_path(full=True),
+                    source=invocation.path,
                     extend_options=test.test_framework.get_pull_options(invocation, logger))
 
                 results = self.extract_results(invocation, logger)  # Produce list of results