From 64d642f4bcf7c885898220e34464530b4c78fe97 Mon Sep 17 00:00:00 2001
From: Oscar Gustafsson <oscar.gustafsson@gmail.com>
Date: Thu, 11 Jul 2024 14:50:28 +0200
Subject: [PATCH] Use Paths rather than strings where possible

---
 vunit/sim_if/__init__.py             | 15 +++++++++++++--
 vunit/sim_if/activehdl.py            |  6 +++---
 vunit/sim_if/ghdl.py                 | 13 +++++--------
 vunit/sim_if/incisive.py             | 18 +++++++++---------
 vunit/sim_if/modelsim.py             | 24 +++++++++++++-----------
 vunit/sim_if/nvc.py                  |  7 ++-----
 vunit/sim_if/rivierapro.py           | 19 ++++++++++---------
 vunit/sim_if/vsim_simulator_mixin.py |  6 +++---
 vunit/test/runner.py                 | 16 ++++++++--------
 vunit/ui/__init__.py                 | 26 +++++++++++++-------------
 vunit/ui/results.py                  |  4 ++--
 11 files changed, 81 insertions(+), 73 deletions(-)

diff --git a/vunit/sim_if/__init__.py b/vunit/sim_if/__init__.py
index 5596dbd3c..12d6b20a7 100644
--- a/vunit/sim_if/__init__.py
+++ b/vunit/sim_if/__init__.py
@@ -50,11 +50,11 @@ class SimulatorInterface(object):  # pylint: disable=too-many-public-methods
     supports_colors_in_gui = False
 
     def __init__(self, output_path, gui):
-        self._output_path = output_path
+        self._output_path = Path(output_path)
         self._gui = gui
 
     @property
-    def output_path(self):
+    def output_path(self) -> Path:
         return self._output_path
 
     @property
@@ -319,6 +319,17 @@ def get_env():
         Allows inheriting classes to overload this to modify environment variables. Return None for default environment
         """
 
+    def get_script_path(self, output_path) -> Path:
+        """
+        Get path for scripts.
+
+        Also creates the required directories if they do not exist.
+        """
+        script_path = Path(output_path) / self.name
+        if not script_path.exists():
+            script_path.mkdir(parents=True)
+        return script_path
+
 
 def isfile(file_name):
     """
diff --git a/vunit/sim_if/activehdl.py b/vunit/sim_if/activehdl.py
index 521b8cfae..4cacae80c 100644
--- a/vunit/sim_if/activehdl.py
+++ b/vunit/sim_if/activehdl.py
@@ -326,8 +326,8 @@ def merge_coverage(self, file_name, args=None):
 
         merge_command += f" -o {{{fix_path(file_name)!s}}}\n"
 
