Added test coverage for the automatic generation of default .gtkw file.

Author: zapta

apio/scons/gtkwave_util.py

@@ -12,7 +12,7 @@
 GTKW_AUTO_FILE_MARKER = "THIS FILE WAS GENERATED AUTOMATICALLY BY APIO"
 
 
-def _get_gtkw_file_header(testbench_path: Path) -> str:
+def _get_gtkw_file_header(testbench_path: str) -> str:
     """Return a header string for the auto generated .gtkw file. 'testbench'
     is the relative path of the testbench file."""
 
@@ -21,7 +21,8 @@ def _get_gtkw_file_header(testbench_path: Path) -> str:
 
     lines = [
         f"# GTKWave display configuration for 'apio sim {tb_path_posix}'",
-        f"# {GTKW_AUTO_FILE_MARKER}. DO NOT EDIT IT MANUALLY!",
+        f"# {GTKW_AUTO_FILE_MARKER}.",
+        "# DO NOT EDIT IT MANUALLY!",
         f"# To customize this file, run 'apio sim {tb_path_posix}'",
         "# and save the file from GTKWave.",
         "",
@@ -33,20 +34,29 @@ def _get_gtkw_file_header(testbench_path: Path) -> str:
     return "\n".join(lines) + "\n"
 
 
-def _signal_sort_key(s: str) -> Tuple[int, str]:
-    """Given a signal name, returns a key to use for signals sorting."""
-    lcs = s.lower()
+def _signal_sort_key(signal: str) -> Tuple[int, str]:
+    """Given a top level signal name, returns a key to use for signals
+    sorting. Signal is expected to have a two part format
+    top-module.signal-name.
+    """
+    # -- Parse signal
+    parts = signal.split(".")
+    assert len(parts) == 2, signal
+
+    # -- Prepare value to test.
+    lc_name1 = parts[0].lower()
+    lc_name2 = parts[1].lower()
 
     # -- Priority 1: clock signals..
-    if re.search(r"clk|clock", s):
-        return (1, lcs)
+    if re.search(r"clk|clock", lc_name2):
+        return (lc_name1, 1, lc_name2)
 
     # -- Priority 2: reset signals.
-    if re.search(r"reset|rst", s):
-        return (2, lcs)
+    if re.search(r"reset|rst", lc_name2):
+        return (lc_name1, 2, lc_name2)
 
     # -- Priority 3: all the rest.
-    return (3, lcs)
+    return (lc_name1, 3, lc_name2)
 
 
 def create_gtkwave_file(
@@ -68,11 +78,7 @@ def create_gtkwave_file(
     vcd = VCDVCD(vcd_path, signal_res=[pattern], store_tvs=False)
 
     # -- Get a list with raw names of matching signals.
-    raw_signals = list(vcd.references_to_ids.keys())
-
-    # -- Strip range suffixes such as "[31:0]""
-    range_pattern = re.compile(r"\[\d+:\d+\]$")
-    signals = [range_pattern.sub("", sig) for sig in raw_signals]
+    signals = list(vcd.references_to_ids.keys())
 
     # -- Use basic heuristics to sort the files.
     signals.sort(key=_signal_sort_key)

tests/unit_tests/scons/test_gtkwave_util.py

@@ -0,0 +1,158 @@
+"""
+Tests of gtkwave_util.py
+"""
+
+from pathlib import Path
+from tests.conftest import ApioRunner
+from apio.commands.apio import apio_top_cli as apio
+from apio.scons.gtkwave_util import create_gtkwave_file, _signal_sort_key
+
+# Expected default gtkw file lines for the examples we use below.
+EXPECTED_GTKW_LINES = [
+    "# GTKWave display configuration for 'apio sim main_tb.v'",
+    "# THIS FILE WAS GENERATED AUTOMATICALLY BY APIO.",
+    "# DO NOT EDIT IT MANUALLY!",
+    "# To customize this file, run 'apio sim main_tb.v'",
+    "# and save the file from GTKWave.",
+    "",
+    "[*] GTKWave Analyzer v3.4.0 (w)1999-2022 BSI",
+    "",
+    "[*]",
+    "testbench.CLK",
+    "testbench.expected_led",
+    "testbench.i[31:0]",
+    "testbench.LED1",
+    "testbench.LED2",
+    "",
+]
+
+
+def test_signal_sort_key():
+    """Test the default signals sorting heuristic."""
+
+    # -- Priority 1: clock.
+    assert _signal_sort_key("tb.clk") == ("tb", 1, "clk")
+    assert _signal_sort_key("tb.Clk") == ("tb", 1, "clk")
+    assert _signal_sort_key("tb.sys_clk") == ("tb", 1, "sys_clk")
+    assert _signal_sort_key("tb.clock1") == ("tb", 1, "clock1")
+    assert _signal_sort_key("tb.CLOCK1") == ("tb", 1, "clock1")
+
+    # -- Priority 2: reset.
+    assert _signal_sort_key("tb.rst") == ("tb", 2, "rst")
+    assert _signal_sort_key("tb.RST") == ("tb", 2, "rst")
+    assert _signal_sort_key("tb.Reset") == ("tb", 2, "reset")
+    assert _signal_sort_key("tb.reset") == ("tb", 2, "reset")
+    assert _signal_sort_key("tb.reset_n") == ("tb", 2, "reset_n")
+    assert _signal_sort_key("tb.sys_rst") == ("tb", 2, "sys_rst")
+    assert _signal_sort_key("tb.sys_reset") == ("tb", 2, "sys_reset")
+    assert _signal_sort_key("tb.RESET_N") == ("tb", 2, "reset_n")
+
+    # -- Priority 3: all other signals.
+    assert _signal_sort_key("tb.other") == ("tb", 3, "other")
+    assert _signal_sort_key("tb.Other") == ("tb", 3, "other")
+    assert _signal_sort_key("tb.OTHER") == ("tb", 3, "other")
+
+
+def test_create_gtkwave_file(apio_runner: ApioRunner):
+    """Test the create_gtkwave_file() function"""
+
+    with apio_runner.in_sandbox() as sb:
+
+        # -- Create relative paths to the .gtkw and .vcd files
+        gtkw_path = Path("main_tb.gtkw")
+        vcd_path = Path("_build/blink-slow/main_tb.vcd")
+
+        # -- Execute "apio examples fetch alhambra-ii/getting-started"
+        result = sb.invoke_apio_cmd(
+            apio, ["examples", "fetch", "alhambra-ii/getting-started"]
+        )
+        sb.assert_result_ok(result)
+
+        # -- Execute "apio test main_tb.v" to create the .vcd file
+        result = sb.invoke_apio_cmd(apio, ["test", "main_tb.v"])
+        sb.assert_result_ok(result)
+
+        # -- Verify that the .vcd file exists.
+        assert vcd_path.is_file()
+
+        # -- Delete the file main_tb.gtkw
+        assert gtkw_path.is_file()
+        gtkw_path.unlink()
+        assert not gtkw_path.exists()
+
+        # -- Create the default .gtkw file
+        create_gtkwave_file("main_tb.v", str(vcd_path), str(gtkw_path))
+        assert gtkw_path.is_file()
+
+        # -- Test the generated .gtkw file.
+        text = sb.read_file(gtkw_path)
+        lines = text.split("\n")
+        print(f"Actual {gtkw_path} lines:")
+        for line in lines:
+            print(f'    "{line}",')
+
+        assert lines == EXPECTED_GTKW_LINES
+
+
+def test_default_signals_creation(apio_runner: ApioRunner):
+    """Test the automatic .gtkw file creation by 'apio sim'."""
+
+    with apio_runner.in_sandbox() as sb:
+
+        # -- Execute "apio examples fetch alhambra-ii/getting-started"
+        result = sb.invoke_apio_cmd(
+            apio, ["examples", "fetch", "alhambra-ii/getting-started"]
+        )
+        sb.assert_result_ok(result)
+
+        # -- Delete the file main_tb.gtkw
+        gtkw_path = Path("main_tb.gtkw")
+        assert gtkw_path.exists()
+        gtkw_path.unlink()
+        assert not gtkw_path.exists()
+
+        # -- Execute "apio sim --no-gtkwave main_tb.v"
+        result = sb.invoke_apio_cmd(apio, ["sim", "--no-gtkwave", "main_tb.v"])
+        sb.assert_result_ok(result)
+        assert gtkw_path.exists()
+
+        # -- Test the generated .gtkw file.
+        text = sb.read_file(gtkw_path)
+        lines = text.split("\n")
+        print(f"Actual {gtkw_path} lines:")
+        for line in lines:
+            print(f'    "{line}",')
+
+        assert lines == EXPECTED_GTKW_LINES
+
+
+def test_user_gtkw_file_protection(apio_runner: ApioRunner):
+    """Test that a user's .gtkw file is not overwritten by apio'."""
+
+    with apio_runner.in_sandbox() as sb:
+
+        # -- Execute "apio examples fetch alhambra-ii/getting-started"
+        result = sb.invoke_apio_cmd(
+            apio, ["examples", "fetch", "alhambra-ii/getting-started"]
+        )
+        sb.assert_result_ok(result)
+
+        # -- Read the user's .gtkw file
+        gtkw_path = Path("main_tb.gtkw")
+        assert gtkw_path.exists()
+        text_before = sb.read_file(gtkw_path)
+
+        # gtkw_path.unlink()
+        # assert not gtkw_path.exists()
+
+        # -- Execute "apio sim --no-gtkwave main_tb.v"
+        result = sb.invoke_apio_cmd(apio, ["sim", "--no-gtkwave", "main_tb.v"])
+        sb.assert_result_ok(result)
+        assert gtkw_path.exists()
+
+        # -- Read the .gtkw file after the operation.
+        assert gtkw_path.exists()
+        text_after = sb.read_file(gtkw_path)
+
+        # -- Verify that apio didn't change it.
+        assert text_after == text_before