mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/7] TLV integration tests and test/py cleanup
@ 2025-09-26 10:14 Jonas Rebmann
  2025-09-26 10:14 ` [PATCH 1/7] test: when testfs feature is available, always enable it Jonas Rebmann
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Jonas Rebmann @ 2025-09-26 10:14 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Jonas Rebmann

With TLV signature coming up, we want to test all things TLV more
thoroughly, including "roundtrip" integration tests that include the
bareboxtlv-generator python-script and the tlv barebox-command.

As this is the third test module to make use of the testfs, it seemed
adequate to revisit its implementation.

Avoid integration test logic outside pytest; migrate logic from .github/
and scripts/ into pytest.

Use fixtures for testcase preconditions.

Cleanups here and there.

The ci-container needs to be rebuilt before the new tests will succeed
in CI.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Jonas Rebmann (7):
      test: when testfs feature is available, always enable it
      test: provide testfs via fixture
      test: move dm-verity testdata generation to fixture
      test: py: test_bootchooser: remove dead code
      commands: tlv: clarify error opening tlv
      ci: container: install crcmod and cryptography
      test: py: add TLV integration tests

 .github/workflows/test-labgrid-pytest.yml |  3 --
 commands/tlv.c                            |  8 ++--
 conftest.py                               | 14 ++++++
 scripts/generate_testfs.sh                | 44 -----------------
 test/Containerfile                        |  8 ++--
 test/py/test_bootchooser.py               |  4 --
 test/py/test_dm.py                        | 53 ++++++++++++++++++--
 test/py/test_fit.py                       |  9 +---
 test/py/test_tlv.py                       | 80 +++++++++++++++++++++++++++++++
 9 files changed, 153 insertions(+), 70 deletions(-)
---
base-commit: 7e32be40a193007e138c0eb589eb852420655523
change-id: 20250926-tlv-integration-945bdd7903d9

Best regards,
--  
Jonas Rebmann <jre@pengutronix.de>




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/7] test: when testfs feature is available, always enable it
  2025-09-26 10:14 [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann
@ 2025-09-26 10:14 ` Jonas Rebmann
  2025-09-26 10:14 ` [PATCH 2/7] test: provide testfs via fixture Jonas Rebmann
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Jonas Rebmann @ 2025-09-26 10:14 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Jonas Rebmann

Add 9pfs devices to the qemu commandline on all platforms that support
it (multi_v7, multi_v8) so tests relying upon it can be ran without
specifying --fs testfs=${KBUILD_OUTPUT}/testfs.

Moving logic out of the github workflow into pytest makes integration
tests more accessible to developers.

Introduce the fixture in existing test and drop obsolete test.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 .github/workflows/test-labgrid-pytest.yml | 3 ---
 conftest.py                               | 6 ++++++
 test/py/test_dm.py                        | 6 +-----
 test/py/test_fit.py                       | 4 ----
 4 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/.github/workflows/test-labgrid-pytest.yml b/.github/workflows/test-labgrid-pytest.yml
index ccf8ae172c..604f1db2fb 100644
--- a/.github/workflows/test-labgrid-pytest.yml
+++ b/.github/workflows/test-labgrid-pytest.yml
@@ -116,9 +116,6 @@ jobs:
           grep -wq '\(QEMUDriver\|ExternalConsoleDriver\):' "$i" || continue
 
           extraargs="${{matrix.lgargs}}"
-          if grep -wq 'testfs' "$i"; then
-            extraargs="${extraargs} --fs testfs=${KBUILD_OUTPUT}/testfs"
-          fi
 
           cfg=$(basename $i .yaml)
           echo "Testing $cfg"
diff --git a/conftest.py b/conftest.py
index 06321c938d..00969e62df 100644
--- a/conftest.py
+++ b/conftest.py
@@ -208,6 +208,12 @@ def strategy(request, target, pytestconfig):
     for arg in pytestconfig.option.qemu_arg:
         strategy.append_qemu_args(arg)
 
+    if "testfs" in features:
+        if not any(fs and fs[0] == "testfs" for fs in pytestconfig.option.qemu_fs):
+            testfs_path = os.path.join(os.environ["LG_BUILDDIR"], "testfs")
+            pytestconfig.option.qemu_fs.append(["testfs", testfs_path])
+            os.makedirs(testfs_path, exist_ok=True)
+
     for i, fs in enumerate(pytestconfig.option.qemu_fs):
         if virtio:
             path = fs.pop()
diff --git a/test/py/test_dm.py b/test/py/test_dm.py
index a9debd85b5..8bdacf7b4e 100644
--- a/test/py/test_dm.py
+++ b/test/py/test_dm.py
@@ -6,11 +6,7 @@ from .helper import of_get_property
 
 
 
-def test_dm_verity(barebox):
-    _, _, returncode = barebox.run("ls /mnt/9p/testfs")
-    if returncode != 0:
-        pytest.xfail("skipping test due to missing --fs testfs=")
-
+def test_dm_verity(barebox, testfs):
     barebox.run_check("cd /mnt/9p/testfs")
 
     # Since commands run in a subshell, export the root hash in a
diff --git a/test/py/test_fit.py b/test/py/test_fit.py
index 1a23a53a32..08d5feba5c 100644
--- a/test/py/test_fit.py
+++ b/test/py/test_fit.py
@@ -19,10 +19,6 @@ def test_fit(barebox, env, target, barebox_config):
     if 'testfs' not in env.get_target_features():
         pytest.xfail("testfs feature not specified")
 
-    _, _, returncode = barebox.run("ls /mnt/9p/testfs")
-    if returncode != 0:
-        pytest.xfail("skipping test due to missing --fs testfs=")
-
     _, _, returncode = barebox.run(f"ls {fit_name('gzipped')}")
     if returncode != 0:
         pytest.xfail("skipping test due to missing FIT image")

-- 
2.51.0.297.gca2559c1d6




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/7] test: provide testfs via fixture
  2025-09-26 10:14 [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann
  2025-09-26 10:14 ` [PATCH 1/7] test: when testfs feature is available, always enable it Jonas Rebmann