-        merge_script_name = str(Path(self._output_path) / "acdb_merge.tcl")
-        with Path(merge_script_name).open("w", encoding="utf-8") as fptr:
+        merge_script_name = self.output_path / "acdb_merge.tcl"
+        with merge_script_name.open("w", encoding="utf-8") as fptr:
             fptr.write(merge_command + "\n")
 
         vcover_cmd = [
@@ -525,7 +525,7 @@ def simulate(self, output_path, test_suite_name, config, elaborate_only):
         """
         Run a test bench
         """
-        script_path = Path(output_path) / self.name
+        script_path = self.get_script_path(output_path)
         common_file_name = script_path / "common.tcl"
         batch_file_name = script_path / "batch.tcl"
         gui_file_name = script_path / "gui.tcl"
diff --git a/vunit/sim_if/ghdl.py b/vunit/sim_if/ghdl.py
index 6fd33dee0..e99639d02 100644
--- a/vunit/sim_if/ghdl.py
+++ b/vunit/sim_if/ghdl.py
@@ -348,17 +348,14 @@ def simulate(self, output_path, test_suite_name, config, elaborate_only):  # pyl
         Simulate with entity as top level using generics
         """
 
-        script_path = str(Path(output_path) / self.name)
-
-        if not Path(script_path).exists():
-            makedirs(script_path)
+        script_path = self.get_script_path(output_path)
 
         ghdl_e = elaborate_only and config.sim_options.get("ghdl.elab_e", False)
 
         if self._gtkwave_fmt is not None:
-            data_file_name = str(Path(script_path) / f"wave.{self._gtkwave_fmt!s}")
-            if Path(data_file_name).exists():
-                remove(data_file_name)
+            data_file_name = Path(script_path) / f"wave.{self._gtkwave_fmt!s}"
+            if data_file_name.exists():
+                data_file_name.unlink()
         else:
             data_file_name = None
 
@@ -383,7 +380,7 @@ def simulate(self, output_path, test_suite_name, config, elaborate_only):  # pyl
             status = False
 
         if self._gui and not elaborate_only:
-            cmd = ["gtkwave"] + shlex.split(self._gtkwave_args) + [data_file_name]
+            cmd = ["gtkwave"] + shlex.split(self._gtkwave_args) + [str(data_file_name)]
 
             init_file = config.sim_options.get(self.name + ".gtkwave_script.gui", None)
             if init_file is not None:
diff --git a/vunit/sim_if/incisive.py b/vunit/sim_if/incisive.py
index f4b829300..ea48906a5 100644
--- a/vunit/sim_if/incisive.py
+++ b/vunit/sim_if/incisive.py
@@ -126,7 +126,7 @@ def _create_cdslib(self):
 # needed for referencing the library 'basic' for cells 'cds_alias', 'cds_thru' etc. in analog models:
 # NOTE: 'virtuoso' executable not found!
 # define basic ".../tools/dfII/etc/cdslib/basic"
-define work "{self._output_path}/libraries/work"
+define work "{self.output_path}/libraries/work"
 """
             if cds_root_virtuoso is None
             else f"""\
@@ -134,7 +134,7 @@ def _create_cdslib(self):
 softinclude {self._cds_root_irun}/tools/inca/files/cds.lib
 # needed for referencing the library 'basic' for cells 'cds_alias', 'cds_thru' etc. in analog models:
 define basic "{cds_root_virtuoso}/tools/dfII/etc/cdslib/basic"
-define work "{self._output_path}/libraries/work"
+define work "{self.output_path}/libraries/work"
 """
         )
 
@@ -193,7 +193,7 @@ def compile_vhdl_file_command(self, source_file):
         args += ["-work work"]
         args += [f'-cdslib "{self._cdslib!s}"']
         args += self._hdlvar_args()
-        args += [f'-log "{(Path(self._output_path) / f"irun_compile_vhdl_file_{source_file.library.name!s}.log")!s}"']
+        args += [f'-log "{(Path(self.output_path) / f"irun_compile_vhdl_file_{source_file.library.name!s}.log")!s}"']
         if not self._log_level == "debug":
             args += ["-quiet"]
         else:
@@ -204,7 +204,7 @@ def compile_vhdl_file_command(self, source_file):
         args += [f"-makelib {source_file.library.directory!s}"]
         args += [f'"{source_file.name!s}"']
         args += ["-endlib"]
-        argsfile = str(Path(self._output_path) / f"irun_compile_vhdl_file_{source_file.library.name!s}.args")
+        argsfile = str(Path(self.output_path) / f"irun_compile_vhdl_file_{source_file.library.name!s}.args")
         write_file(argsfile, "\n".join(args))
         return [cmd, "-f", argsfile]
 
@@ -228,7 +228,7 @@ def compile_verilog_file_command(self, source_file):
         args += [f'-cdslib "{self._cdslib!s}"']
         args += self._hdlvar_args()
         args += [
-            f'-log "{(Path(self._output_path) / f"irun_compile_verilog_file_{source_file.library.name!s}.log")!s}"'
+            f'-log "{(Path(self.output_path) / f"irun_compile_verilog_file_{source_file.library.name!s}.log")!s}"'
         ]
         if not self._log_level == "debug":
             args += ["-quiet"]
