[wic][PATCH 0/2] tests/unit/test_ksparser_parse: parse the .wks file, fix a diskid crash

[wic][PATCH 0/2] tests/unit/test_ksparser_parse: parse the .wks file, fix a diskid crash

[Trevor Woerner]

This continues the standalone unit-test work, moving up from ksparser's
argparse types to the KickStart parser that drives a whole .wks file.
It keeps the shape the suite settled on: the source fix is its own
standalone commit, and the green test module lands last, so the suite
passes at every commit.

The fix: a bootloader line with --ptable msdos and a non-integer
--diskid builds its error message with "... % self.ptable". self is the
KickStart instance, which has no ptable attribute, so the interpolation
raises AttributeError before the intended KickStartError. The user gets
an opaque traceback instead of a described error. Interpolate the
offending value instead (matching the gpt branch just below), and fix
the "--ptbale" typo in the same message.

The test module adds end-to-end coverage for KickStart(): happy-path
part/bootloader lines, degenerate files (empty, comment-only, CRLF,
unknown directive), mutually-exclusive and invalid option combinations,
multiple-bootloader rejection, diskid parsing for both ptables, and
include handling. get_bitbake_var() is neutralised so the parser runs
with no BitBake and no host tools. Backing the fix out turns the two
msdos --diskid cases red.

Trevor Woerner (2):
  ksparser: fix crash on an invalid msdos --diskid
  tests/unit/test_ksparser_parse: cover the .wks KickStart parser

 src/wic/ksparser.py               |   4 +-
 tests/unit/test_ksparser_parse.py | 217 ++++++++++++++++++++++++++++++
 2 files changed, 219 insertions(+), 2 deletions(-)
 create mode 100644 tests/unit/test_ksparser_parse.py

-- 
2.50.0.173.g8b6f19ccfc3a

[wic][PATCH 1/2] ksparser: fix crash on an invalid msdos --diskid

[Trevor Woerner]

When a bootloader line sets --ptable msdos with a --diskid that is not a
32-bit integer, the error path builds its message with "... % self.ptable".
self is the KickStart instance, which has no ptable attribute, so the
interpolation raises AttributeError before the intended KickStartError is
raised. Instead of a described error naming the bad value, the user sees an
opaque traceback.

Interpolate the offending value (parsed.diskid) into the message, matching
the gpt branch just below, so a bad msdos --diskid raises a KickStartError
that names it. Fix the "--ptbale" typo in the same message while here.

AI-Generated: codex/claude-opus 4.8 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
 src/wic/ksparser.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/wic/ksparser.py b/src/wic/ksparser.py
index dcf26f788083..a1276af8d3c6 100644
--- a/src/wic/ksparser.py
+++ b/src/wic/ksparser.py
@@ -322,9 +322,9 @@ class KickStart():
                                     try:
                                         self.bootloader.diskid = int(parsed.diskid, 0)
                                     except ValueError:
-                                        err = "with --ptbale msdos only 32bit integers " \
+                                        err = "with --ptable msdos only 32bit integers " \
                                               "are allowed for --diskid. %s could not " \
-                                              "be parsed" % self.ptable
+                                              "be parsed" % parsed.diskid
                                         raise KickStartError(err)
                                 else:
                                     try:
-- 
2.50.0.173.g8b6f19ccfc3a

[wic][PATCH 2/2] tests/unit/test_ksparser_parse: cover the .wks KickStart parser

[Trevor Woerner]

Add end-to-end coverage for KickStart(): happy-path part/bootloader
lines, degenerate files (empty, comment-only, CRLF, unknown directive),
mutually-exclusive and invalid option combinations, multiple-bootloader
rejection, diskid parsing for both ptables, and include handling. The
bitbake variable lookup is neutralised so the parser runs with no
BitBake and no host tools.

The msdos --diskid cases assert that a non-integer value raises a
KickStartError naming the offending value.

AI-Generated: codex/claude-opus 4.8 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
 tests/unit/test_ksparser_parse.py | 217 ++++++++++++++++++++++++++++++
 1 file changed, 217 insertions(+)
 create mode 100644 tests/unit/test_ksparser_parse.py