@ 2025-09-26 10:14 ` Jonas Rebmann
  2025-09-26 10:14 ` [PATCH 3/7] test: move dm-verity testdata generation to fixture Jonas Rebmann
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Jonas Rebmann @ 2025-09-26 10:14 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Jonas Rebmann

Move the logic of providing testfs out of the tests and into a fixture.
Pytest-modules that prepare testing data in their own fixtures should
build on top of this.

Tests using the fixture will be skipped if the feature is unavailable;
remove that check from existing tests.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 conftest.py         | 8 ++++++++
 test/py/test_fit.py | 5 +----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/conftest.py b/conftest.py
index 00969e62df..116beb5d14 100644
--- a/conftest.py
+++ b/conftest.py
@@ -231,3 +231,11 @@ def strategy(request, target, pytestconfig):
         strategy.force(state)
 
     return strategy
+
+@pytest.fixture(scope="session")
+def testfs(strategy, env):
+    if "testfs" not in env.get_target_features():
+        pytest.skip("testfs not supported on this platform")
+
+    path = os.path.join(os.environ["LG_BUILDDIR"], "testfs")
+    return path
diff --git a/test/py/test_fit.py b/test/py/test_fit.py
index 08d5feba5c..3b2b8ff871 100644
--- a/test/py/test_fit.py
+++ b/test/py/test_fit.py
@@ -15,10 +15,7 @@ def generate_bootscript(barebox, image, name="test"):
     return name
 
 
-def test_fit(barebox, env, target, barebox_config):
-    if 'testfs' not in env.get_target_features():
-        pytest.xfail("testfs feature not specified")
-
+def test_fit(barebox, env, target, barebox_config, testfs):
     _, _, returncode = barebox.run(f"ls {fit_name('gzipped')}")
     if returncode != 0:
         pytest.xfail("skipping test due to missing FIT image")