@@ -248,7 +248,7 @@ def compile_verilog_file_command(self, source_file):
         args += [f"-makelib {source_file.library.name!s}"]
         args += [f'"{source_file.name!s}"']
         args += ["-endlib"]
-        argsfile = str(Path(self._output_path) / f"irun_compile_verilog_file_{source_file.library.name!s}.args")
+        argsfile = str(self.output_path / f"irun_compile_verilog_file_{source_file.library.name!s}.args")
         write_file(argsfile, "\n".join(args))
         return [cmd, "-f", argsfile]
 
@@ -292,7 +292,7 @@ def simulate(
         Elaborates and Simulates with entity as top level using generics
         """
 
-        script_path = str(Path(output_path) / self.name)
+        script_path = self.get_script_path(output_path)
         launch_gui = self._gui is not False and not elaborate_only
 
         if elaborate_only:
@@ -320,11 +320,11 @@ def simulate(
             args += ["-ncerror EVBSTR"]  # promote to error: "bad string literal in generic association"
             args += ["-ncerror EVBNAT"]  # promote to error: "bad natural literal in generic association"
             args += ["-work work"]
-            args += [f'-nclibdirname "{Path(self._output_path) / "libraries"!s}"']  # @TODO: ugly
+            args += [f'-nclibdirname "{self.output_path / "libraries"!s}"']  # @TODO: ugly
             args += config.sim_options.get("incisive.irun_sim_flags", [])
             args += [f'-cdslib "{self._cdslib!s}"']
             args += self._hdlvar_args()
-            args += [f'-log "{(Path(script_path) / f"irun_{step!s}.log")!s}"']
+            args += [f'-log "{(script_path / f"irun_{step!s}.log")!s}"']
             if not self._log_level == "debug":
                 args += ["-quiet"]
             else:
diff --git a/vunit/sim_if/modelsim.py b/vunit/sim_if/modelsim.py
index dc353211d..fa4e3a917 100644
--- a/vunit/sim_if/modelsim.py
+++ b/vunit/sim_if/modelsim.py
@@ -102,9 +102,9 @@ def _create_modelsim_ini(self):
         """
         Create the modelsim.ini file
         """
-        parent = str(Path(self._sim_cfg_file_name).parent)
-        if not file_exists(parent):
-            os.makedirs(parent)
+        parent = Path(self._sim_cfg_file_name).parent
+        if not parent.exists():
+            parent.mkdir(parents=True)
 
         original_modelsim_ini = os.environ.get("VUNIT_MODELSIM_INI", str(Path(self._prefix).parent / "modelsim.ini"))
         with Path(original_modelsim_ini).open("rb") as fread:
@@ -202,13 +202,15 @@ def create_library(self, library_name, path, mapped_libraries=None):
         """
         mapped_libraries = mapped_libraries if mapped_libraries is not None else {}
 
-        apath = str(Path(path).parent.resolve())
+        path = Path(path)
 
-        if not file_exists(apath):
-            os.makedirs(apath)
+        apath = path.parent.resolve()
 
-        if not file_exists(path):
-            proc = Process([str(Path(self._prefix) / "vlib"), "-unix", path], env=self.get_env())
+        if not apath.exists():
+            apath.mkdir(parents=True)
+
+        if not path.exits(path):
+            proc = Process([Path(self._prefix) / "vlib", "-unix", path], env=self.get_env())
             proc.consume_output(callback=None)
 
         if library_name in mapped_libraries and mapped_libraries[library_name] == path:
@@ -383,9 +385,9 @@ def merge_coverage(self, file_name, args=None):
         if args is None:
             args = []
 
-        coverage_files = str(Path(self._output_path) / "coverage_files.txt")
-        vcover_cmd = [str(Path(self._prefix) / "vcover"), "merge", "-inputs"] + [coverage_files] + args + [file_name]
-        with Path(coverage_files).open("w", encoding="utf-8") as fptr:
+        coverage_files = self.output_path / "coverage_files.txt"
+        vcover_cmd = [Path(self._prefix) / "vcover", "merge", "-inputs"] + [coverage_files] + args + [file_name]
+        with coverage_files.open("w", encoding="utf-8") as fptr:
             for coverage_file in self._coverage_files:
                 if file_exists(coverage_file):
                     fptr.write(str(coverage_file) + "\n")
diff --git a/vunit/sim_if/nvc.py b/vunit/sim_if/nvc.py
index 8f64ecae3..edd028a48 100644
--- a/vunit/sim_if/nvc.py
+++ b/vunit/sim_if/nvc.py
@@ -246,15 +246,12 @@ def simulate(self, output_path, test_suite_name, config, elaborate_only):  # pyl
         Simulate with entity as top level using generics
         """
 
-        script_path = Path(output_path) / self.name
-
-        if not script_path.exists():
-            makedirs(script_path)
+        script_path = self.get_script_path(output_path)
 
         if self._gui:
             wave_file = script_path / (f"{config.entity_name}.fst")
             if wave_file.exists():
-                remove(wave_file)
+                wave_file.unlink()
         else:
             wave_file = None
 
diff --git a/vunit/sim_if/rivierapro.py b/vunit/sim_if/rivierapro.py
index c794e9c0b..fb480fd3d 100644
--- a/vunit/sim_if/rivierapro.py
+++ b/vunit/sim_if/rivierapro.py
@@ -66,7 +66,7 @@ def find_prefix_from_path(cls):
         """
 
         def no_avhdl(path):
-            return not file_exists(str(Path(path) / "avhdl.exe"))
+            return not (Path(path) / "avhdl.exe").exists()
 
         return cls.find_toolchain(["vsim", "vsimsa"], constraints=[no_avhdl])
 
@@ -218,18 +218,18 @@ def compile_verilog_file_command(self, source_file):
                 args[-1] += f"={value!s}"
         return args
 
-    def create_library(self, library_name, path, mapped_libraries=None):
+    def create_library(self, library_name, path: Path, mapped_libraries=None):
         """
         Create and map a library_name to path
         """
         mapped_libraries = mapped_libraries if mapped_libraries is not None else {}
 
-        apath = str(Path(path).parent.resolve())
+        apath = path.parent.resolve()
 
-        if not file_exists(apath):
-            os.makedirs(apath)
+        if not apath.exists():
+            apath.mkdir(parents=True)
 
-        if not file_exists(path):
+        if not path.exists():
             proc = Process(
                 [str(Path(self._prefix) / "vlib"), library_name, path],
                 cwd=str(Path(self._sim_cfg_file_name).parent),
@@ -251,10 +251,11 @@ def _create_library_cfg(self):
         """
         Create the library.cfg file if it does not exist
         """
-        if file_exists(self._sim_cfg_file_name):
+        cfg_file = Path(self._sim_cfg_file_name)
+        if cfg_file.exists():
             return
 
-        with Path(self._sim_cfg_file_name).open("w", encoding="utf-8") as ofile:
+        with cfg_file.open("w", encoding="utf-8") as ofile:
             ofile.write(f'$INCLUDE = "{self._builtin_library_cfg!s}"\n')
 
     @property
@@ -418,7 +419,7 @@ def merge_coverage(self, file_name, args=None):
         fname = file_name.replace("\\", "/")
         merge_command += f" -o {{{fname}}}"
 
-        merge_script_name = Path(self._output_path) / "acdb_merge.tcl"
+        merge_script_name = self.output_path / "acdb_merge.tcl"
         with merge_script_name.open("w", encoding="utf-8") as fptr:
             fptr.write(merge_command + "\n")
 
diff --git a/vunit/sim_if/vsim_simulator_mixin.py b/vunit/sim_if/vsim_simulator_mixin.py
index 1ff8bb59c..31dfbd78d 100644
--- a/vunit/sim_if/vsim_simulator_mixin.py
+++ b/vunit/sim_if/vsim_simulator_mixin.py
@@ -311,7 +311,7 @@ def simulate(self, output_path, test_suite_name, config, elaborate_only):
         """
         Run a test bench
         """
-        script_path = Path(output_path) / self.name
+        script_path = self.get_script_path(output_path)
 
         common_file_name = script_path / "common.do"
         gui_file_name = script_path / "gui.do"
@@ -336,11 +336,11 @@ def simulate(self, output_path, test_suite_name, config, elaborate_only):
         return self._run_batch_file(str(batch_file_name))
 
 
-def fix_path(path):
+def fix_path(path: Path) -> str:
     """
     Adjust path for TCL usage
     """
-    return path.replace("\\", "/").replace(" ", "\\ ")
+    return str(path).replace("\\", "/").replace(" ", "\\ ")
 
 
 def get_is_test_suite_done_tcl(vunit_result_file):
diff --git a/vunit/test/runner.py b/vunit/test/runner.py
index 5fa35af31..9456bc19e 100644
--- a/vunit/test/runner.py
+++ b/vunit/test/runner.py
@@ -49,7 +49,7 @@ def __init__(  # pylint: disable=too-many-arguments
         self._abort = False
         self._local = threading.local()
         self._report = report
-        self._output_path = output_path
+        self._output_path = Path(output_path)
         assert verbosity in (
             self.VERBOSITY_QUIET,
             self.VERBOSITY_NORMAL,
@@ -78,8 +78,8 @@ def run(self, test_suites):
         Run a list of test suites
         """
 
-        if not Path(self._output_path).exists():
-            os.makedirs(self._output_path)
+        if not self._output_path.exists():
+            self._output_path.mkdir(parents=True)
 
         self._create_test_mapping_file(test_suites)
 
@@ -176,7 +176,7 @@ def _get_output_path(self, test_suite_name):
         Construct the full output path of a test case.
         Ensure no bad characters and no long path names.
         """
-        output_path = str(Path(self._output_path).resolve())
+        output_path = self._output_path.resolve()
         safe_name = "".join(char if _is_legal(char) else "_" for char in test_suite_name) + "_"
         hash_name = hash_string(test_suite_name)
 
@@ -190,7 +190,7 @@ def _get_output_path(self, test_suite_name):
         else:
             full_name = safe_name + hash_name
 
-        return str(Path(output_path) / full_name)
+        return str(output_path / full_name)
 
     def _add_skipped_tests(self, test_suite, results, start_time, num_tests, output_file_name):
         """
@@ -283,7 +283,7 @@ def _create_test_mapping_file(self, test_suites):
         Create a file mapping test name to test output folder.
         This is to allow the user to find the test output folder when it is hashed
         """
-        mapping_file_name = Path(self._output_path) / "test_name_to_path_mapping.txt"
+        mapping_file_name = self._output_path / "test_name_to_path_mapping.txt"
 
         # Load old mapping to remember non-deleted test folders as well
         # even when re-running only a single test case
@@ -304,11 +304,11 @@ def _create_test_mapping_file(self, test_suites):
             for value in mapping:
                 fptr.write(value + "\n")
 
-    def _print_output(self, output_file_name):
+    def _print_output(self, output_file_path: Path):
         """
         Print contents of output file if it exists
         """
-        with Path(output_file_name).open("r", encoding="utf-8") as fread:
+        with output_file_path.open("r", encoding="utf-8") as fread:
             for line in fread.readlines():
                 self._stdout_ansi.write(line)
 
diff --git a/vunit/ui/__init__.py b/vunit/ui/__init__.py
index 3ddc531e3..3f8ec5888 100644
--- a/vunit/ui/__init__.py
+++ b/vunit/ui/__init__.py
@@ -118,7 +118,7 @@ def __init__(
     ):
         self._args = args
         self._configure_logging(args.log_level)
-        self._output_path = str(Path(args.output_path).resolve())
+        self._output_path = Path(args.output_path).resolve()
 
         if args.no_color:
             self._printer = NO_COLOR_PRINTER
@@ -145,10 +145,10 @@ def test_filter(name, attribute_names):
         # Use default simulator options if no simulator was present
         if self._simulator_class is None:
             simulator_class = SimulatorInterface
-            self._simulator_output_path = str(Path(self._output_path) / "none")
+            self._simulator_output_path = self._output_path / "none"
         else:
             simulator_class = self._simulator_class
-            self._simulator_output_path = str(Path(self._output_path) / simulator_class.name)
+            self._simulator_output_path = self._output_path / simulator_class.name
 
         self._create_output_path(args.clean)
 
@@ -169,7 +169,7 @@ def _create_database(self):
         Check for Python version used to create the database is the
         same as the running python instance or re-create
         """
-        project_database_file_name = str(Path(self._output_path) / "project_database")
+        project_database_file_name = str(self._output_path / "project_database")
         create_new = False
         key = b"version"
         version = str((9, sys.version)).encode()
@@ -290,9 +290,9 @@ def add_library(
 
         """
         standard = self._which_vhdl_standard(vhdl_standard)
-        path = Path(self._simulator_output_path) / "libraries" / library_name
+        path = self._simulator_output_path / "libraries" / library_name
         if not self._project.has_library(library_name):
-            self._project.add_library(library_name, str(path.resolve()), standard)
+            self._project.add_library(library_name, path.resolve(), standard)
         elif not allow_duplicate:
             raise ValueError(f"Library {library_name!s} already added. Use allow_duplicate to ignore this error.")
         return self.library(library_name)
@@ -765,8 +765,8 @@ def _create_simulator_if(self):
             )
             sys.exit(1)
 
-        if not Path(self._simulator_output_path).exists():
-            os.makedirs(self._simulator_output_path)
+        if not self._simulator_output_path.exists():
+            self._simulator_output_path.mkdir(parents=True)
 
         return self._simulator_class.from_args(args=self._args, output_path=self._simulator_output_path)
 
@@ -893,8 +893,8 @@ def _create_output_path(self, clean: bool):
         """
         if clean:
             ostools.renew_path(self._output_path)
-        elif not Path(self._output_path).exists():
-            os.makedirs(self._output_path)
+        elif not self._output_path.exists():
+            self._output_path.mkdir(parents=True)
 
         ostools.renew_path(self._preprocessed_path)
 
@@ -904,11 +904,11 @@ def vhdl_standard(self) -> str:
 
     @property
     def _preprocessed_path(self):
-        return str(Path(self._output_path) / "preprocessed")
+        return str(self._output_path / "preprocessed")
 
     @property
     def codecs_path(self):
-        return str(Path(self._output_path) / "codecs")
+        return str(self._output_path / "codecs")
 
     def _compile(self, simulator_if: SimulatorInterface):
         """
@@ -952,7 +952,7 @@ def _run_test(self, test_cases, report):
 
         runner = TestRunner(
             report,
-            str(Path(self._output_path) / TEST_OUTPUT_PATH),
+            str(self._output_path / TEST_OUTPUT_PATH),
             verbosity=verbosity,
             num_threads=self._args.num_threads,
             fail_fast=self._args.fail_fast,
diff --git a/vunit/ui/results.py b/vunit/ui/results.py
index ea2ddfd6b..86e454e86 100644
--- a/vunit/ui/results.py
+++ b/vunit/ui/results.py
@@ -19,7 +19,7 @@ class Results(object):
     """
 
     def __init__(self, output_path, simulator_if, report):
-        self._output_path = output_path
+        self._output_path = Path(output_path)
         self._simulator_if = simulator_if
         self._report = report
 
@@ -44,7 +44,7 @@ def get_report(self):
             report.tests.update(
                 {
                     test.name: TestResult(
-                        Path(self._output_path) / TEST_OUTPUT_PATH,
+                        self._output_path / TEST_OUTPUT_PATH,
                         obj["status"],
                         obj["time"],
                         obj["path"],