diff --git a/tests/unit/test_ksparser_parse.py b/tests/unit/test_ksparser_parse.py
new file mode 100644
index 000000000000..d5edc1250efc
--- /dev/null
+++ b/tests/unit/test_ksparser_parse.py
@@ -0,0 +1,217 @@
+r"""
+End-to-end tests for the KickStart .wks parser (wic/ksparser.py).
+
+KickStart(confpath) reads a .wks file line by line, runs each line
+through an argparse-based grammar, validates filesystem/option
+combinations, and builds Partition objects plus a bootloader config.
+It is pure logic apart from get_bitbake_var() (used for ${VAR}
+expansion and the bootloader APPEND), which these tests neutralise so
+no bitbake or host tools are needed.
+
+Each test writes a small .wks file and asserts on the parsed result or
+on the specific error raised.
+"""
+
+import sys
+from pathlib import Path
+
+import pytest
+
+_SRC = Path(__file__).resolve().parent.parent.parent / "src"
+if str(_SRC) not in sys.path:
+    sys.path.insert(0, str(_SRC))
+
+import wic.misc as misc
+import wic.ksparser as ksparser
+from wic.ksparser import KickStart, KickStartError
+
+
+@pytest.fixture(autouse=True)
+def no_bitbake(monkeypatch):
+    """Neutralise bitbake variable lookups in both modules.
+
+    get_bitbake_var is imported into ksparser's namespace and also used
+    via misc; patch both so ${VAR} expansion and APPEND resolve to None
+    instead of trying to run `bitbake -e`.
+    """
+    monkeypatch.setattr(misc, "get_bitbake_var", lambda *a, **k: None)
+    monkeypatch.setattr(ksparser, "get_bitbake_var", lambda *a, **k: None)
+
+
+def _wks(tmp_path, text):
+    path = tmp_path / "test.wks"
+    path.write_text(text)
+    return str(path)
+
+
+def _parse(tmp_path, text):
+    return KickStart(_wks(tmp_path, text))
+
+
+# ---------------------------------------------------------------------------
+# Happy path
+# ---------------------------------------------------------------------------
+
+class TestParseHappyPath:
+
+    def test_single_part(self, tmp_path):
+        ks = _parse(tmp_path, "part / --source rootfs --fstype ext4 --size 100M\n")
+        assert len(ks.partitions) == 1
+        p = ks.partitions[0]
+        assert p.fstype == "ext4"
+        assert p.size == 100 * 1024
+        assert p.mountpoint == "/"
+
+    def test_defaults_applied_without_fixed_size(self, tmp_path):
+        ks = _parse(tmp_path, "part / --source rootfs --fstype ext4 --size 1M\n")
+        p = ks.partitions[0]
+        assert p.overhead_factor == KickStart.DEFAULT_OVERHEAD_FACTOR
+        assert p.extra_filesystem_space == KickStart.DEFAULT_EXTRA_FILESYSTEM_SPACE
+
+    def test_partnum_increments(self, tmp_path):
+        ks = _parse(
+            tmp_path,
+            "part /boot --source bootimg --fstype vfat --size 50M\n"
+            "part / --source rootfs --fstype ext4 --size 100M\n",
+        )
+        assert [p.lineno for p in ks.partitions] == [1, 2]
+        assert len(ks.partitions) == 2
+
+    def test_bootloader_parsed(self, tmp_path):
+        ks = _parse(tmp_path, "bootloader --ptable gpt\n")
+        assert ks.bootloader.ptable == "gpt"
+
+    def test_comment_and_blank_lines_ignored(self, tmp_path):
+        ks = _parse(
+            tmp_path,
+            "# a comment\n"
+            "\n"
+            "   \n"
+            "part / --source rootfs --fstype ext4 --size 1M\n",
+        )
+        assert len(ks.partitions) == 1
+
+
+# ---------------------------------------------------------------------------
+# Degenerate files
+# ---------------------------------------------------------------------------
+
+class TestParseDegenerateFiles:
+
+    def test_empty_file(self, tmp_path):
+        ks = _parse(tmp_path, "")
+        assert ks.partitions == []
+        assert ks.bootloader is not None  # defaults filled in
+
+    def test_comment_only_file(self, tmp_path):
+        ks = _parse(tmp_path, "# only comments\n# nothing else\n")
+        assert ks.partitions == []
+
+    def test_crlf_line_endings(self, tmp_path):
+        ks = _parse(tmp_path, "part / --source rootfs --fstype ext4 --size 1M\r\n")
+        assert len(ks.partitions) == 1
+
+    def test_unknown_directive_rejected(self, tmp_path):
+        with pytest.raises(KickStartError):
+            _parse(tmp_path, "frobnicate everything\n")
+
+
+# ---------------------------------------------------------------------------
+# Mutually-exclusive and invalid option combinations
+# ---------------------------------------------------------------------------
+
+class TestParseInvalidCombinations:
+
+    def test_size_and_fixed_size_conflict(self, tmp_path):
+        with pytest.raises(KickStartError):
+            _parse(tmp_path, "part / --source rootfs --fstype ext4 "
+                             "--size 100M --fixed-size 200M\n")
+
+    def test_fixed_size_with_overhead_factor_rejected(self, tmp_path):
+        with pytest.raises(KickStartError):
+            _parse(tmp_path, "part / --source rootfs --fstype ext4 "
+                             "--fixed-size 100M --overhead-factor 1.5\n")
+
+    def test_squashfs_with_label_rejected(self, tmp_path):
+        with pytest.raises(KickStartError):
+            _parse(tmp_path, "part / --source rootfs --fstype squashfs --label boot\n")
+
+    def test_squashfs_with_fsuuid_rejected(self, tmp_path):
+        with pytest.raises(KickStartError):
+            _parse(tmp_path, "part / --source rootfs --fstype squashfs --fsuuid 0x1234\n")
+
+    def test_erofs_with_label_rejected(self, tmp_path):
+        with pytest.raises(KickStartError):
+            _parse(tmp_path, "part / --source rootfs --fstype erofs --label x --size 1M\n")
+
+    def test_use_label_without_label_rejected(self, tmp_path):
+        with pytest.raises(KickStartError):
+            _parse(tmp_path, "part / --source rootfs --fstype ext4 --size 1M --use-label\n")
+
+    def test_msdos_fsuuid_too_long_rejected(self, tmp_path):
+        with pytest.raises(KickStartError):
+            _parse(tmp_path, "part / --source rootfs --fstype msdos "
+                             "--size 1M --fsuuid 0xDEADBEEF12\n")
+
+
+# ---------------------------------------------------------------------------
+# Multiple bootloaders
+# ---------------------------------------------------------------------------
+
+class TestParseBootloader:
+
+    def test_two_bootloaders_rejected(self, tmp_path):
+        with pytest.raises(KickStartError):
+            _parse(tmp_path, "bootloader --ptable gpt\nbootloader --ptable msdos\n")
+
+    def test_msdos_diskid_integer(self, tmp_path):
+        ks = _parse(tmp_path, "bootloader --ptable msdos --diskid 0x12345678\n")
+        assert ks.bootloader.diskid == 0x12345678
+
+    def test_gpt_bad_diskid_raises_kickstart_error(self, tmp_path):
+        with pytest.raises(KickStartError):
+            _parse(tmp_path, "bootloader --ptable gpt --diskid not-a-uuid\n")
+
+    def test_msdos_bad_diskid_raises_kickstart_error(self, tmp_path):
+        # A non-integer --diskid on an msdos ptable must raise a described
+        # KickStartError, not an opaque AttributeError from the error path
+        # interpolating a missing attribute.
+        with pytest.raises(KickStartError) as excinfo:
+            _parse(tmp_path, "bootloader --ptable msdos --diskid not-an-int\n")
+        # The message names the offending value so the user can see what
+        # was rejected.
+        assert "not-an-int" in str(excinfo.value)
+
+    def test_msdos_bad_diskid_hex_string_rejected(self, tmp_path):
+        # int(x, 0) parses "0x..." but a stray non-hex digit still fails;
+        # the same described KickStartError path handles it.
+        with pytest.raises(KickStartError) as excinfo:
+            _parse(tmp_path, "bootloader --ptable msdos --diskid 0xZZ\n")
+        assert "0xZZ" in str(excinfo.value)
+
+    def test_gpt_diskid_uuid_parsed(self, tmp_path):
+        ks = _parse(
+            tmp_path,
+            "bootloader --ptable gpt "
+            "--diskid 12345678-1234-5678-1234-567812345678\n",
+        )
+        assert str(ks.bootloader.diskid) == \
+            "12345678-1234-5678-1234-567812345678"
+
+
+# ---------------------------------------------------------------------------
+# Include handling
+# ---------------------------------------------------------------------------
+
+class TestParseInclude:
+
+    def test_include_missing_file_rejected(self, tmp_path):
+        with pytest.raises(KickStartError):
+            _parse(tmp_path, "include does-not-exist.wks\n")
+
+    def test_include_pulls_in_partitions(self, tmp_path):
+        inc = tmp_path / "inc.wks"
+        inc.write_text("part / --source rootfs --fstype ext4 --size 5M\n")
+        ks = _parse(tmp_path, "include inc.wks\n")
+        assert len(ks.partitions) == 1
+        assert ks.partitions[0].size == 5 * 1024
-- 
2.50.0.173.g8b6f19ccfc3a