-- 
2.51.0.297.gca2559c1d6




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/7] test: move dm-verity testdata generation to fixture
  2025-09-26 10:14 [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann
  2025-09-26 10:14 ` [PATCH 1/7] test: when testfs feature is available, always enable it Jonas Rebmann
  2025-09-26 10:14 ` [PATCH 2/7] test: provide testfs via fixture Jonas Rebmann
@ 2025-09-26 10:14 ` Jonas Rebmann
  2025-09-26 14:25   ` Jonas Rebmann
  2025-09-26 10:14 ` [PATCH 4/7] test: py: test_bootchooser: remove dead code Jonas Rebmann
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Jonas Rebmann @ 2025-09-26 10:14 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Jonas Rebmann

Simplify barebox integration test setup by moving logic away from
scripts/ and .github/.

Instead of generating testdata in separate scripts, they should be
implemented as testfs fixtures which are automatically ran as part of
the test suite.

Includes error handling and cleanup.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 scripts/generate_testfs.sh | 44 ---------------------------------------
 test/py/test_dm.py         | 51 ++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 46 deletions(-)

diff --git a/scripts/generate_testfs.sh b/scripts/generate_testfs.sh
index 1c358ff846..3c200bd401 100755
--- a/scripts/generate_testfs.sh
+++ b/scripts/generate_testfs.sh
@@ -28,47 +28,3 @@ generate_fit()
 	       ${KBUILD_OUTPUT}/testfs/barebox-gzipped.fit
 }
 [ -f .github/testfs/${KBUILD_DEFCONFIG}-gzipped.its ] && generate_fit
-
-alias pad128k="dd if=/dev/zero bs=128k count=1 status=none"
-
-generate_dm_verity()
-{
-    work=$(mktemp -d)
-    cd ${work}
-
-    # Create two dummy files; use lots of padding to make sure that
-    # when we alter the contents of 'english' in root-bad, 'latin' is
-    # still be readable, as their contents wont (a) share the same
-    # hash block and (b) the block cache layer won't accedentally read
-    # the invalid block.
-
-    pad128k  >latin
-    echo -n "veritas vos liberabit" >>latin
-    pad128k >>latin
-
-    pad128k  >english
-    echo -n "truth will set you free" >>english
-    pad128k >>english
-
-    truncate -s 1M good.fat
-    mkfs.vfat good.fat
-    mcopy -i good.fat latin english ::
-
-    veritysetup format \
-		--root-hash-file=good.hash \
-		good.fat good.verity
-
-    sed 's/truth will set you free/LIAR LIAR PANTS ON FIRE/' \
-	<good.fat >bad.fat
-
-    cd -
-    cp \
-	${work}/good.fat \
-	${work}/good.verity \
-	${work}/good.hash \
-	${work}/bad.fat \
-	${KBUILD_OUTPUT}/testfs
-
-    rm -rf ${work}
-}
-generate_dm_verity
diff --git a/test/py/test_dm.py b/test/py/test_dm.py
index 8bdacf7b4e..837881f4af 100644
--- a/test/py/test_dm.py
+++ b/test/py/test_dm.py
@@ -4,10 +4,57 @@ import re
 import pytest
 from .helper import of_get_property
 
+import os, subprocess, shutil
 
+def pad128k(f):
+    f.write(b"\0" * 128 * 1024)
 
-def test_dm_verity(barebox, testfs):
-    barebox.run_check("cd /mnt/9p/testfs")
+@pytest.fixture(scope="module")
+def dm_testdata(testfs):
+    path = os.path.join(testfs, "dm")
+    os.makedirs(path, exist_ok=True)
+    cwd = os.getcwd()
+    os.chdir(path)
+
+    with open("latin", "wb") as f:
+        pad128k(f)
+        f.write(b"veritas vos liberabit")
+        pad128k(f)
+
+    with open("english", "wb") as f:
+        pad128k(f)
+        f.write(b"truth will set you free")
+        pad128k(f)
+
+    try:
+        subprocess.run(["truncate", "-s", "1M", "good.fat"], check=True)
+        subprocess.run(["mkfs.vfat", "good.fat"], check=True)
+        subprocess.run(["mcopy", "-i", "good.fat", "latin", "english", "::"], check=True)
+
+        subprocess.run([
+            "veritysetup", "format",
+            "--root-hash-file=good.hash",
+            "good.fat", "good.verity"
+        ], check=True)
+    except FileNotFoundError as e:
+        pytest.skip(f"Skip dm tests due to missing dependency: {e}")
+
+    with open("good.fat", "rb") as f: data = f.read()
+    with open("bad.fat", "wb") as f:
+        f.write(data.replace(
+            b"truth will set you free",
+            b"LIAR LIAR PANTS ON FIRE"
+        ))
+
+    os.chdir(cwd)
+
+    yield path
+
+    shutil.rmtree(path)
+
+
+def test_dm_verity(barebox, dm_testdata):
+    barebox.run_check("cd /mnt/9p/testfs/dm")
 
     # Since commands run in a subshell, export the root hash in a
     # global, so that we can access it from subsequent commands

-- 
2.51.0.297.gca2559c1d6




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 4/7] test: py: test_bootchooser: remove dead code
  2025-09-26 10:14 [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann
                   ` (2 preceding siblings ...)
  2025-09-26 10:14 ` [PATCH 3/7] test: move dm-verity testdata generation to fixture Jonas Rebmann
@ 2025-09-26 10:14 ` Jonas Rebmann
  2025-09-26 10:14 ` [PATCH 5/7] commands: tlv: clarify error opening tlv Jonas Rebmann
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Jonas Rebmann @ 2025-09-26 10:14 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Jonas Rebmann

This seems to have landed here by accident.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 test/py/test_bootchooser.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/test/py/test_bootchooser.py b/test/py/test_bootchooser.py
index a5f2b25d65..bd8aee7399 100644
--- a/test/py/test_bootchooser.py
+++ b/test/py/test_bootchooser.py
@@ -5,10 +5,6 @@ from .helper import globalvars_set, devinfo, getparam_int, \
                     getstate_int, getenv_int
 
 
-def fit_name(suffix):
-    return f"/mnt/9p/testfs/barebox-{suffix}.fit"
-
-
 def generate_bootscript(barebox, command, name="test"):
     barebox.run_check(f"echo -o /env/boot/{name} '#!/bin/sh'")
     barebox.run_check(f"echo -a /env/boot/{name} '{command}'")

-- 
2.51.0.297.gca2559c1d6




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 5/7] commands: tlv: clarify error opening tlv
  2025-09-26 10:14 [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann
                   ` (3 preceding siblings ...)
  2025-09-26 10:14 ` [PATCH 4/7] test: py: test_bootchooser: remove dead code Jonas Rebmann
@ 2025-09-26 10:14 ` Jonas Rebmann
  2025-09-26 10:14 ` [PATCH 6/7] ci: container: install crcmod and cryptography Jonas Rebmann
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Jonas Rebmann @ 2025-09-26 10:14 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Jonas Rebmann

Returning PTR_ERR(tlvdev) instead of printing an error message leads to
the same output when CONFIG_CMD_TLV and when the specified path could
not be read:

  tlv: No such file or directory

In the latter case, print an explanatory message instead.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 commands/tlv.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/commands/tlv.c b/commands/tlv.c
index 53fbc2a291..6118a2947f 100644
--- a/commands/tlv.c
+++ b/commands/tlv.c
@@ -28,9 +28,11 @@ static int do_tlv(int argc, char *argv[])
 	if (!filename)
 		return COMMAND_ERROR_USAGE;
 
-	tlvdev = tlv_register_device_by_path(argv[optind], NULL);
-	if (IS_ERR(tlvdev))
-		return PTR_ERR(tlvdev);
+	tlvdev = tlv_register_device_by_path(filename, NULL);
+	if (IS_ERR(tlvdev)) {
+		printf("Could not open \"%s\": %m\n", filename);
+		return COMMAND_ERROR;
+	}
 
 	if (fixup)
 		return tlv_of_register_fixup(tlvdev);

-- 
2.51.0.297.gca2559c1d6




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 6/7] ci: container: install crcmod and cryptography
  2025-09-26 10:14 [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann
                   ` (4 preceding siblings ...)
  2025-09-26 10:14 ` [PATCH 5/7] commands: tlv: clarify error opening tlv Jonas Rebmann
@ 2025-09-26 10:14 ` Jonas Rebmann
  2025-09-26 10:14 ` [PATCH 7/7] test: py: add TLV integration tests Jonas Rebmann
  2025-09-26 13:57 ` [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann
  7 siblings, 0 replies; 13+ messages in thread
From: Jonas Rebmann @ 2025-09-26 10:14 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Jonas Rebmann

Needed for bareboxtlv-generator.py which will be part of integration
tests.

While at it, sort python packages alphabetically.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 test/Containerfile | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/test/Containerfile b/test/Containerfile
index c6c3c57596..f2e785d17c 100644
--- a/test/Containerfile
+++ b/test/Containerfile
@@ -43,11 +43,13 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
 	qemu-system-common \
 	ovmf \
 	python3 \
-	python3-pip \
-	python3-virtualenv \
-	python3-setuptools \
+	python3-crcmod \
+	python3-cryptography \
 	python3-jsonschema \
 	python3-libfdt \
+	python3-pip \
+	python3-setuptools \
+	python3-virtualenv \
 	python3-yaml \
 	virtualenv \
 	sudo \

-- 
2.51.0.297.gca2559c1d6




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 7/7] test: py: add TLV integration tests
  2025-09-26 10:14 [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann
                   ` (5 preceding siblings ...)
  2025-09-26 10:14 ` [PATCH 6/7] ci: container: install crcmod and cryptography Jonas Rebmann
@ 2025-09-26 10:14 ` Jonas Rebmann
  2025-09-26 13:57 ` [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann
  7 siblings, 0 replies; 13+ messages in thread
From: Jonas Rebmann @ 2025-09-26 10:14 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Jonas Rebmann

With TLV signature coming up, we want to test all things TLV more
thoroughly, including "roundtrip" integration tests that include the
bareboxtlv-generator python-script and the tlv barebox-command.

 - Encode the example TLV data
 - Add a "corrupted" variant of that tlv binary with a bit error
 - Test decoding those binaries using bareboxtlv-generator
 - Test decoding those binaries using the tlv-command

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 test/py/test_tlv.py | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/test/py/test_tlv.py b/test/py/test_tlv.py
new file mode 100644
index 0000000000..3e8faafd25
--- /dev/null
+++ b/test/py/test_tlv.py
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import os
+import re
+import subprocess
+from pathlib import Path
+from crcmod.predefined import mkPredefinedCrcFun
+
+_crc32_mpeg = mkPredefinedCrcFun("crc-32-mpeg")
+
+import pytest
+
+
+class _TLV_Testdata:
+    def generator(self, args, check=True):
+        cmd = [os.sys.executable, str(self.generator_py)] + args
+        res = subprocess.run(cmd, text=True)
+        if check and res.returncode != 0:
+            raise RuntimeError(f"generator failed ({res.returncode}): {res.stdout}\n{res.stderr}")
+        return res
+
+    def __init__(self, testfs):
+        self.dir = Path(testfs)
+        self.scripts_dir = Path("scripts/bareboxtlv-generator")
+        self.data = self.scripts_dir / "data-example.yaml"
+        self.schema = self.scripts_dir / "schema-example.yaml"
+        self.generator_py = self.scripts_dir / "bareboxtlv-generator.py"
+        self.unsigned_bin = self.dir / 'unsigned.tlv'
+        self.corrupted_bin = self.dir / 'unsigned_corrupted.tlv'
+
+@pytest.fixture(scope="module")
+def tlv_testdata(testfs):
+    t = _TLV_Testdata(testfs)
+    t.generator(["--input-data", str(t.data), str(t.schema), str(t.unsigned_bin)])
+    assert t.unsigned_bin.exists(), "unsigned TLV not created"
+
+    with open(t.unsigned_bin, 'r+b') as f:
+        data = bytearray(f.read())
+    data[0x20] ^= 1
+    with open(t.corrupted_bin, "wb") as f:
+        f.write(data)
+
+    return t
+
+def test_tlv_generator(tlv_testdata):
+    t = tlv_testdata
+    out_yaml = t.dir / 'out.yaml'
+
+
+    good = t.generator(["--output-data", str(out_yaml), str(t.schema), str(t.unsigned_bin)], check=False)
+    assert good.returncode == 0, f"valid unsigned TLV failed to decode: {good.stderr}\n{good.stdout}"
+
+    bad = t.generator(["--output-data", str(t.dir / 'bad.yaml'), str(t.schema), str(t.corrupted_bin)], check=False)
+    assert bad.returncode != 0, "unsigned TLV with invalid CRC unexpectedly decoded successfully"
+
+def test_tlv_command(barebox, tlv_testdata):
+    t = tlv_testdata
+    _, _, rc = barebox.run("ls /mnt/9p/testfs")
+    if rc != 0:
+        pytest.xfail("skipping test due to missing --fs testfs=")
+
+    with open(t.data, 'r', encoding='utf-8') as f:
+        yaml_lines = [l.strip() for l in f if l.strip() and not l.strip().startswith('#')]
+
+    stdout = barebox.run_check(f"tlv /mnt/9p/testfs/{t.unsigned_bin.name}")
+    tlv_lines = stdout[1:-1]
+
+    assert len(yaml_lines) == len(tlv_lines), \
+        f"YAML and TLV output line count mismatch for {t.unsigned_bin.name}"
+
+    for yline, tline in zip(yaml_lines, tlv_lines):
+        ykey, yval = [x.strip() for x in yline.split(':', 1)]
+        m = re.match(r'^\s*([^=]+) = "(.*)";$', tline)
+        assert m, f"malformed tlv line: {tline}"
+        tkey, tval = m.group(1), m.group(2)
+        m = re.match(r'^([^:]+):\s*(?:"([^"]*)"\s*|(.*))$', yline)
+        assert m, f"malformed yaml line: {yline}"
+        ykey, yval = m.group(1), m.group(2) or m.group(3)
+        assert ykey == tkey, f"key mismatch: {ykey} != {tkey}"
+        assert str(yval) == str(tval), f"value mismatch for {ykey}: {yval} != {tval}"

-- 
2.51.0.297.gca2559c1d6




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/7] TLV integration tests and test/py cleanup
  2025-09-26 10:14 [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann
                   ` (6 preceding siblings ...)
  2025-09-26 10:14 ` [PATCH 7/7] test: py: add TLV integration tests Jonas Rebmann
@ 2025-09-26 13:57 ` Jonas Rebmann
  7 siblings, 0 replies; 13+ messages in thread
From: Jonas Rebmann @ 2025-09-26 13:57 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX

Hi,

On 2025-09-26 12:14, Jonas Rebmann wrote:
> With TLV signature coming up, we want to test all things TLV more
> thoroughly, including "roundtrip" integration tests that include the
> bareboxtlv-generator python-script and the tlv barebox-command.
> 
> As this is the third test module to make use of the testfs, it seemed
> adequate to revisit its implementation.

I was too hasty when preparing this series and only ran CI after
sending. CI fails on more than one end. That I forgot the patch that
enables CONFIG_CMD_TLV and didn't add a skip_disabled(barebox_config,
"CONFIG_CMD_TLV") for the TLV tests is only the tip of the iceberg.

I'll send a v2 shortly.

Regards,
Jonas

-- 
Pengutronix e.K.                           | Jonas Rebmann               |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/7] test: move dm-verity testdata generation to fixture
  2025-09-26 10:14 ` [PATCH 3/7] test: move dm-verity testdata generation to fixture Jonas Rebmann
@ 2025-09-26 14:25   ` Jonas Rebmann
  2025-09-28  9:54     ` Tobias Waldekranz
  2025-09-29  7:53     ` Sascha Hauer
  0 siblings, 2 replies; 13+ messages in thread
From: Jonas Rebmann @ 2025-09-26 14:25 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX, Tobias Waldekranz, Ahmad Fatoum

Hi all,

On 2025-09-26 12:14, Jonas Rebmann wrote:
> Simplify barebox integration test setup by moving logic away from
> scripts/ and .github/.
> 
> Instead of generating testdata in separate scripts, they should be
> implemented as testfs fixtures which are automatically ran as part of
> the test suite.
> 
> Includes error handling and cleanup.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>   scripts/generate_testfs.sh | 44 ---------------------------------------
>   test/py/test_dm.py         | 51 ++++++++++++++++++++++++++++++++++++++++++++--
>   2 files changed, 49 insertions(+), 46 deletions(-)
> 
> diff --git a/scripts/generate_testfs.sh b/scripts/generate_testfs.sh
> index 1c358ff846..3c200bd401 100755
> --- a/scripts/generate_testfs.sh
> +++ b/scripts/generate_testfs.sh
> @@ -28,47 +28,3 @@ generate_fit()
>   	       ${KBUILD_OUTPUT}/testfs/barebox-gzipped.fit
>   }
>   [ -f .github/testfs/${KBUILD_DEFCONFIG}-gzipped.its ] && generate_fit

It took me some time to figure out why this patch broke CI.

With the set -e in this script, this last line of fit generation logic
should have been

   [ -f .github/testfs/${KBUILD_DEFCONFIG}-gzipped.its ] && generate_fit || true

given the intent here is instead of erroring out, to just skip fit
generation if the its does not exist.

The only reason CI didn't break when Tobias added mipsel and riscv, for
which no .github/testfs/${KBUILD_DEFCONFIG}-gzipped.its exist, in commit
739a1a7855f5 ("ci: pytest: Open up testfs to more consumers than the FIT
test"), is that the github workflow (incorrectly) invokes the script
with exec. This is why only when this check became the last line of the
script, the error return code propagated into github ci.

I would like to drop scripts/generate_testfs.sh entirely in v2,
migrating the generate_fit logic to a pytest fixture too.

The reason I haven't yet is mainly that there we cannot rely on
KBUILD_DEFCONFIG to select the test data.

I thought about selecting the its from something in barebox_config or
something in strategy.qemu instead. Any suggestions?

And I'd like to also move those testfs files out of .github. I would
like to consider the pytest tests something more than just artifacts of
the github workflow.

As this excursion into cleanup around the integration tests is getting a
bit more involved than I hoped, I'm happy about feedback already while
I'm working on v2.

> -
> -alias pad128k="dd if=/dev/zero bs=128k count=1 status=none"
> -
> -generate_dm_verity()
> -{
> -    work=$(mktemp -d)
> -    cd ${work}
> -
> -    # Create two dummy files; use lots of padding to make sure that
> -    # when we alter the contents of 'english' in root-bad, 'latin' is
> -    # still be readable, as their contents wont (a) share the same
> -    # hash block and (b) the block cache layer won't accedentally read
> -    # the invalid block.
> -
> -    pad128k  >latin
> -    echo -n "veritas vos liberabit" >>latin
> -    pad128k >>latin
> -
> -    pad128k  >english
> -    echo -n "truth will set you free" >>english
> -    pad128k >>english
> -
> -    truncate -s 1M good.fat
> -    mkfs.vfat good.fat
> -    mcopy -i good.fat latin english ::
> -
> -    veritysetup format \
> -		--root-hash-file=good.hash \
> -		good.fat good.verity
> -
> -    sed 's/truth will set you free/LIAR LIAR PANTS ON FIRE/' \
> -	<good.fat >bad.fat
> -
> -    cd -
> -    cp \
> -	${work}/good.fat \
> -	${work}/good.verity \
> -	${work}/good.hash \
> -	${work}/bad.fat \
> -	${KBUILD_OUTPUT}/testfs
> -
> -    rm -rf ${work}
> -}
> -generate_dm_verity
> diff --git a/test/py/test_dm.py b/test/py/test_dm.py
> index 8bdacf7b4e..837881f4af 100644
> --- a/test/py/test_dm.py
> +++ b/test/py/test_dm.py
> @@ -4,10 +4,57 @@ import re
>   import pytest
>   from .helper import of_get_property
>   
> +import os, subprocess, shutil
>   
> +def pad128k(f):
> +    f.write(b"\0" * 128 * 1024)
>   
> -def test_dm_verity(barebox, testfs):
> -    barebox.run_check("cd /mnt/9p/testfs")
> +@pytest.fixture(scope="module")
> +def dm_testdata(testfs):
> +    path = os.path.join(testfs, "dm")
> +    os.makedirs(path, exist_ok=True)
> +    cwd = os.getcwd()
> +    os.chdir(path)
> +
> +    with open("latin", "wb") as f:
> +        pad128k(f)
> +        f.write(b"veritas vos liberabit")
> +        pad128k(f)
> +
> +    with open("english", "wb") as f:
> +        pad128k(f)
> +        f.write(b"truth will set you free")
> +        pad128k(f)
> +
> +    try:
> +        subprocess.run(["truncate", "-s", "1M", "good.fat"], check=True)
> +        subprocess.run(["mkfs.vfat", "good.fat"], check=True)
> +        subprocess.run(["mcopy", "-i", "good.fat", "latin", "english", "::"], check=True)
> +
> +        subprocess.run([
> +            "veritysetup", "format",
> +            "--root-hash-file=good.hash",
> +            "good.fat", "good.verity"
> +        ], check=True)
> +    except FileNotFoundError as e:
> +        pytest.skip(f"Skip dm tests due to missing dependency: {e}")
> +
> +    with open("good.fat", "rb") as f: data = f.read()
> +    with open("bad.fat", "wb") as f:
> +        f.write(data.replace(
> +            b"truth will set you free",
> +            b"LIAR LIAR PANTS ON FIRE"
> +        ))
> +
> +    os.chdir(cwd)
> +
> +    yield path
> +
> +    shutil.rmtree(path)
> +
> +
> +def test_dm_verity(barebox, dm_testdata):
> +    barebox.run_check("cd /mnt/9p/testfs/dm")
>   
>       # Since commands run in a subshell, export the root hash in a
>       # global, so that we can access it from subsequent commands
> 

Regards,
Jonas

-- 
Pengutronix e.K.                           | Jonas Rebmann               |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/7] test: move dm-verity testdata generation to fixture
  2025-09-26 14:25   ` Jonas Rebmann
@ 2025-09-28  9:54     ` Tobias Waldekranz
  2025-09-29  8:29       ` Jonas Rebmann
  2025-09-29  7:53     ` Sascha Hauer
  1 sibling, 1 reply; 13+ messages in thread
From: Tobias Waldekranz @ 2025-09-28  9:54 UTC (permalink / raw)
  To: Jonas Rebmann, Sascha Hauer, BAREBOX, Ahmad Fatoum

On fre, sep 26, 2025 at 16:25, Jonas Rebmann <jre@pengutronix.de> wrote:
> Hi all,
>
> On 2025-09-26 12:14, Jonas Rebmann wrote:
>> Simplify barebox integration test setup by moving logic away from
>> scripts/ and .github/.
>> 
>> Instead of generating testdata in separate scripts, they should be
>> implemented as testfs fixtures which are automatically ran as part of
>> the test suite.
>> 
>> Includes error handling and cleanup.
>> 
>> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
>> ---
>>   scripts/generate_testfs.sh | 44 ---------------------------------------
>>   test/py/test_dm.py         | 51 ++++++++++++++++++++++++++++++++++++++++++++--
>>   2 files changed, 49 insertions(+), 46 deletions(-)
>> 
>> diff --git a/scripts/generate_testfs.sh b/scripts/generate_testfs.sh
>> index 1c358ff846..3c200bd401 100755
>> --- a/scripts/generate_testfs.sh
>> +++ b/scripts/generate_testfs.sh
>> @@ -28,47 +28,3 @@ generate_fit()
>>   	       ${KBUILD_OUTPUT}/testfs/barebox-gzipped.fit
>>   }
>>   [ -f .github/testfs/${KBUILD_DEFCONFIG}-gzipped.its ] && generate_fit
>
> It took me some time to figure out why this patch broke CI.
>
> With the set -e in this script, this last line of fit generation logic
> should have been
>
>    [ -f .github/testfs/${KBUILD_DEFCONFIG}-gzipped.its ] && generate_fit || true
>
> given the intent here is instead of erroring out, to just skip fit
> generation if the its does not exist.

Quite right. Sorry for leaving that foot gun there :)

> The only reason CI didn't break when Tobias added mipsel and riscv, for
> which no .github/testfs/${KBUILD_DEFCONFIG}-gzipped.its exist, in commit
> 739a1a7855f5 ("ci: pytest: Open up testfs to more consumers than the FIT
> test"), is that the github workflow (incorrectly) invokes the script
> with exec. This is why only when this check became the last line of the
> script, the error return code propagated into github ci.
>
> I would like to drop scripts/generate_testfs.sh entirely in v2,
> migrating the generate_fit logic to a pytest fixture too.
>
> The reason I haven't yet is mainly that there we cannot rely on
> KBUILD_DEFCONFIG to select the test data.
>
> I thought about selecting the its from something in barebox_config or
> something in strategy.qemu instead. Any suggestions?
>
> And I'd like to also move those testfs files out of .github. I would
> like to consider the pytest tests something more than just artifacts of
> the github workflow.
>
> As this excursion into cleanup around the integration tests is getting a
> bit more involved than I hoped, I'm happy about feedback already while
> I'm working on v2.

I'll leave some thoughts that went through my mind as I was doing the dm
work. This is from the perspective of someone with almost no experience
of neither pytest in general, nor of labgrid.

It would be great if much of the logic from the workflow file could be
moved to a regular Make target or something, so that it is easier to
answer questions like "Will the CI job run with testfs for this config
now?"

As an example, in Infix, our GitHub workflow for launching the test
suite is just 'make test'. This will then launch a docker container,
which in turn runs a bunch of QEMU instances and the tests. One very
nice property with this approach is that it is very easy to launch the
exact same test environment on your local machine.

>> -
>> -alias pad128k="dd if=/dev/zero bs=128k count=1 status=none"
>> -
>> -generate_dm_verity()
>> -{
>> -    work=$(mktemp -d)
>> -    cd ${work}
>> -
>> -    # Create two dummy files; use lots of padding to make sure that
>> -    # when we alter the contents of 'english' in root-bad, 'latin' is
>> -    # still be readable, as their contents wont (a) share the same
>> -    # hash block and (b) the block cache layer won't accedentally read
>> -    # the invalid block.
>> -
>> -    pad128k  >latin
>> -    echo -n "veritas vos liberabit" >>latin
>> -    pad128k >>latin
>> -
>> -    pad128k  >english
>> -    echo -n "truth will set you free" >>english
>> -    pad128k >>english
>> -
>> -    truncate -s 1M good.fat
>> -    mkfs.vfat good.fat
>> -    mcopy -i good.fat latin english ::
>> -
>> -    veritysetup format \
>> -		--root-hash-file=good.hash \
>> -		good.fat good.verity
>> -
>> -    sed 's/truth will set you free/LIAR LIAR PANTS ON FIRE/' \
>> -	<good.fat >bad.fat
>> -
>> -    cd -
>> -    cp \
>> -	${work}/good.fat \
>> -	${work}/good.verity \
>> -	${work}/good.hash \
>> -	${work}/bad.fat \
>> -	${KBUILD_OUTPUT}/testfs
>> -
>> -    rm -rf ${work}
>> -}
>> -generate_dm_verity
>> diff --git a/test/py/test_dm.py b/test/py/test_dm.py
>> index 8bdacf7b4e..837881f4af 100644
>> --- a/test/py/test_dm.py
>> +++ b/test/py/test_dm.py
>> @@ -4,10 +4,57 @@ import re
>>   import pytest
>>   from .helper import of_get_property
>>   
>> +import os, subprocess, shutil
>>   
>> +def pad128k(f):
>> +    f.write(b"\0" * 128 * 1024)
>>   
>> -def test_dm_verity(barebox, testfs):
>> -    barebox.run_check("cd /mnt/9p/testfs")
>> +@pytest.fixture(scope="module")
>> +def dm_testdata(testfs):
>> +    path = os.path.join(testfs, "dm")
>> +    os.makedirs(path, exist_ok=True)
>> +    cwd = os.getcwd()
>> +    os.chdir(path)
>> +
>> +    with open("latin", "wb") as f:
>> +        pad128k(f)
>> +        f.write(b"veritas vos liberabit")
>> +        pad128k(f)
>> +
>> +    with open("english", "wb") as f:
>> +        pad128k(f)
>> +        f.write(b"truth will set you free")
>> +        pad128k(f)
>> +
>> +    try:
>> +        subprocess.run(["truncate", "-s", "1M", "good.fat"], check=True)
>> +        subprocess.run(["mkfs.vfat", "good.fat"], check=True)
>> +        subprocess.run(["mcopy", "-i", "good.fat", "latin", "english", "::"], check=True)
>> +
>> +        subprocess.run([
>> +            "veritysetup", "format",
>> +            "--root-hash-file=good.hash",
>> +            "good.fat", "good.verity"
>> +        ], check=True)
>> +    except FileNotFoundError as e:
>> +        pytest.skip(f"Skip dm tests due to missing dependency: {e}")
>> +
>> +    with open("good.fat", "rb") as f: data = f.read()
>> +    with open("bad.fat", "wb") as f:
>> +        f.write(data.replace(
>> +            b"truth will set you free",
>> +            b"LIAR LIAR PANTS ON FIRE"
>> +        ))
>> +
>> +    os.chdir(cwd)
>> +
>> +    yield path
>> +
>> +    shutil.rmtree(path)
>> +
>> +
>> +def test_dm_verity(barebox, dm_testdata):
>> +    barebox.run_check("cd /mnt/9p/testfs/dm")
>>   
>>       # Since commands run in a subshell, export the root hash in a
>>       # global, so that we can access it from subsequent commands
>> 
>
> Regards,
> Jonas
>
> -- 
> Pengutronix e.K.                           | Jonas Rebmann               |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/7] test: move dm-verity testdata generation to fixture
  2025-09-26 14:25   ` Jonas Rebmann
  2025-09-28  9:54     ` Tobias Waldekranz
@ 2025-09-29  7:53     ` Sascha Hauer
  1 sibling, 0 replies; 13+ messages in thread
From: Sascha Hauer @ 2025-09-29  7:53 UTC (permalink / raw)
  To: Jonas Rebmann; +Cc: BAREBOX, Tobias Waldekranz, Ahmad Fatoum

On Fri, Sep 26, 2025 at 04:25:49PM +0200, Jonas Rebmann wrote:
> Hi all,
> 
> On 2025-09-26 12:14, Jonas Rebmann wrote:
> > Simplify barebox integration test setup by moving logic away from
> > scripts/ and .github/.
> > 
> > Instead of generating testdata in separate scripts, they should be
> > implemented as testfs fixtures which are automatically ran as part of
> > the test suite.
> > 
> > Includes error handling and cleanup.
> > 
> > Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> > ---
> >   scripts/generate_testfs.sh | 44 ---------------------------------------
> >   test/py/test_dm.py         | 51 ++++++++++++++++++++++++++++++++++++++++++++--
> >   2 files changed, 49 insertions(+), 46 deletions(-)
> > 
> > diff --git a/scripts/generate_testfs.sh b/scripts/generate_testfs.sh
> > index 1c358ff846..3c200bd401 100755
> > --- a/scripts/generate_testfs.sh
> > +++ b/scripts/generate_testfs.sh
> > @@ -28,47 +28,3 @@ generate_fit()
> >   	       ${KBUILD_OUTPUT}/testfs/barebox-gzipped.fit
> >   }
> >   [ -f .github/testfs/${KBUILD_DEFCONFIG}-gzipped.its ] && generate_fit
> 
> It took me some time to figure out why this patch broke CI.
> 
> With the set -e in this script, this last line of fit generation logic
> should have been
> 
>   [ -f .github/testfs/${KBUILD_DEFCONFIG}-gzipped.its ] && generate_fit || true

I fixed this up in -next so that we no longer have this in the tree when
you send your next series.

I've rewritten this to:

if [ -f .github/testfs/${KBUILD_DEFCONFIG}-gzipped.its ]; then
	generate_fit
fi

for better readability.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/7] test: move dm-verity testdata generation to fixture
  2025-09-28  9:54     ` Tobias Waldekranz
@ 2025-09-29  8:29       ` Jonas Rebmann
  0 siblings, 0 replies; 13+ messages in thread
From: Jonas Rebmann @ 2025-09-29  8:29 UTC (permalink / raw)
  To: Tobias Waldekranz, Sascha Hauer, BAREBOX, Ahmad Fatoum

Hi Tobias,

Thanks for your reply.

On 2025-09-28 11:54, Tobias Waldekranz wrote:
> I'll leave some thoughts that went through my mind as I was doing the dm
> work. This is from the perspective of someone with almost no experience
> of neither pytest in general, nor of labgrid.
> 
> It would be great if much of the logic from the workflow file could be
> moved to a regular Make target or something, so that it is easier to
> answer questions like "Will the CI job run with testfs for this config
> now?"

I agree that there should be a simple way to "make test" and that this
should closely resemble what we run in CI. With all the skip and xfail
logic we have, it can happen quite easily to have pytest succeed when
run manually and then fail on CI.

What I did to replicate the github CI was using "act" with -r (to keep
containers running) and podman. Then after the run I opened a shell in
the container to tinker with the CI failure. This approach is not
something we can present in the docs to explain to contributors how to
get the CI green.

Agreed that we should move the build, kconfig, test invocation logic
from the github workflow to a script. Maybe a Makefile, maybe just
shell, and invoke that from the workflow yaml. Then we document how
contributors can invoke that script in the barebox ci container to
replicate what happens on github.

I didn't get to do it this time unfortunately.

Maybe next time I touch the pytests if the list here agrees on this
direction.

Regards,
Jonas

-- 
Pengutronix e.K.                           | Jonas Rebmann               |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-09-29  8:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-26 10:14 [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann
2025-09-26 10:14 ` [PATCH 1/7] test: when testfs feature is available, always enable it Jonas Rebmann
2025-09-26 10:14 ` [PATCH 2/7] test: provide testfs via fixture Jonas Rebmann
2025-09-26 10:14 ` [PATCH 3/7] test: move dm-verity testdata generation to fixture Jonas Rebmann
2025-09-26 14:25   ` Jonas Rebmann
2025-09-28  9:54     ` Tobias Waldekranz
2025-09-29  8:29       ` Jonas Rebmann
2025-09-29  7:53     ` Sascha Hauer
2025-09-26 10:14 ` [PATCH 4/7] test: py: test_bootchooser: remove dead code Jonas Rebmann
2025-09-26 10:14 ` [PATCH 5/7] commands: tlv: clarify error opening tlv Jonas Rebmann
2025-09-26 10:14 ` [PATCH 6/7] ci: container: install crcmod and cryptography Jonas Rebmann
2025-09-26 10:14 ` [PATCH 7/7] test: py: add TLV integration tests Jonas Rebmann
2025-09-26 13:57 ` [PATCH 0/7] TLV integration tests and test/py cleanup Jonas Rebmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox