All of lore.kernel.org
 help / color / mirror / Atom feed
* [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test
@ 2026-07-01  7:40 Trevor Woerner
  2026-07-01  7:40 ` [wic][PATCH v3 01/10] tests: add the standalone test-suite skeleton Trevor Woerner
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Trevor Woerner @ 2026-07-01  7:40 UTC (permalink / raw)
  To: yocto-patches

v1 sent the whole test suite as a single 6000-line commit that leaned on
xfail markers to record the bugs it found. Review feedback was that this
is not reviewable: the request was for a series of small commits where
each test lands green next to the source fix that makes it pass, with no
xfails and no failing tests at any commit boundary.

This series is that rework. Rather than resend the entire suite at once,
it establishes the test framework and a single representative test so the
structure and conventions can be reviewed first. Later functions will
follow as their own per-function commits once this base is agreed.

The series contains:

  - the suite skeleton (pyproject test config, layout, .gitignore,
    README Testing section);
  - the conftest session banner;
  - the run-tests.sh wrapper, with optional coverage and ruff linting
    added in their own commits;
  - the suite docs (overview README, authoring guide, and review rubric)
    and the one ruff per-file-ignore the sys.path bootstrap needs;
  - the first unit test, tests/unit/test_bb_utils, which tests
    mkdirhier() and carries the one-line errno-import fix the test
    exposes, so the suite is green.

The whole suite is green at every commit (zero failures, zero xfails)
and tests/run-tests.sh --lint-tests is clean throughout.

Changes in v3:

  - tests/unit/test_bb_utils now uses pytest's tmp_path fixture instead
    of tempfile.mkdtemp(), so the tests no longer leak scratch
    directories under /tmp.
  - the suite README documents the standard pytest scratch-directory
    controls (TMPDIR and --basetemp), so no custom temporary-directory
    variable is needed.
  - a new commit adds tests/docs/reviewing.md, the rubric a test change
    is reviewed against, linked from the suite README.

Trevor Woerner (10):
  tests: add the standalone test-suite skeleton
  tests: add a session banner via conftest.py
  tests: add the run-tests.sh wrapper
  tests: add optional coverage reporting to run-tests.sh
  tests: add ruff linting to run-tests.sh
  tests/docs: add the suite overview README
  tests/docs: add the test-authoring guide
  tests: ignore E402 in the test tree for the sys.path bootstrap
  tests/unit/test_bb_utils: test mkdirhier() and fix its missing errno
    import
  tests/docs: add the review rubric

 .gitignore                  |  10 +++
 README.md                   |  15 ++++
 pyproject.toml              |  30 +++++++
 src/wic/bb/utils.py         |   1 +
 tests/conftest.py           |  38 +++++++++
 tests/docs/README.md        |  83 ++++++++++++++++++
 tests/docs/authoring.md     | 124 +++++++++++++++++++++++++++
 tests/docs/linting.md       |  59 +++++++++++++
 tests/docs/reviewing.md     | 164 ++++++++++++++++++++++++++++++++++++
 tests/run-tests.sh          | 159 ++++++++++++++++++++++++++++++++++
 tests/unit/.gitkeep         |   0
 tests/unit/test_bb_utils.py | 116 +++++++++++++++++++++++++
 12 files changed, 799 insertions(+)
 create mode 100644 tests/conftest.py
 create mode 100644 tests/docs/README.md
 create mode 100644 tests/docs/authoring.md
 create mode 100644 tests/docs/linting.md
 create mode 100644 tests/docs/reviewing.md
 create mode 100755 tests/run-tests.sh
 create mode 100644 tests/unit/.gitkeep
 create mode 100644 tests/unit/test_bb_utils.py

-- 
2.50.0.173.g8b6f19ccfc3a


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

* [wic][PATCH v3 01/10] tests: add the standalone test-suite skeleton
  2026-07-01  7:40 [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test Trevor Woerner
@ 2026-07-01  7:40 ` Trevor Woerner
  2026-07-06  8:18   ` [yocto-patches] " Paul Barker
  2026-07-01  7:40 ` [wic][PATCH v3 02/10] tests: add a session banner via conftest.py Trevor Woerner
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Trevor Woerner @ 2026-07-01  7:40 UTC (permalink / raw)
  To: yocto-patches

wic currently has no test mechanism of its own; it relies on the
oe-selftest from oe-core for all its testing, which means a full
bitbake build is needed to exercise even pure-Python logic. This
commit lays the groundwork for a small standalone suite that runs
from a plain checkout with nothing but pytest, so that logic can be
pinned down and kept stable as the code evolves.

This commit adds the skeleton only; it does not add any tests.

What this adds:

  - pyproject.toml: a "tests" optional-dependency group (pytest only)
    so that "pip install -e .[tests]" pulls in what the suite needs,
    plus a [tool.pytest.ini_options] section. The pytest options keep
    a test's scratch directory only when that test fails, so repeated
    runs do not accumulate leftover files.

  - tests/unit/ and tests/docs/: the directories that hold the
    in-process unit suites and the suite documentation. Git cannot
    track an empty directory, so a placeholder .gitkeep is committed
    in each to create it. The .gitkeep files have no content and are
    removed once real files land in those directories.

  - .gitignore: ignore the .pytest_cache/ directory that pytest
    writes at the top of the checkout.

  - README.md: a Testing section pointing at the suite and at
    tests/docs/, and a tests/* entry in the project layout list.

AI-Generated: codex/claude-opus 4.7 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
changes in v3:
- no change in this revision.
changes in v2:
- v1 submitted the entire test suite as a single commit; v2 breaks
  the work into a reviewable series, and this patch is one step of it.
---
 .gitignore          |  3 +++
 README.md           | 14 ++++++++++++++
 pyproject.toml      | 12 ++++++++++++
 tests/docs/.gitkeep |  0
 tests/unit/.gitkeep |  0
 5 files changed, 29 insertions(+)
 create mode 100644 tests/docs/.gitkeep
 create mode 100644 tests/unit/.gitkeep

diff --git a/.gitignore b/.gitignore
index eeb8a6ec4087..07992096c0fb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1 +1,4 @@
 **/__pycache__
+
+# pytest cache
+/.pytest_cache/
diff --git a/README.md b/README.md
index 75229421763c..497ebb1de0f0 100644
--- a/README.md
+++ b/README.md
@@ -46,6 +46,20 @@ environment file or folder (generated via `bitbake -c rootfs_wicenv
 - `src/wic/*`: core engine, plugins, and helpers.
 - `src/bb/*`: various bitbake helpers that were brought along and used in other parts of wic.
 - `src/oe/*`: various oe-core helpers that were brought along and used in other parts of wic.
+- `tests/*`: the standalone test suite and its documentation (see Testing below).
+
+## Testing
+
+wic ships a standalone test suite under `tests/` that runs from a plain
+checkout, with no bitbake and no OpenEmbedded build required. The test
+extras pull in everything the suite needs:
+
+```
+pip install -e ".[tests]"
+```
+
+See [tests/docs/](tests/docs/) for how to run the suite and the
+conventions it follows.
 
 ## Contributing
 
diff --git a/pyproject.toml b/pyproject.toml
index fdc1ce0f5ece..d660e39007c4 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -21,6 +21,11 @@ classifiers = [
 Homepage = "https://git.yoctoproject.org/wic"
 Repository = "https://git.yoctoproject.org/wic"
 
+[project.optional-dependencies]
+tests = [
+    "pytest >= 7.0",
+]
+
 [project.scripts]
 wic = "wic.cli:main"
 
@@ -36,3 +41,10 @@ build-backend = "hatchling.build"
 
 [tool.hatch.version]
 path = "src/wic/cli.py"
+
+[tool.pytest.ini_options]
+# Keep a test's scratch directory only when it fails; a passing test's
+# tmp_path is removed automatically so repeated runs do not accumulate
+# leftover files under the pytest base temp directory.
+tmp_path_retention_policy = "failed"
+tmp_path_retention_count  = 1
diff --git a/tests/docs/.gitkeep b/tests/docs/.gitkeep
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tests/unit/.gitkeep b/tests/unit/.gitkeep
new file mode 100644
index 000000000000..e69de29bb2d1
-- 
2.50.0.173.g8b6f19ccfc3a



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

* [wic][PATCH v3 02/10] tests: add a session banner via conftest.py
  2026-07-01  7:40 [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test Trevor Woerner
  2026-07-01  7:40 ` [wic][PATCH v3 01/10] tests: add the standalone test-suite skeleton Trevor Woerner
@ 2026-07-01  7:40 ` Trevor Woerner
  2026-07-06  8:17   ` [yocto-patches] " Paul Barker
  2026-07-01  7:40 ` [wic][PATCH v3 03/10] tests: add the run-tests.sh wrapper Trevor Woerner
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Trevor Woerner @ 2026-07-01  7:40 UTC (permalink / raw)
  To: yocto-patches

When a test run starts it is useful to see, at a glance, exactly what
is being tested: which host the run is on, which Python and pytest are
in use, and -- most importantly -- which wic the suite imported and
what version it reports. Without that, a passing or failing run is
hard to attribute, especially when more than one wic checkout or
virtualenv is in play.

pytest looks for a file named conftest.py and, among other things,
calls its pytest_report_header() hook to print extra lines in the
session header before collection begins. This commit adds that file
with a single hook that prints a short banner:

  - the host node name, operating system, and machine type;
  - the running Python version and the pytest version;
  - the version wic reports and the filesystem path the wic module was
    imported from.

The wic lookup is defensive: if wic cannot be imported (for example
the test extras were not installed, or the suite is being run outside
the checkout) the banner says so rather than aborting the run, so the
failure is visible in the header instead of as an opaque collection
error.

The file name conftest.py is mandatory; pytest discovers it by name,
so it cannot be called anything else.

AI-Generated: codex/claude-opus 4.7 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
changes in v3:
- no change in this revision.
changes in v2:
- v1 submitted the entire test suite as a single commit; v2 breaks
  the work into a reviewable series, and this patch is one step of it.
---
 tests/conftest.py | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 tests/conftest.py

diff --git a/tests/conftest.py b/tests/conftest.py
new file mode 100644
index 000000000000..1c368ebf8595
--- /dev/null
+++ b/tests/conftest.py
@@ -0,0 +1,38 @@
+# Session banner for the wic test suite.
+#
+# Before collection, print the environment the tests exercise:
+#   - the host
+#   - the Python and pytest versions
+#   - the wic under test (its version and the module path of its import)
+
+import platform
+
+import pytest
+
+
+def _wic_under_test():
+    """Return (version, module_path) for the wic being tested."""
+    try:
+        import wic.cli as wic_cli
+    except Exception as exc:  # pragma: no cover - reported in the banner
+        return ("(import failed: %s)" % exc, "(unimported)")
+    version = getattr(wic_cli, "__version__", "(unknown)")
+    module_path = getattr(wic_cli, "__file__", "(unknown)")
+    return (version, module_path)
+
+
+def _format_banner():
+    wic_version, wic_path = _wic_under_test()
+    lines = [
+        "wic test suite",
+        "  host:   %s  %s %s" % (
+            platform.node(), platform.system(), platform.machine()),
+        "  python: %s   pytest: %s" % (
+            platform.python_version(), pytest.__version__),
+        "  wic:    %s  (%s)" % (wic_version, wic_path),
+    ]
+    return "\n".join(lines)
+
+
+def pytest_report_header(config):
+    return _format_banner()
-- 
2.50.0.173.g8b6f19ccfc3a



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

* [wic][PATCH v3 03/10] tests: add the run-tests.sh wrapper
  2026-07-01  7:40 [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test Trevor Woerner
  2026-07-01  7:40 ` [wic][PATCH v3 01/10] tests: add the standalone test-suite skeleton Trevor Woerner
  2026-07-01  7:40 ` [wic][PATCH v3 02/10] tests: add a session banner via conftest.py Trevor Woerner
@ 2026-07-01  7:40 ` Trevor Woerner
  2026-07-06  8:21   ` [yocto-patches] " Paul Barker
  2026-07-01  7:40 ` [wic][PATCH v3 04/10] tests: add optional coverage reporting to run-tests.sh Trevor Woerner
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Trevor Woerner @ 2026-07-01  7:40 UTC (permalink / raw)
  To: yocto-patches

The suite is run with pytest, but invoking pytest directly has two
sharp edges: it must be run from the right directory for the pyproject
configuration to apply, and it happily runs against whatever Python
happens to be first on PATH, even one that cannot import wic. A run
against the wrong interpreter produces a misleading banner and, worse,
can silently exercise a different wic than the one in the checkout.

This commit adds tests/run-tests.sh, a thin wrapper that removes both
hazards:

  - it locates the repository root from its own path and changes there
    before running, so it works regardless of the caller's directory;

  - it checks that the interpreter can import wic before handing off to
    pytest, and fails loudly with the install command if it cannot;

  - with no arguments it runs the whole suite under tests/; any
    argument it does not recognise (a path, -k EXPR, -v, ...) is passed
    straight through to pytest, and an explicit -- forwards everything
    after it.

The interpreter can be overridden with the PYTHON environment variable
(default python3). -h/--help prints usage and exits.

The README Testing section gains the run-tests.sh invocation alongside
the install step.

AI-Generated: codex/claude-opus 4.7 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
changes in v3:
- no change in this revision.
changes in v2:
- v1 submitted the entire test suite as a single commit; v2 breaks
  the work into a reviewable series, and this patch is one step of it.
---
 README.md          |  1 +
 tests/run-tests.sh | 75 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)
 create mode 100755 tests/run-tests.sh

diff --git a/README.md b/README.md
index 497ebb1de0f0..e557eb435316 100644
--- a/README.md
+++ b/README.md
@@ -56,6 +56,7 @@ extras pull in everything the suite needs:
 
 ```
 pip install -e ".[tests]"
+tests/run-tests.sh
 ```
 
 See [tests/docs/](tests/docs/) for how to run the suite and the
diff --git a/tests/run-tests.sh b/tests/run-tests.sh
new file mode 100755
index 000000000000..9db6d50338d6
--- /dev/null
+++ b/tests/run-tests.sh
@@ -0,0 +1,75 @@
+#!/usr/bin/env bash
+#
+# Run the wic test suite.
+#
+# Thin wrapper around pytest that works from anywhere in the checkout:
+# it locates the repository root, makes sure the interpreter can import
+# wic, and then hands off to pytest.
+#
+# Needs the test extras: pip install -e ".[tests]"
+
+set -euo pipefail
+
+usage() {
+    cat <<'USAGE'
+Usage:
+  tests/run-tests.sh [pytest args]
+
+Options:
+  -h, --help    show this help and exit
+
+Anything else is passed straight through to pytest (for example a
+path, -k EXPR, or -v). With no such argument the whole suite under
+tests/ is run.
+
+Examples:
+  tests/run-tests.sh                  # whole suite
+  tests/run-tests.sh -k filemap -v    # pass args through to pytest
+  tests/run-tests.sh tests/unit       # a single tier or file
+
+Needs the test extras: pip install -e ".[tests]"
+USAGE
+}
+
+REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+PY="${PYTHON:-python3}"
+
+pytest_args=()
+while [ $# -gt 0 ]; do
+    case "$1" in
+        -h|--help)
+            usage
+            exit 0
+            ;;
+        --)
+            shift
+            pytest_args+=("$@")
+            break
+            ;;
+        *)
+            pytest_args+=("$1")
+            ;;
+    esac
+    shift
+done
+
+cd "$REPO_ROOT"
+
+# Make sure the interpreter that will run pytest can actually import wic.
+# The suites pass even without an install (each adds src/ to sys.path),
+# but the session banner and any install-dependent behaviour would be
+# wrong, so fail loudly instead of running against the wrong interpreter.
+if ! "$PY" -c "import wic" >/dev/null 2>&1; then
+    echo "error: '$PY' cannot import wic." >&2
+    echo "       install wic with its test extras:" >&2
+    echo "       $PY -m pip install -e \".[tests]\"" >&2
+    exit 1
+fi
+
+# Default target: the whole suite. Overridden if the caller passed a
+# path or -k/-m selector to pytest.
+if [ ${#pytest_args[@]} -eq 0 ]; then
+    pytest_args=("tests")
+fi
+
+exec "$PY" -m pytest "${pytest_args[@]}"
-- 
2.50.0.173.g8b6f19ccfc3a



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

* [wic][PATCH v3 04/10] tests: add optional coverage reporting to run-tests.sh
  2026-07-01  7:40 [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test Trevor Woerner
                   ` (2 preceding siblings ...)
  2026-07-01  7:40 ` [wic][PATCH v3 03/10] tests: add the run-tests.sh wrapper Trevor Woerner
@ 2026-07-01  7:40 ` Trevor Woerner
  2026-07-06  8:33   ` [yocto-patches] " Paul Barker
  2026-07-01  7:40 ` [wic][PATCH v3 05/10] tests: add ruff linting " Trevor Woerner
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Trevor Woerner @ 2026-07-01  7:40 UTC (permalink / raw)
  To: yocto-patches

Knowing which lines a test run actually exercised is the difference
between "the suite is green" and "the suite is green and we know what
it touched". This commit wires branch coverage of the wic source into
the runner, kept entirely opt-in so a plain run stays fast and quiet.

pyproject.toml gains coverage and pytest-cov in the tests extra, so
"pip install -e .[tests]" pulls in what the new flags need.

run-tests.sh gains two options:

  - --coverage measures branch coverage of src/wic during the run and
    prints a terminal report listing the lines that were missed;

  - --html [DIR] additionally writes a browsable HTML report (default
    htmlcov/, or DIR if given) and implies --coverage.

If --coverage is requested but pytest-cov is not installed the runner
fails loudly with the install command rather than running without the
measurement it was asked for.

The coverage data file and the HTML report directory are build
artifacts, so .gitignore learns to ignore .coverage and htmlcov/.

AI-Generated: codex/claude-opus 4.7 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
changes in v3:
- no change in this revision.
changes in v2:
- v1 submitted the entire test suite as a single commit; v2 breaks
  the work into a reviewable series, and this patch is one step of it.
---
 .gitignore         |  4 ++++
 pyproject.toml     |  2 ++
 tests/run-tests.sh | 39 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 07992096c0fb..534c49538091 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,3 +2,7 @@
 
 # pytest cache
 /.pytest_cache/
+
+# coverage data and reports
+/.coverage
+/htmlcov/
diff --git a/pyproject.toml b/pyproject.toml
index d660e39007c4..ece2757bb686 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -24,6 +24,8 @@ Repository = "https://git.yoctoproject.org/wic"
 [project.optional-dependencies]
 tests = [
     "pytest >= 7.0",
+    "coverage >= 7.0",
+    "pytest-cov >= 4.0",
 ]
 
 [project.scripts]
diff --git a/tests/run-tests.sh b/tests/run-tests.sh
index 9db6d50338d6..a483da6a63a6 100755
--- a/tests/run-tests.sh
+++ b/tests/run-tests.sh
@@ -13,9 +13,13 @@ set -euo pipefail
 usage() {
     cat <<'USAGE'
 Usage:
-  tests/run-tests.sh [pytest args]
+  tests/run-tests.sh [--coverage] [--html [DIR]] [pytest args]
 
 Options:
+  --coverage    also measure branch coverage of src/wic and print a
+                terminal report listing the lines that were missed
+  --html [DIR]  also write an HTML coverage report (default dir:
+                htmlcov/); implies --coverage
   -h, --help    show this help and exit
 
 Anything else is passed straight through to pytest (for example a
@@ -24,6 +28,9 @@ tests/ is run.
 
 Examples:
   tests/run-tests.sh                  # whole suite
+  tests/run-tests.sh --coverage       # + terminal coverage report
+  tests/run-tests.sh --html           # + HTML report in htmlcov/
+  tests/run-tests.sh --html /tmp/cov  # + HTML report in /tmp/cov
   tests/run-tests.sh -k filemap -v    # pass args through to pytest
   tests/run-tests.sh tests/unit       # a single tier or file
 
@@ -34,9 +41,25 @@ USAGE
 REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
 PY="${PYTHON:-python3}"
 
+coverage=0
+html=0
+html_dir="htmlcov"
 pytest_args=()
 while [ $# -gt 0 ]; do
     case "$1" in
+        --coverage)
+            coverage=1
+            ;;
+        --html)
+            coverage=1
+            html=1
+            # Optional directory argument: consume the next token only if
+            # it is not another option or the pytest pass-through marker.
+            case "${2:-}" in
+                ""|-*|--) ;;
+                *) html_dir="$2"; shift ;;
+            esac
+            ;;
         -h|--help)
             usage
             exit 0
@@ -72,4 +95,18 @@ if [ ${#pytest_args[@]} -eq 0 ]; then
     pytest_args=("tests")
 fi
 
+if [ "$coverage" -eq 1 ]; then
+    if ! "$PY" -c "import pytest_cov" >/dev/null 2>&1; then
+        echo "error: coverage requested but pytest-cov is not installed." >&2
+        echo "       run: $PY -m pip install -e \".[tests]\"" >&2
+        exit 1
+    fi
+    cov_args=(--cov=wic --cov-branch --cov-report=term-missing)
+    if [ "$html" -eq 1 ]; then
+        cov_args+=(--cov-report="html:${html_dir}")
+        echo "HTML coverage report: ${html_dir}/index.html" >&2
+    fi
+    exec "$PY" -m pytest "${pytest_args[@]}" "${cov_args[@]}"
+fi
+
 exec "$PY" -m pytest "${pytest_args[@]}"
-- 
2.50.0.173.g8b6f19ccfc3a



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

* [wic][PATCH v3 05/10] tests: add ruff linting to run-tests.sh
  2026-07-01  7:40 [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test Trevor Woerner
                   ` (3 preceding siblings ...)
  2026-07-01  7:40 ` [wic][PATCH v3 04/10] tests: add optional coverage reporting to run-tests.sh Trevor Woerner
@ 2026-07-01  7:40 ` Trevor Woerner
  2026-07-06  8:40   ` [yocto-patches] " Paul Barker
  2026-07-01  7:40 ` [wic][PATCH v3 06/10] tests/docs: add the suite overview README Trevor Woerner
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Trevor Woerner @ 2026-07-01  7:40 UTC (permalink / raw)
  To: yocto-patches

A test suite is only trustworthy if its own code is clean, so this
commit brings ruff into the runner and holds the test tree to a clean
bar. It also makes it easy to preview what ruff thinks of the wic
source, without yet enforcing it.

pyproject.toml gains ruff in the tests extra and a [tool.ruff] section.
The configuration is deliberately minimal for now: the test suite is
the only tree under an enforced clean bar; the wic source under src/ is
not yet ruff-clean and is reported, not gated.

run-tests.sh gains two lint modes, each used on its own:

  - --lint-tests runs ruff over tests/ and exits. The test suite must
    report nothing; a finding here is a bug in our own test code and is
    expected to be fixed.

  - --lint-src runs ruff over src/ and exits. The source is not yet
    ruff-clean, so this is a preview: the runner prints ruff's findings
    and exits with its status, but nothing in the suite asserts on
    them. Keeping the two trees on separate flags means cleaning up the
    source later does not disturb the test-tree gate.

A lint mode cannot be combined with coverage, with the other lint mode,
or with pytest arguments; the runner rejects such combinations loudly
rather than silently dropping the extras. If ruff is not installed it
fails with the install command.

tests/docs/linting.md documents the two modes and why src/ is held back
for now. That file replaces the tests/docs/.gitkeep placeholder, which
is no longer needed now that the directory has real content.

.gitignore learns to ignore ruff's .ruff_cache/ directory.

AI-Generated: codex/claude-opus 4.7 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
changes in v3:
- no change in this revision.
changes in v2:
- v1 submitted the entire test suite as a single commit; v2 breaks
  the work into a reviewable series, and this patch is one step of it.
---
 .gitignore            |  3 +++
 pyproject.toml        |  8 ++++++++
 tests/docs/.gitkeep   |  0
 tests/docs/linting.md | 39 +++++++++++++++++++++++++++++++++++
 tests/run-tests.sh    | 47 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 97 insertions(+)
 delete mode 100644 tests/docs/.gitkeep
 create mode 100644 tests/docs/linting.md

diff --git a/.gitignore b/.gitignore
index 534c49538091..3c3cfb328fb0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,3 +6,6 @@
 # coverage data and reports
 /.coverage
 /htmlcov/
+
+# ruff cache
+/.ruff_cache/
diff --git a/pyproject.toml b/pyproject.toml
index ece2757bb686..656adcd4930a 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -26,6 +26,7 @@ tests = [
     "pytest >= 7.0",
     "coverage >= 7.0",
     "pytest-cov >= 4.0",
+    "ruff >= 0.5",
 ]
 
 [project.scripts]
@@ -50,3 +51,10 @@ path = "src/wic/cli.py"
 # leftover files under the pytest base temp directory.
 tmp_path_retention_policy = "failed"
 tmp_path_retention_count  = 1
+
+[tool.ruff]
+# For now only the test suite is actually linted (run-tests.sh
+# --lint-tests passes the tests/ path); the wic source under src/ is
+# not yet ruff-clean and is left out until its findings are fixed (see
+# tests/docs/linting.md). --lint-src can still be run to preview the
+# source findings, but it is reported, not enforced.
diff --git a/tests/docs/.gitkeep b/tests/docs/.gitkeep
deleted file mode 100644
index e69de29bb2d1..000000000000
diff --git a/tests/docs/linting.md b/tests/docs/linting.md
new file mode 100644
index 000000000000..71b4de21c100
--- /dev/null
+++ b/tests/docs/linting.md
@@ -0,0 +1,39 @@
+# Linting
+
+## Contents
+
+- [Running the linter](#running-the-linter)
+- [tests/ must be clean](#tests-must-be-clean)
+- [src/ is not linted yet](#src-is-not-linted-yet)
+
+The test suite is linted with [ruff](https://docs.astral.sh/ruff/). It
+is configured in `pyproject.toml` (`[tool.ruff]`).
+
+## Running the linter
+
+The runner exposes ruff through two separate modes, each used on its
+own:
+
+```bash
+tests/run-tests.sh --lint-tests   # ruff over tests/
+tests/run-tests.sh --lint-src     # ruff over src/ (preview only)
+```
+
+A lint mode cannot be combined with coverage, with the other lint
+mode, or with pytest arguments; the runner rejects such combinations.
+
+## tests/ must be clean
+
+Our own test code is held to a clean bar: `tests/run-tests.sh
+--lint-tests` reports nothing. If you add a test that trips a rule, fix
+the test before the change lands.
+
+## src/ is not linted yet
+
+`--lint-src` runs ruff over the wic source, but the source is **not**
+yet ruff-clean, so its findings are a preview report rather than a
+gate: the runner prints them and exits with ruff's status, but nothing
+in the suite asserts on them. Treating `src/` findings as a hard
+failure now would block every run on fixes that have not landed. Once
+the source is cleaned up, `src/` can be promoted to the same clean bar
+as `tests/`.
diff --git a/tests/run-tests.sh b/tests/run-tests.sh
index a483da6a63a6..085dcc93f91d 100755
--- a/tests/run-tests.sh
+++ b/tests/run-tests.sh
@@ -14,23 +14,34 @@ usage() {
     cat <<'USAGE'
 Usage:
   tests/run-tests.sh [--coverage] [--html [DIR]] [pytest args]
+  tests/run-tests.sh --lint-tests
+  tests/run-tests.sh --lint-src
 
 Options:
   --coverage    also measure branch coverage of src/wic and print a
                 terminal report listing the lines that were missed
   --html [DIR]  also write an HTML coverage report (default dir:
                 htmlcov/); implies --coverage
+  --lint-tests  run ruff over tests/ and exit; the test suite is held
+                to a clean bar, so this must report nothing
+  --lint-src    run ruff over src/ and exit; src/ is not yet ruff-clean,
+                so this is a preview report and is not enforced
   -h, --help    show this help and exit
 
 Anything else is passed straight through to pytest (for example a
 path, -k EXPR, or -v). With no such argument the whole suite under
 tests/ is run.
 
+The two lint modes each run on their own; they cannot be combined with
+coverage, with each other, or with pytest arguments.
+
 Examples:
   tests/run-tests.sh                  # whole suite
   tests/run-tests.sh --coverage       # + terminal coverage report
   tests/run-tests.sh --html           # + HTML report in htmlcov/
   tests/run-tests.sh --html /tmp/cov  # + HTML report in /tmp/cov
+  tests/run-tests.sh --lint-tests     # ruff over tests/
+  tests/run-tests.sh --lint-src       # ruff over src/ (preview)
   tests/run-tests.sh -k filemap -v    # pass args through to pytest
   tests/run-tests.sh tests/unit       # a single tier or file
 
@@ -44,6 +55,8 @@ PY="${PYTHON:-python3}"
 coverage=0
 html=0
 html_dir="htmlcov"
+lint_tests=0
+lint_src=0
 pytest_args=()
 while [ $# -gt 0 ]; do
     case "$1" in
@@ -60,6 +73,12 @@ while [ $# -gt 0 ]; do
                 *) html_dir="$2"; shift ;;
             esac
             ;;
+        --lint-tests)
+            lint_tests=1
+            ;;
+        --lint-src)
+            lint_src=1
+            ;;
         -h|--help)
             usage
             exit 0
@@ -78,6 +97,34 @@ done
 
 cd "$REPO_ROOT"
 
+# The lint modes run ruff and exit, so each must be used on its own.
+# Reject combining them with coverage, with each other, or with pytest
+# arguments loudly instead of silently ignoring the extras.
+if [ $((lint_tests + lint_src)) -gt 0 ]; then
+    if [ "$lint_tests" -eq 1 ] && [ "$lint_src" -eq 1 ]; then
+        echo "error: --lint-tests and --lint-src cannot be combined." >&2
+        echo "       run one lint mode at a time." >&2
+        exit 2
+    fi
+    if [ "$coverage" -eq 1 ] || [ "$html" -eq 1 ] || [ ${#pytest_args[@]} -gt 0 ]; then
+        echo "error: a lint mode must be used on its own." >&2
+        echo "       lint, or drop the lint flag to run the suite." >&2
+        exit 2
+    fi
+    if ! "$PY" -c "import ruff" >/dev/null 2>&1 && ! command -v ruff >/dev/null 2>&1; then
+        echo "error: lint requested but ruff is not installed." >&2
+        echo "       run: $PY -m pip install -e \".[tests]\"" >&2
+        exit 1
+    fi
+    if [ "$lint_tests" -eq 1 ]; then
+        # The test suite is held to a clean bar; a finding here is a bug
+        # in our own test code and must be fixed.
+        exec "$PY" -m ruff check tests
+    fi
+    # src/ is not yet ruff-clean; this is a preview report, not a gate.
+    exec "$PY" -m ruff check src
+fi
+
 # Make sure the interpreter that will run pytest can actually import wic.
 # The suites pass even without an install (each adds src/ to sys.path),
 # but the session banner and any install-dependent behaviour would be
-- 
2.50.0.173.g8b6f19ccfc3a



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

* [wic][PATCH v3 06/10] tests/docs: add the suite overview README
  2026-07-01  7:40 [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test Trevor Woerner
                   ` (4 preceding siblings ...)
  2026-07-01  7:40 ` [wic][PATCH v3 05/10] tests: add ruff linting " Trevor Woerner
@ 2026-07-01  7:40 ` Trevor Woerner
  2026-07-06  8:47   ` [yocto-patches] " Paul Barker
  2026-07-01  7:40 ` [wic][PATCH v3 07/10] tests/docs: add the test-authoring guide Trevor Woerner
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Trevor Woerner @ 2026-07-01  7:40 UTC (permalink / raw)
  To: yocto-patches

The test suite needs a front door: a short document that says what the
suite is, how it is laid out, and how to run it. This commit adds
tests/docs/README.md to be that overview.

The README covers:

  - the layout of tests/, file by file;
  - that the suite is unit-only and needs no host tools or build
    environment, so it runs from a plain checkout;
  - how to install the test extras and invoke run-tests.sh, pointing at
    run-tests.sh --help as the authoritative, always-current list of
    options rather than duplicating them here where they would drift;
  - that anything run-tests.sh does not recognise is passed through to
    pytest;
  - a pointer to linting.md for how ruff is applied;
  - a small table of the other documents under tests/docs/.

It deliberately does not enumerate every run-tests.sh option, since the
runner is expected to grow more of them; the table and the --help
output stay the source of truth.

AI-Generated: codex/claude-opus 4.7 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
changes in v3:
- document the standard pytest scratch-directory mechanisms
  (TMPDIR and --basetemp) in the suite README, so there is no need
  for a custom temporary-directory variable.
changes in v2:
- v1 submitted the entire test suite as a single commit; v2 breaks
  the work into a reviewable series, and this patch is one step of it.
---
 tests/docs/README.md | 82 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 tests/docs/README.md

diff --git a/tests/docs/README.md b/tests/docs/README.md
new file mode 100644
index 000000000000..ba7bd5e192a3
--- /dev/null
+++ b/tests/docs/README.md
@@ -0,0 +1,82 @@
+# wic test suite
+
+A standalone test suite for the wic source tree. It runs from a plain
+checkout with nothing but `pytest` -- no bitbake, no OpenEmbedded
+build, and no target image -- so wic's logic can be exercised and kept
+stable as the code changes.
+
+## Contents
+
+- [Layout](#layout)
+- [Running](#running)
+- [Linting](#linting)
+- [Documentation](#documentation)
+
+## Layout
+
+```
+tests/
+  conftest.py        session banner describing the run
+  run-tests.sh       wrapper for running the suite
+  unit/              unit tests that import wic modules directly
+  docs/              this documentation
+```
+
+The suite is unit-only: every test under `tests/unit/` imports a wic
+module in-process and asserts on its behaviour, so none of it needs
+host tools or a build environment.
+
+## Running
+
+Install wic with its test extras, then run the suite:
+
+```bash
+pip install -e ".[tests]"
+tests/run-tests.sh
+```
+
+`run-tests.sh` works from anywhere in the checkout. It can also report
+branch coverage of the wic source; run `tests/run-tests.sh --help` for
+the current list of options.
+
+Anything `run-tests.sh` does not recognise is handed straight to
+pytest, so the whole pytest command line is available:
+
+```bash
+tests/run-tests.sh -k filemap -v          # one area, verbose
+tests/run-tests.sh tests/unit             # a single tier or file
+```
+
+## Scratch files
+
+Tests that need scratch space use pytest's `tmp_path` fixture, so there
+is no wic-specific temporary-directory setting. Scratch directories are
+created under pytest's base temporary directory and cleaned up according
+to the retention policy in `pyproject.toml` (a passing test's directory
+is removed, a failing one is kept).
+
+By default pytest roots that base directory at the system temporary
+directory (usually `/tmp`). If that fills up, or you want the scratch
+files somewhere else, use the standard pytest mechanisms rather than a
+custom variable. Both are passed straight through by `run-tests.sh`:
+
+```bash
+TMPDIR=/path/to/scratch tests/run-tests.sh   # honoured by pytest
+tests/run-tests.sh --basetemp=/path/to/scratch
+```
+
+`--basetemp` puts everything under the given directory and clears it at
+the start of each run; `TMPDIR` keeps the last few sessions there.
+
+## Linting
+
+The test suite is linted with ruff and is held to a clean bar. See
+[linting.md](linting.md) for the lint modes and how the source tree is
+treated.
+
+## Documentation
+
+| File | Content |
+|------|---------|
+| [authoring.md](authoring.md) | How to add a unit test to the suite |
+| [linting.md](linting.md)     | How ruff is used on the suite |
-- 
2.50.0.173.g8b6f19ccfc3a



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

* [wic][PATCH v3 07/10] tests/docs: add the test-authoring guide
  2026-07-01  7:40 [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test Trevor Woerner
                   ` (5 preceding siblings ...)
  2026-07-01  7:40 ` [wic][PATCH v3 06/10] tests/docs: add the suite overview README Trevor Woerner
@ 2026-07-01  7:40 ` Trevor Woerner
  2026-07-06  8:52   ` [yocto-patches] " Paul Barker
  2026-07-01  7:40 ` [wic][PATCH v3 08/10] tests: ignore E402 in the test tree for the sys.path bootstrap Trevor Woerner
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Trevor Woerner @ 2026-07-01  7:40 UTC (permalink / raw)
  To: yocto-patches

A suite is only as good as the tests added to it over time, so it needs
a guide that says where a new test goes, what one looks like, and how
to choose its inputs and assertions. This commit adds
tests/docs/authoring.md.

The guide covers:

  - where a test goes: one test_<area>.py per wic module under
    tests/unit/, extending an existing file or starting a new one;

  - the shape of a test: a small worked example, a preference for
    parametrize over in-test loops so each input/output pair is its own
    named case, and use of the tmp_path fixture for scratch files;

  - asserting the correct behaviour rather than whatever wic currently
    does, so a test never bakes in a bug; when a test exposes a defect,
    the source fix lands in the same change so the test passes;

  - probing the boundaries: a checklist of input classes worth covering
    for numbers, strings, paths, types/shape, and state, each asserting
    a specific value or exception rather than merely "it did not
    crash";

  - keeping the assertion strong: never weaken an assertion to get
    green;

  - how to run just the test you wrote, with and without coverage.

AI-Generated: codex/claude-opus 4.7 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
changes in v3:
- no change in this revision.
changes in v2:
- v1 submitted the entire test suite as a single commit; v2 breaks
  the work into a reviewable series, and this patch is one step of it.
---
 tests/docs/authoring.md | 124 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)
 create mode 100644 tests/docs/authoring.md

diff --git a/tests/docs/authoring.md b/tests/docs/authoring.md
new file mode 100644
index 000000000000..c60cceb2d32d
--- /dev/null
+++ b/tests/docs/authoring.md
@@ -0,0 +1,124 @@
+# Authoring a unit test
+
+Unit tests live under `tests/unit/`, one module per area of wic, named
+`test_<area>.py`. They import the wic module under test directly and
+run in-process, so they need no host tools and no build environment.
+
+## Contents
+
+- [Where a test goes](#where-a-test-goes)
+- [Shape of a test](#shape-of-a-test)
+- [Assert the correct behaviour](#assert-the-correct-behaviour)
+- [Probe the boundaries](#probe-the-boundaries)
+- [Keep the assertion strong](#keep-the-assertion-strong)
+- [Running what you wrote](#running-what-you-wrote)
+
+## Where a test goes
+
+Group tests by the wic module they exercise, one `test_<area>.py` per
+module. Start a new file when you begin covering a module that has no
+file yet; otherwise extend the existing one.
+
+## Shape of a test
+
+```python
+import pytest
+
+from wic.ksparser import sizetype
+
+
+class TestSizetype:
+    # sizetype(default, size_in_bytes=False) returns a parser f(arg);
+    # with default "M", a bare number is read as mebibytes-in-KiB.
+    def test_plain_value_uses_default_unit(self):
+        parse = sizetype("M")
+        assert parse("100") == 100 * 1024
+
+    @pytest.mark.parametrize("arg,expected", [
+        ("1K", 1),
+        ("1G", 1 * 1024 * 1024),
+        ("0", 0),
+    ])
+    def test_suffixes(self, arg, expected):
+        assert sizetype("M")(arg) == expected
+```
+
+Prefer `@pytest.mark.parametrize` to express each input/output pair as
+its own case rather than looping inside one test, so a single bad
+value shows up as one named failure.
+
+Use `pytest`'s `tmp_path` fixture for any scratch files; a passing
+test's scratch directory is removed automatically (see the retention
+policy in `pyproject.toml`), and a failing one is kept for inspection.
+
+## Assert the correct behaviour
+
+Every assertion states the behaviour wic is *expected* to provide,
+never the behaviour it currently happens to have. A test that locks in
+a wrong result is worse than no test: it gives false confidence and it
+turns red exactly when someone repairs the code.
+
+When a test exposes a defect, the fix to the wic source lands in the
+same change as the test, so the test asserts the correct behaviour and
+passes. Do not commit a test that asserts a known-wrong result.
+
+Wrong -- bakes in the bug:
+
+```python
+# value lost its comma; asserting the broken output
+assert result["file"] == "s3://bucket/a"
+```
+
+Right -- asserts the correct output (and the fix lands with it):
+
+```python
+assert result["file"] == "s3://bucket/a,b.img"
+```
+
+## Probe the boundaries
+
+For a parameter, probe the boundary and just past it rather than only
+the comfortable middle of its range. The classes worth covering:
+
+- numbers: empty, zero, negative, the maximum and one beyond it,
+  non-power-of-two and non-multiple-of-1024/1000 values, primes,
+  fractional values where an integer is assumed, `0` versus `0.0`
+  versus `"0"`, off-by-one at a block or sector boundary, and very
+  large magnitudes near `2**31` and `2**63`
+- strings: the empty string, whitespace only, embedded spaces, tabs,
+  newlines and CRLF endings, non-ASCII characters, shell
+  metacharacters in a value that becomes part of a command line,
+  leading-dash values that look like options, over-delimited or
+  malformed forms (stray commas, doubled or missing separators), and
+  very long values
+- paths: traversal (`../`), doubled slashes, a trailing slash or its
+  absence, `.` and `..` components, broken or looping symlinks, and a
+  non-existent path
+- types and shape: each wrong type the parameter might receive (a
+  string where an integer is expected, a list where a string is
+  expected, `None`, and so on), the wrong argument count, and
+  duplicate keys or entries
+- state: a second call that must not see stale cached data, a
+  `cache=False` path that must evict, calling an operation twice, and
+  finalising a half-constructed object
+
+Each case asserts a specific outcome -- an exact value, or a specific
+exception -- rather than merely "it did not crash." A test that only
+checks for the absence of an exception will pass against badly wrong
+output.
+
+## Keep the assertion strong
+
+If a test is hard to make pass, the answer is in the code or in
+understanding the correct behaviour, never in weakening the assertion
+to something vague enough to pass. Loosening an assertion to get green
+is how a suite quietly stops catching regressions.
+
+## Running what you wrote
+
+```bash
+tests/run-tests.sh tests/unit/test_<area>.py -v
+tests/run-tests.sh --coverage tests/unit/test_<area>.py
+```
+
+A new test should leave the suite green, with zero failures.
-- 
2.50.0.173.g8b6f19ccfc3a



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

* [wic][PATCH v3 08/10] tests: ignore E402 in the test tree for the sys.path bootstrap
  2026-07-01  7:40 [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test Trevor Woerner
                   ` (6 preceding siblings ...)
  2026-07-01  7:40 ` [wic][PATCH v3 07/10] tests/docs: add the test-authoring guide Trevor Woerner
@ 2026-07-01  7:40 ` Trevor Woerner
  2026-07-06  8:55   ` [yocto-patches] " Paul Barker
  2026-07-01  7:40 ` [wic][PATCH v3 09/10] tests/unit/test_bb_utils: test mkdirhier() and fix its missing errno import Trevor Woerner
  2026-07-01  7:40 ` [wic][PATCH v3 10/10] tests/docs: add the review rubric Trevor Woerner
  9 siblings, 1 reply; 21+ messages in thread
From: Trevor Woerner @ 2026-07-01  7:40 UTC (permalink / raw)
  To: yocto-patches

The unit tests that follow run against a plain checkout that has not
been installed. To do that, each test module prepends the in-tree src/
directory to sys.path and only then imports wic:

    _SRC = Path(__file__).resolve().parent.parent.parent / "src"
    if str(_SRC) not in sys.path:
        sys.path.insert(0, str(_SRC))

    from wic.bb.utils import mkdirhier

That bootstrap necessarily runs before the wic imports, which trips
ruff's E402 (module-level import not at top of file). The ordering is
required, not accidental: the import would fail without the sys.path
adjustment ahead of it.

Rather than scatter per-line noqa comments through every test module,
this commit relaxes E402 for the test tree once, via a
[tool.ruff.lint.per-file-ignores] entry scoped to tests/**. It is the
only rule relaxed for tests/; the suite is otherwise held to a clean
bar by --lint-tests.

tests/docs/linting.md gains a section describing this single exception
and why it exists.

AI-Generated: codex/claude-opus 4.7 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
changes in v3:
- no change in this revision.
changes in v2:
- v1 submitted the entire test suite as a single commit; v2 breaks
  the work into a reviewable series, and this patch is one step of it.
---
 pyproject.toml        |  8 ++++++++
 tests/docs/linting.md | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/pyproject.toml b/pyproject.toml
index 656adcd4930a..fb42fe6dd135 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -58,3 +58,11 @@ tmp_path_retention_count  = 1
 # not yet ruff-clean and is left out until its findings are fixed (see
 # tests/docs/linting.md). --lint-src can still be run to preview the
 # source findings, but it is reported, not enforced.
+
+[tool.ruff.lint.per-file-ignores]
+# Test modules prepend the in-tree src/ directory to sys.path before
+# importing wic, so the suite runs against a checkout that has not been
+# installed. That bootstrap necessarily runs before the wic imports,
+# which trips E402 (module-level import not at top of file). The
+# ordering is deliberate; ignore E402 for the test tree only.
+"tests/**" = ["E402"]
diff --git a/tests/docs/linting.md b/tests/docs/linting.md
index 71b4de21c100..1c1bfcc06f82 100644
--- a/tests/docs/linting.md
+++ b/tests/docs/linting.md
@@ -4,6 +4,7 @@
 
 - [Running the linter](#running-the-linter)
 - [tests/ must be clean](#tests-must-be-clean)
+- [The one intentional exception in tests/](#the-one-intentional-exception-in-tests)
 - [src/ is not linted yet](#src-is-not-linted-yet)
 
 The test suite is linted with [ruff](https://docs.astral.sh/ruff/). It
@@ -28,6 +29,25 @@ Our own test code is held to a clean bar: `tests/run-tests.sh
 --lint-tests` reports nothing. If you add a test that trips a rule, fix
 the test before the change lands.
 
+## The one intentional exception in tests/
+
+Test modules prepend the in-tree `src/` directory to `sys.path` before
+importing `wic`, so the suite runs against a plain checkout that has not
+been installed:
+
+```python
+_SRC = Path(__file__).resolve().parent.parent.parent / "src"
+if str(_SRC) not in sys.path:
+    sys.path.insert(0, str(_SRC))
+
+from wic.bb.utils import mkdirhier
+```
+
+That bootstrap necessarily runs before the `wic` imports, which trips
+`E402` (module-level import not at top of file). The ordering is
+required, so `E402` is ignored for the test tree via `per-file-ignores`
+in `pyproject.toml`. This is the only rule relaxed for `tests/`.
+
 ## src/ is not linted yet
 
 `--lint-src` runs ruff over the wic source, but the source is **not**
-- 
2.50.0.173.g8b6f19ccfc3a



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

* [wic][PATCH v3 09/10] tests/unit/test_bb_utils: test mkdirhier() and fix its missing errno import
  2026-07-01  7:40 [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test Trevor Woerner
                   ` (7 preceding siblings ...)
  2026-07-01  7:40 ` [wic][PATCH v3 08/10] tests: ignore E402 in the test tree for the sys.path bootstrap Trevor Woerner
@ 2026-07-01  7:40 ` Trevor Woerner
  2026-07-06  9:06   ` [yocto-patches] " Paul Barker
  2026-07-01  7:40 ` [wic][PATCH v3 10/10] tests/docs: add the review rubric Trevor Woerner
  9 siblings, 1 reply; 21+ messages in thread
From: Trevor Woerner @ 2026-07-01  7:40 UTC (permalink / raw)
  To: yocto-patches

mkdirhier() wraps os.makedirs() and, on OSError, re-checks the error to
decide whether to swallow a benign "already exists" race or re-raise:

    except OSError as e:
        if e.errno != errno.EEXIST or not os.path.isdir(directory):
            raise e

but the module never imports errno. The reference to errno.EEXIST is
only evaluated when os.makedirs() actually raises, so the defect is
invisible on the happy path and on the unexpanded-${} guard. The moment
a real OSError occurs -- a parent component that is a file, a permission
error, anything -- the handler itself raises NameError: name 'errno' is
not defined, masking the original error with a misleading one.

The fix is a one-line import. With errno available the handler behaves
as intended: an EEXIST on an existing directory is swallowed, while any
other OSError (and an EEXIST whose path is not a directory) propagates
unchanged.

This commit adds tests/unit/test_bb_utils.py, covering mkdirhier() end
to end:

  - the happy path (nested, single-level, idempotent, existing dir);
  - the unexpanded-${} guard (rejected, nothing created, and that a
    bare brace without a dollar is allowed);
  - the OSError handler: a real mkdir under a file component surfaces an
    OSError, an arbitrary OSError propagates with its errno intact, an
    EEXIST on an existing directory is swallowed, and an EEXIST on a
    regular file propagates.

The error-path tests fail with NameError without the import and pass
with it, so the test and the fix belong together in this one change.

AI-Generated: codex/claude-opus 4.8 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
changes in v3:
- switch the tests from tempfile.mkdtemp() to pytest's tmp_path
  fixture so each test's scratch directory is cleaned up instead of
  leaking under /tmp; no change to what is tested.
changes in v2:
- v1 recorded this bug with an xfail marker in one large commit;
  v2 drops the xfail, asserts the correct behaviour directly, and
  lands the one-line errno-import fix in this same commit so the
  test passes green.
---
 src/wic/bb/utils.py         |   1 +
 tests/unit/test_bb_utils.py | 116 ++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)
 create mode 100644 tests/unit/test_bb_utils.py

diff --git a/src/wic/bb/utils.py b/src/wic/bb/utils.py
index 3750056ba563..0b40dd0c1d59 100644
--- a/src/wic/bb/utils.py
+++ b/src/wic/bb/utils.py
@@ -1,6 +1,7 @@
 """
 Minimal subset of BitBake's bb.utils used by standalone wic.
 """
+import errno
 import os
 
 # from bitbake/lib/bb/utils.py
diff --git a/tests/unit/test_bb_utils.py b/tests/unit/test_bb_utils.py
new file mode 100644
index 000000000000..aeca054109ff
--- /dev/null
+++ b/tests/unit/test_bb_utils.py
@@ -0,0 +1,116 @@
+r"""
+Unit tests for wic.bb.utils -- a small standalone subset of BitBake's
+bb.utils. mkdirhier() is a mkdir -p wrapper with two guards: an
+unexpanded-${} check and an OSError handler that re-checks EEXIST.
+"""
+import errno
+import os
+import sys
+import unittest.mock as mock
+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))
+
+from wic.bb.utils import mkdirhier
+
+
+# ---------------------------------------------------------------------------
+# Happy path
+# ---------------------------------------------------------------------------
+
+class TestMkdirhierHappyPath:
+    def test_creates_nested_directory(self, tmp_path):
+        target = os.path.join(str(tmp_path), "a", "b", "c")
+        mkdirhier(target)
+        assert os.path.isdir(target)
+
+    def test_idempotent_on_existing_directory(self, tmp_path):
+        target = os.path.join(str(tmp_path), "x")
+        mkdirhier(target)
+        # Second call must not raise (exist_ok=True).
+        mkdirhier(target)
+        assert os.path.isdir(target)
+
+    def test_single_level(self, tmp_path):
+        target = os.path.join(str(tmp_path), "single")
+        mkdirhier(target)
+        assert os.path.isdir(target)
+
+    def test_existing_root_directory(self, tmp_path):
+        # An already-existing directory (the tmpdir itself) is a no-op.
+        mkdirhier(str(tmp_path))
+        assert os.path.isdir(str(tmp_path))
+
+
+# ---------------------------------------------------------------------------
+# Unexpanded bitbake-variable guard
+# ---------------------------------------------------------------------------
+
+class TestMkdirhierUnexpandedVar:
+    def test_unexpanded_var_rejected(self):
+        with pytest.raises(Exception) as ei:
+            mkdirhier("/tmp/${WORKDIR}/x")
+        assert "unexpanded bitbake variable" in str(ei.value)
+
+    def test_unexpanded_var_not_created(self, tmp_path):
+        bad = os.path.join(str(tmp_path), "${VAR}", "sub")
+        with pytest.raises(Exception):
+            mkdirhier(bad)
+        # Nothing should have been created.
+        assert not os.path.exists(os.path.join(str(tmp_path), "${VAR}"))
+
+    def test_brace_without_dollar_is_allowed(self, tmp_path):
+        # Only the literal '${' marker triggers the guard; a bare '{' is a
+        # legal (if unusual) directory name.
+        target = os.path.join(str(tmp_path), "plain{brace")
+        mkdirhier(target)
+        assert os.path.isdir(target)
+
+
+# ---------------------------------------------------------------------------
+# OSError handler
+# ---------------------------------------------------------------------------
+
+class TestMkdirhierErrorPath:
+    def test_oserror_under_a_file_component(self, tmp_path):
+        # Create a regular file, then try to mkdir a subdir *under* it. On
+        # Linux os.makedirs raises NotADirectoryError (an OSError subclass);
+        # the handler must let it surface rather than swallow it.
+        f = os.path.join(str(tmp_path), "afile")
+        Path(f).write_text("x")
+        target = os.path.join(f, "sub")
+        with pytest.raises(OSError):
+            mkdirhier(target)
+
+    def test_generic_oserror_propagates(self):
+        # An arbitrary OSError (EACCES != EEXIST) must propagate unchanged.
+        err = OSError()
+        err.errno = errno.EACCES
+        with mock.patch("wic.bb.utils.os.makedirs", side_effect=err):
+            with pytest.raises(OSError) as ei:
+                mkdirhier("/whatever/path")
+        assert ei.value.errno == errno.EACCES
+
+    def test_eexist_on_existing_dir_is_swallowed(self, tmp_path):
+        # An EEXIST OSError on a path that is already a directory is the
+        # benign race the handler exists to absorb; it must not propagate.
+        err = OSError()
+        err.errno = errno.EEXIST
+        with mock.patch("wic.bb.utils.os.makedirs", side_effect=err):
+            mkdirhier(str(tmp_path))  # exists and is a dir -> swallowed, no raise
+
+    def test_eexist_on_existing_file_propagates(self, tmp_path):
+        # EEXIST but the path is a regular file, not a directory: the
+        # handler's isdir() re-check fails, so the error must propagate.
+        f = os.path.join(str(tmp_path), "afile")
+        Path(f).write_text("x")
+        err = OSError()
+        err.errno = errno.EEXIST
+        with mock.patch("wic.bb.utils.os.makedirs", side_effect=err):
+            with pytest.raises(OSError) as ei:
+                mkdirhier(f)
+        assert ei.value.errno == errno.EEXIST
-- 
2.50.0.173.g8b6f19ccfc3a



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

* [wic][PATCH v3 10/10] tests/docs: add the review rubric
  2026-07-01  7:40 [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test Trevor Woerner
                   ` (8 preceding siblings ...)
  2026-07-01  7:40 ` [wic][PATCH v3 09/10] tests/unit/test_bb_utils: test mkdirhier() and fix its missing errno import Trevor Woerner
@ 2026-07-01  7:40 ` Trevor Woerner
  2026-07-06  9:08   ` [yocto-patches] " Paul Barker
  9 siblings, 1 reply; 21+ messages in thread
From: Trevor Woerner @ 2026-07-01  7:40 UTC (permalink / raw)
  To: yocto-patches

The suite already documents how to write a test (authoring.md) and how
it is linted (linting.md), but not how a test change is judged. This
adds tests/docs/reviewing.md, the review-time companion to authoring.md,
so the standard a change is held to is written down where both
reviewers and contributors can see it.

The rubric records the conventions the suite is built on:

  - one function per commit, with the subject keyed to the test file;
  - a test asserts the correct behaviour, and when it exposes a defect
    the source fix lands in the same commit rather than as an xfail or a
    test that bakes in the wrong result;
  - a fix must be proven to matter by backing it out and watching the
    test go red;
  - the suite is green and lint-clean at every commit, not only at the
    tip of a series;
  - assertions are specific, boundaries are probed, and an assertion is
    never weakened to force a pass;
  - coverage is read as a guide to untested branches, not a score;
  - each commit message stands alone and references no other commit.

It closes with a short reviewer checklist that collects those points,
and is linked from the docs table in the suite README.

AI-Generated: codex/claude-opus 4.8 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
changes in v3:
- new in v3: adds tests/docs/reviewing.md, the review rubric for a
  test change, and links it from the suite README.
---
 tests/docs/README.md    |   1 +
 tests/docs/reviewing.md | 164 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 165 insertions(+)
 create mode 100644 tests/docs/reviewing.md

diff --git a/tests/docs/README.md b/tests/docs/README.md
index ba7bd5e192a3..d44212a287ab 100644
--- a/tests/docs/README.md
+++ b/tests/docs/README.md
@@ -79,4 +79,5 @@ treated.
 | File | Content |
 |------|---------|
 | [authoring.md](authoring.md) | How to add a unit test to the suite |
+| [reviewing.md](reviewing.md) | How a test change is reviewed |
 | [linting.md](linting.md)     | How ruff is used on the suite |
diff --git a/tests/docs/reviewing.md b/tests/docs/reviewing.md
new file mode 100644
index 000000000000..a9b002a6dabd
--- /dev/null
+++ b/tests/docs/reviewing.md
@@ -0,0 +1,164 @@
+# Reviewing a test change
+
+This is the rubric a reviewer applies to a change that adds or edits
+tests, and the bar a contributor should meet before sending one. It is
+the review-time companion to [authoring.md](authoring.md): authoring.md
+says how to write a good test, this says how such a change is judged.
+
+## Contents
+
+- [One function per commit](#one-function-per-commit)
+- [Test and fix land together](#test-and-fix-land-together)
+- [Prove the fix matters](#prove-the-fix-matters)
+- [Green at every commit](#green-at-every-commit)
+- [Assertions and boundaries](#assertions-and-boundaries)
+- [Scratch files](#scratch-files)
+- [Coverage](#coverage)
+- [Commit message](#commit-message)
+- [Reviewer checklist](#reviewer-checklist)
+
+## One function per commit
+
+A commit covers one function under test. A module with a single
+function is one commit; a module with five functions is five commits,
+one per function. All the tests for that function (happy path, guards,
+boundaries, error paths) belong in the same commit, because they share
+the function and its setup; do not split a commit per assertion.
+
+The subject line names the test file and the function:
+
+```text
+tests/unit/test_<module>: test <function>()
+tests/unit/test_<module>: test <function>() and fix
+```
+
+The second form is for when a fix to the wic source rides along (see
+below). Keep the subject short: it says only "and fix", and the full
+explanation of what was fixed goes in the commit body. There is no
+leading `wic:`; the subject is keyed to the test file.
+
+## Test and fix land together
+
+A test asserts the behaviour wic is *expected* to provide, never the
+behaviour it currently happens to have. When a test exposes a defect,
+the fix to the wic source lands in the same commit, so the test asserts
+the correct behaviour and passes. A change that adds a test asserting a
+known-wrong result, or that marks a failing case `xfail` to defer the
+fix, does not meet the bar: it either bakes in the bug or leaves a red
+test behind. Reject both.
+
+## Prove the fix matters
+
+When a commit carries a fix, the test must actually exercise the bug.
+The cheap proof is to back the source change out and watch the test go
+red, then restore it and watch it pass:
+
+```bash
+git stash push -- src/wic/<file>
+tests/run-tests.sh tests/unit/test_<module>.py   # the new tests fail
+git stash pop
+tests/run-tests.sh tests/unit/test_<module>.py   # the new tests pass
+```
+
+A test that stays green with the fix removed is not testing the fix.
+Either the assertion is too weak or the wrong code path is exercised;
+in review, treat that as a finding.
+
+## Green at every commit
+
+The suite is green at every commit boundary, not just at the tip of the
+series: zero failures and zero `xfail` markers, with
+`tests/run-tests.sh --lint-tests` passing 100%, reporting nothing. The
+test tree is held to a clean bar; a single lint finding fails the
+commit. A series that only goes green at the end cannot be bisected and
+is not acceptable; check intermediate commits, not just `HEAD`.
+
+`--lint-src` is a preview report over the wic source, not a gate (see
+[linting.md](linting.md)); findings there do not block a test change.
+
+## Assertions and boundaries
+
+Every case asserts a specific outcome, an exact value or a specific
+exception, never merely that nothing was raised. A test that only
+checks for the absence of a crash passes against badly wrong output and
+gives false confidence.
+
+A good test probes the boundary of each parameter and just past it,
+rather than only the comfortable middle of its range. The classes worth
+covering (empty, zero, negative, maximum and one beyond, wrong types,
+path traversal, stale state, and so on) are enumerated in
+authoring.md under "Probe the boundaries"; in review, look for the ones
+that matter for the function at hand and note the ones that are
+missing.
+
+If an assertion was weakened to make a test pass, the change is going
+the wrong way. The answer to a hard-to-pass test is in the code or in
+understanding the correct behaviour, never in loosening the assertion.
+
+## Scratch files
+
+A test that needs scratch space uses pytest's `tmp_path` fixture, which
+gives each test its own directory and removes it automatically when the
+test passes (a failing test's directory is kept for inspection). A
+change that reaches for `tempfile.mkdtemp()`, `/tmp` directly, or any
+other hand-rolled scratch path is going the wrong way: those leak
+directories across runs and are worth a finding.
+
+The scratch tree lands under the system temporary directory by default.
+To send it elsewhere, when `/tmp` is small or a run produces a lot of
+scratch, use the standard pytest controls rather than a custom setting;
+both are passed straight through by `run-tests.sh` and are described in
+the suite [README](README.md#scratch-files):
+
+```bash
+TMPDIR=/path/to/scratch tests/run-tests.sh
+tests/run-tests.sh --basetemp=/path/to/scratch
+```
+
+## Coverage
+
+Coverage is a guide, not a score to maximise. Run it for the module
+under review and read which lines and branches the new tests reach:
+
+```bash
+tests/run-tests.sh --coverage tests/unit/test_<module>.py
+```
+
+Use it to confirm the boundary rule above was actually met: the
+function's own guards and error paths should be reached, not just its
+happy path. A reachable branch of the function under review that no
+test exercises usually means a boundary case is missing, so the fix is
+to add that case, not to chase a coverage number. A branch that cannot
+sensibly be triggered is not a gap.
+
+## Commit message
+
+Each commit stands alone. The body explains the function, the tests,
+and, when a fix rides along, what the bug was, why it was invisible
+before, and why the test and fix must land together; a short failing
+snippet helps. Do not reference other commits by number or hash, since
+the series may be reordered or rebased and such a reference would go
+stale. When a fix rides along, the body is where the fix is explained
+in full, since the subject only says "and fix". Do not weaken the
+explanation to save space: a reviewer reading the commit in isolation
+should understand the whole change.
+
+## Reviewer checklist
+
+- The commit covers exactly one function, with the subject in the form
+  above.
+- All of that function's cases (happy path, guards, boundaries, error
+  paths) are present and each asserts a specific value or exception.
+- If a fix rides along, it is in the same commit and the test fails
+  without it (verified by backing the fix out).
+- No `xfail`, no test asserting a known-wrong result, no weakened
+  assertion.
+- Scratch space uses the `tmp_path` fixture, not `mkdtemp()` or a raw
+  `/tmp` path.
+- The suite is green and `--lint-tests` is clean at this commit, not
+  only at the tip.
+- Coverage confirms the function's own guards and error paths are
+  reached; any reachable but untested branch is treated as a missing
+  boundary case.
+- The commit message stands alone, references no other commit, and
+  fully explains any fix that the short "and fix" subject only names.
-- 
2.50.0.173.g8b6f19ccfc3a



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

* Re: [yocto-patches] [wic][PATCH v3 02/10] tests: add a session banner via conftest.py
  2026-07-01  7:40 ` [wic][PATCH v3 02/10] tests: add a session banner via conftest.py Trevor Woerner
@ 2026-07-06  8:17   ` Paul Barker
  0 siblings, 0 replies; 21+ messages in thread
From: Paul Barker @ 2026-07-06  8:17 UTC (permalink / raw)
  To: yocto-patches

[-- Attachment #1: Type: text/plain, Size: 1798 bytes --]

On Wed, 2026-07-01 at 03:40 -0400, Trevor Woerner via
lists.yoctoproject.org wrote:
> When a test run starts it is useful to see, at a glance, exactly what
> is being tested: which host the run is on, which Python and pytest are
> in use, and -- most importantly -- which wic the suite imported and
> what version it reports. Without that, a passing or failing run is
> hard to attribute, especially when more than one wic checkout or
> virtualenv is in play.
> 
> pytest looks for a file named conftest.py and, among other things,
> calls its pytest_report_header() hook to print extra lines in the
> session header before collection begins. This commit adds that file
> with a single hook that prints a short banner:
> 
>   - the host node name, operating system, and machine type;
>   - the running Python version and the pytest version;
>   - the version wic reports and the filesystem path the wic module was
>     imported from.
> 
> The wic lookup is defensive: if wic cannot be imported (for example
> the test extras were not installed, or the suite is being run outside
> the checkout) the banner says so rather than aborting the run, so the
> failure is visible in the header instead of as an opaque collection
> error.
> 
> The file name conftest.py is mandatory; pytest discovers it by name,
> so it cannot be called anything else.
> 
> AI-Generated: codex/claude-opus 4.7 (xhigh)
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>

Claude is over-enthusiastic about writing explanations as usual - we
don't need to explain how conftest.py works. We don't need to explain an
obvious try/except around an import that may fail.

Othewise though, this looks good.

Reviewed-by: Paul Barker <paul@pbarker.dev>

Best regards,

-- 
Paul Barker


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

* Re: [yocto-patches] [wic][PATCH v3 01/10] tests: add the standalone test-suite skeleton
  2026-07-01  7:40 ` [wic][PATCH v3 01/10] tests: add the standalone test-suite skeleton Trevor Woerner
@ 2026-07-06  8:18   ` Paul Barker
  0 siblings, 0 replies; 21+ messages in thread
From: Paul Barker @ 2026-07-06  8:18 UTC (permalink / raw)
  To: yocto-patches

[-- Attachment #1: Type: text/plain, Size: 1699 bytes --]

On Wed, 2026-07-01 at 03:40 -0400, Trevor Woerner via
lists.yoctoproject.org wrote:
> wic currently has no test mechanism of its own; it relies on the
> oe-selftest from oe-core for all its testing, which means a full
> bitbake build is needed to exercise even pure-Python logic. This
> commit lays the groundwork for a small standalone suite that runs
> from a plain checkout with nothing but pytest, so that logic can be
> pinned down and kept stable as the code evolves.
> 
> This commit adds the skeleton only; it does not add any tests.
> 
> What this adds:
> 
>   - pyproject.toml: a "tests" optional-dependency group (pytest only)
>     so that "pip install -e .[tests]" pulls in what the suite needs,
>     plus a [tool.pytest.ini_options] section. The pytest options keep
>     a test's scratch directory only when that test fails, so repeated
>     runs do not accumulate leftover files.
> 
>   - tests/unit/ and tests/docs/: the directories that hold the
>     in-process unit suites and the suite documentation. Git cannot
>     track an empty directory, so a placeholder .gitkeep is committed
>     in each to create it. The .gitkeep files have no content and are
>     removed once real files land in those directories.
> 
>   - .gitignore: ignore the .pytest_cache/ directory that pytest
>     writes at the top of the checkout.
> 
>   - README.md: a Testing section pointing at the suite and at
>     tests/docs/, and a tests/* entry in the project layout list.
> 
> AI-Generated: codex/claude-opus 4.7 (xhigh)
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>

Reviewed-by: Paul Barker <paul@pbarker.dev>

Best regards,

-- 
Paul Barker


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

* Re: [yocto-patches] [wic][PATCH v3 03/10] tests: add the run-tests.sh wrapper
  2026-07-01  7:40 ` [wic][PATCH v3 03/10] tests: add the run-tests.sh wrapper Trevor Woerner
@ 2026-07-06  8:21   ` Paul Barker
  0 siblings, 0 replies; 21+ messages in thread
From: Paul Barker @ 2026-07-06  8:21 UTC (permalink / raw)
  To: yocto-patches

[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]

On Wed, 2026-07-01 at 03:40 -0400, Trevor Woerner via
lists.yoctoproject.org wrote:
> The suite is run with pytest, but invoking pytest directly has two
> sharp edges: it must be run from the right directory for the pyproject
> configuration to apply, and it happily runs against whatever Python
> happens to be first on PATH, even one that cannot import wic. A run
> against the wrong interpreter produces a misleading banner and, worse,
> can silently exercise a different wic than the one in the checkout.
> 
> This commit adds tests/run-tests.sh, a thin wrapper that removes both
> hazards:
> 
>   - it locates the repository root from its own path and changes there
>     before running, so it works regardless of the caller's directory;
> 
>   - it checks that the interpreter can import wic before handing off to
>     pytest, and fails loudly with the install command if it cannot;
> 
>   - with no arguments it runs the whole suite under tests/; any
>     argument it does not recognise (a path, -k EXPR, -v, ...) is passed
>     straight through to pytest, and an explicit -- forwards everything
>     after it.
> 
> The interpreter can be overridden with the PYTHON environment variable
> (default python3). -h/--help prints usage and exits.
> 
> The README Testing section gains the run-tests.sh invocation alongside
> the install step.
> 
> AI-Generated: codex/claude-opus 4.7 (xhigh)
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>

I don't think this is needed, since we already attempt to import wic in
conftest.py, we can call pytest.exit() from there if the import fails.

Best regards,

-- 
Paul Barker


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

* Re: [yocto-patches] [wic][PATCH v3 04/10] tests: add optional coverage reporting to run-tests.sh
  2026-07-01  7:40 ` [wic][PATCH v3 04/10] tests: add optional coverage reporting to run-tests.sh Trevor Woerner
@ 2026-07-06  8:33   ` Paul Barker
  0 siblings, 0 replies; 21+ messages in thread
From: Paul Barker @ 2026-07-06  8:33 UTC (permalink / raw)
  To: yocto-patches

[-- Attachment #1: Type: text/plain, Size: 2624 bytes --]

On Wed, 2026-07-01 at 03:40 -0400, Trevor Woerner via
lists.yoctoproject.org wrote:
> Knowing which lines a test run actually exercised is the difference
> between "the suite is green" and "the suite is green and we know what
> it touched". This commit wires branch coverage of the wic source into
> the runner, kept entirely opt-in so a plain run stays fast and quiet.
> 
> pyproject.toml gains coverage and pytest-cov in the tests extra, so
> "pip install -e .[tests]" pulls in what the new flags need.
> 
> run-tests.sh gains two options:
> 
>   - --coverage measures branch coverage of src/wic during the run and
>     prints a terminal report listing the lines that were missed;
> 
>   - --html [DIR] additionally writes a browsable HTML report (default
>     htmlcov/, or DIR if given) and implies --coverage.
> 
> If --coverage is requested but pytest-cov is not installed the runner
> fails loudly with the install command rather than running without the
> measurement it was asked for.
> 
> The coverage data file and the HTML report directory are build
> artifacts, so .gitignore learns to ignore .coverage and htmlcov/.
> 
> AI-Generated: codex/claude-opus 4.7 (xhigh)
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>

Ok, now the run-tests.sh script starts to make sense.

Have you considered something like `just`
(https://github.com/casey/just) as a command runner? It's much simpler
to maintain a `justfile` than a shell script.

Using claude (opus 4.8) to convert run-tests.sh (as it is at the end of
this series of patches) to a much simpler justfile, I get:

    # List the available recipes.
    default:
        @just --list

    # Run the test suite. Extra args pass through to pytest (e.g. `just test -k filemap`).
    test *args="tests":
        pytest {{args}}

    # Run the suite with a terminal branch-coverage report for wic.
    coverage *args="tests":
        pytest {{args}} --cov=wic --cov-branch --cov-report=term-missing

    # Run the suite with an HTML coverage report (default dir: htmlcov/).
    coverage-html dir="htmlcov" *args="tests":
        pytest {{args}} --cov=wic --cov-branch --cov-report=term-missing --cov-report=html:{{dir}}

    # Lint the test suite; held to a clean bar, so this must report nothing.
    lint-tests:
        ruff check tests

    # Lint src/ (not yet ruff-clean, so this is a preview report, not a gate).
    lint: lint-tests
        ruff check src

Feel free to keep the shell script if you want to, but I think the above
is easier to maintain :)

Best regards,

-- 
Paul Barker


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

* Re: [yocto-patches] [wic][PATCH v3 05/10] tests: add ruff linting to run-tests.sh
  2026-07-01  7:40 ` [wic][PATCH v3 05/10] tests: add ruff linting " Trevor Woerner
@ 2026-07-06  8:40   ` Paul Barker
  0 siblings, 0 replies; 21+ messages in thread
From: Paul Barker @ 2026-07-06  8:40 UTC (permalink / raw)
  To: yocto-patches

[-- Attachment #1: Type: text/plain, Size: 6415 bytes --]

On Wed, 2026-07-01 at 03:40 -0400, Trevor Woerner via
lists.yoctoproject.org wrote:
> A test suite is only trustworthy if its own code is clean, so this
> commit brings ruff into the runner and holds the test tree to a clean
> bar. It also makes it easy to preview what ruff thinks of the wic
> source, without yet enforcing it.
> 
> pyproject.toml gains ruff in the tests extra and a [tool.ruff] section.
> The configuration is deliberately minimal for now: the test suite is
> the only tree under an enforced clean bar; the wic source under src/ is
> not yet ruff-clean and is reported, not gated.
> 
> run-tests.sh gains two lint modes, each used on its own:
> 
>   - --lint-tests runs ruff over tests/ and exits. The test suite must
>     report nothing; a finding here is a bug in our own test code and is
>     expected to be fixed.
> 
>   - --lint-src runs ruff over src/ and exits. The source is not yet
>     ruff-clean, so this is a preview: the runner prints ruff's findings
>     and exits with its status, but nothing in the suite asserts on
>     them. Keeping the two trees on separate flags means cleaning up the
>     source later does not disturb the test-tree gate.
> 
> A lint mode cannot be combined with coverage, with the other lint mode,
> or with pytest arguments; the runner rejects such combinations loudly
> rather than silently dropping the extras. If ruff is not installed it
> fails with the install command.
> 
> tests/docs/linting.md documents the two modes and why src/ is held back
> for now. That file replaces the tests/docs/.gitkeep placeholder, which
> is no longer needed now that the directory has real content.
> 
> .gitignore learns to ignore ruff's .ruff_cache/ directory.
> 
> AI-Generated: codex/claude-opus 4.7 (xhigh)
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> ---
> changes in v3:
> - no change in this revision.
> changes in v2:
> - v1 submitted the entire test suite as a single commit; v2 breaks
>   the work into a reviewable series, and this patch is one step of it.
> ---
>  .gitignore            |  3 +++
>  pyproject.toml        |  8 ++++++++
>  tests/docs/.gitkeep   |  0
>  tests/docs/linting.md | 39 +++++++++++++++++++++++++++++++++++
>  tests/run-tests.sh    | 47 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 97 insertions(+)
>  delete mode 100644 tests/docs/.gitkeep
>  create mode 100644 tests/docs/linting.md
> 
> diff --git a/.gitignore b/.gitignore
> index 534c49538091..3c3cfb328fb0 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -6,3 +6,6 @@
>  # coverage data and reports
>  /.coverage
>  /htmlcov/
> +
> +# ruff cache
> +/.ruff_cache/
> diff --git a/pyproject.toml b/pyproject.toml
> index ece2757bb686..656adcd4930a 100644
> --- a/pyproject.toml
> +++ b/pyproject.toml
> @@ -26,6 +26,7 @@ tests = [
>      "pytest >= 7.0",
>      "coverage >= 7.0",
>      "pytest-cov >= 4.0",
> +    "ruff >= 0.5",

The latest ruff version is 0.15.20, it's had lots of breaking changes
since v0.5 in 2024. Be careful with Claude filling in dependency
versions, it often picks outdated or inappropriate versions to depend
on, always check them by hand.

>  ]
>  
>  [project.scripts]
> @@ -50,3 +51,10 @@ path = "src/wic/cli.py"
>  # leftover files under the pytest base temp directory.
>  tmp_path_retention_policy = "failed"
>  tmp_path_retention_count  = 1
> +
> +[tool.ruff]
> +# For now only the test suite is actually linted (run-tests.sh
> +# --lint-tests passes the tests/ path); the wic source under src/ is
> +# not yet ruff-clean and is left out until its findings are fixed (see
> +# tests/docs/linting.md). --lint-src can still be run to preview the
> +# source findings, but it is reported, not enforced.
> diff --git a/tests/docs/.gitkeep b/tests/docs/.gitkeep
> deleted file mode 100644
> index e69de29bb2d1..000000000000
> diff --git a/tests/docs/linting.md b/tests/docs/linting.md
> new file mode 100644
> index 000000000000..71b4de21c100
> --- /dev/null
> +++ b/tests/docs/linting.md
> @@ -0,0 +1,39 @@
> +# Linting
> +
> +## Contents
> +
> +- [Running the linter](#running-the-linter)
> +- [tests/ must be clean](#tests-must-be-clean)
> +- [src/ is not linted yet](#src-is-not-linted-yet)
> +
> +The test suite is linted with [ruff](https://docs.astral.sh/ruff/). It
> +is configured in `pyproject.toml` (`[tool.ruff]`).
> +
> +## Running the linter
> +
> +The runner exposes ruff through two separate modes, each used on its
> +own:
> +
> +```bash
> +tests/run-tests.sh --lint-tests   # ruff over tests/
> +tests/run-tests.sh --lint-src     # ruff over src/ (preview only)
> +```
> +
> +A lint mode cannot be combined with coverage, with the other lint
> +mode, or with pytest arguments; the runner rejects such combinations.
> +
> +## tests/ must be clean
> +
> +Our own test code is held to a clean bar: `tests/run-tests.sh
> +--lint-tests` reports nothing. If you add a test that trips a rule, fix
> +the test before the change lands.
> +
> +## src/ is not linted yet
> +
> +`--lint-src` runs ruff over the wic source, but the source is **not**
> +yet ruff-clean, so its findings are a preview report rather than a
> +gate: the runner prints them and exits with ruff's status, but nothing
> +in the suite asserts on them. Treating `src/` findings as a hard
> +failure now would block every run on fixes that have not landed. Once
> +the source is cleaned up, `src/` can be promoted to the same clean bar
> +as `tests/`.

I am a bit concerned about a test suite needing its own docs directory.
Claude likes to write output I guess. But this level of detail is
unnecessary and is a pain to keep up-to-date with any changes in the
test suite.

You probably just need a top level TESTING.md file or something, with a
stripped down set of docs on running and authoring tests, any
assumptions that aren't documented in the test code, etc.

If you want to enforce that tests is kept clean of linter errors,
perhaps use pre-commit (https://pre-commit.com) to run the linter before
committing. This pairs well with using Claude for development as it
forces the AI agent to fix the errors before making a commit. If it's
automated, it also doesn't need documenting so much, just point at how
to enable pre-commit.

Best regards,

-- 
Paul Barker



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

* Re: [yocto-patches] [wic][PATCH v3 06/10] tests/docs: add the suite overview README
  2026-07-01  7:40 ` [wic][PATCH v3 06/10] tests/docs: add the suite overview README Trevor Woerner
@ 2026-07-06  8:47   ` Paul Barker
  0 siblings, 0 replies; 21+ messages in thread
From: Paul Barker @ 2026-07-06  8:47 UTC (permalink / raw)
  To: yocto-patches

[-- Attachment #1: Type: text/plain, Size: 5460 bytes --]

On Wed, 2026-07-01 at 03:40 -0400, Trevor Woerner via
lists.yoctoproject.org wrote:
> The test suite needs a front door: a short document that says what the
> suite is, how it is laid out, and how to run it. This commit adds
> tests/docs/README.md to be that overview.
> 
> The README covers:
> 
>   - the layout of tests/, file by file;
>   - that the suite is unit-only and needs no host tools or build
>     environment, so it runs from a plain checkout;
>   - how to install the test extras and invoke run-tests.sh, pointing at
>     run-tests.sh --help as the authoritative, always-current list of
>     options rather than duplicating them here where they would drift;
>   - that anything run-tests.sh does not recognise is passed through to
>     pytest;
>   - a pointer to linting.md for how ruff is applied;
>   - a small table of the other documents under tests/docs/.
> 
> It deliberately does not enumerate every run-tests.sh option, since the
> runner is expected to grow more of them; the table and the --help
> output stay the source of truth.
> 
> AI-Generated: codex/claude-opus 4.7 (xhigh)
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> ---
> changes in v3:
> - document the standard pytest scratch-directory mechanisms
>   (TMPDIR and --basetemp) in the suite README, so there is no need
>   for a custom temporary-directory variable.
> changes in v2:
> - v1 submitted the entire test suite as a single commit; v2 breaks
>   the work into a reviewable series, and this patch is one step of it.
> ---
>  tests/docs/README.md | 82 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 tests/docs/README.md
> 
> diff --git a/tests/docs/README.md b/tests/docs/README.md
> new file mode 100644
> index 000000000000..ba7bd5e192a3
> --- /dev/null
> +++ b/tests/docs/README.md
> @@ -0,0 +1,82 @@
> +# wic test suite
> +
> +A standalone test suite for the wic source tree. It runs from a plain
> +checkout with nothing but `pytest` -- no bitbake, no OpenEmbedded
> +build, and no target image -- so wic's logic can be exercised and kept
> +stable as the code changes.
> +
> +## Contents
> +
> +- [Layout](#layout)
> +- [Running](#running)
> +- [Linting](#linting)
> +- [Documentation](#documentation)

A table of contents like this will easily fall out of sync with the rest
of the file. Claude likes to generate them, but it doesn't always
remember to update them. I don't think it's needed for such a small
file. If you think it's worth keeping, use something like md-toc
(https://github.com/frnmst/md-toc) and integrate it with pre-commit.

> +
> +## Layout
> +
> +```
> +tests/
> +  conftest.py        session banner describing the run
> +  run-tests.sh       wrapper for running the suite
> +  unit/              unit tests that import wic modules directly
> +  docs/              this documentation
> +```
> +
> +The suite is unit-only: every test under `tests/unit/` imports a wic
> +module in-process and asserts on its behaviour, so none of it needs
> +host tools or a build environment.
> +
> +## Running
> +
> +Install wic with its test extras, then run the suite:
> +
> +```bash
> +pip install -e ".[tests]"
> +tests/run-tests.sh
> +```
> +
> +`run-tests.sh` works from anywhere in the checkout. It can also report
> +branch coverage of the wic source; run `tests/run-tests.sh --help` for
> +the current list of options.
> +
> +Anything `run-tests.sh` does not recognise is handed straight to
> +pytest, so the whole pytest command line is available:
> +
> +```bash
> +tests/run-tests.sh -k filemap -v          # one area, verbose
> +tests/run-tests.sh tests/unit             # a single tier or file
> +```
> +
> +## Scratch files
> +
> +Tests that need scratch space use pytest's `tmp_path` fixture, so there
> +is no wic-specific temporary-directory setting. Scratch directories are
> +created under pytest's base temporary directory and cleaned up according
> +to the retention policy in `pyproject.toml` (a passing test's directory
> +is removed, a failing one is kept).
> +
> +By default pytest roots that base directory at the system temporary
> +directory (usually `/tmp`). If that fills up, or you want the scratch
> +files somewhere else, use the standard pytest mechanisms rather than a
> +custom variable. Both are passed straight through by `run-tests.sh`:
> +
> +```bash
> +TMPDIR=/path/to/scratch tests/run-tests.sh   # honoured by pytest
> +tests/run-tests.sh --basetemp=/path/to/scratch
> +```
> +
> +`--basetemp` puts everything under the given directory and clears it at
> +the start of each run; `TMPDIR` keeps the last few sessions there.

Claude loves to write! You can just say that pytest's tmp_path fixture
is used and link to its docs
(https://docs.pytest.org/en/stable/how-to/tmp_path.html#the-tmp-path-fixture).

> +
> +## Linting
> +
> +The test suite is linted with ruff and is held to a clean bar. See
> +[linting.md](linting.md) for the lint modes and how the source tree is
> +treated.
> +
> +## Documentation
> +
> +| File | Content |
> +|------|---------|
> +| [authoring.md](authoring.md) | How to add a unit test to the suite |
> +| [linting.md](linting.md)     | How ruff is used on the suite |

Tables like this add very little value and fall out-of-sync quickly.

Best regards,

-- 
Paul Barker


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

* Re: [yocto-patches] [wic][PATCH v3 07/10] tests/docs: add the test-authoring guide
  2026-07-01  7:40 ` [wic][PATCH v3 07/10] tests/docs: add the test-authoring guide Trevor Woerner
@ 2026-07-06  8:52   ` Paul Barker
  0 siblings, 0 replies; 21+ messages in thread
From: Paul Barker @ 2026-07-06  8:52 UTC (permalink / raw)
  To: yocto-patches

[-- Attachment #1: Type: text/plain, Size: 7500 bytes --]

On Wed, 2026-07-01 at 03:40 -0400, Trevor Woerner via
lists.yoctoproject.org wrote:
> A suite is only as good as the tests added to it over time, so it needs
> a guide that says where a new test goes, what one looks like, and how
> to choose its inputs and assertions. This commit adds
> tests/docs/authoring.md.
> 
> The guide covers:
> 
>   - where a test goes: one test_<area>.py per wic module under
>     tests/unit/, extending an existing file or starting a new one;
> 
>   - the shape of a test: a small worked example, a preference for
>     parametrize over in-test loops so each input/output pair is its own
>     named case, and use of the tmp_path fixture for scratch files;
> 
>   - asserting the correct behaviour rather than whatever wic currently
>     does, so a test never bakes in a bug; when a test exposes a defect,
>     the source fix lands in the same change so the test passes;
> 
>   - probing the boundaries: a checklist of input classes worth covering
>     for numbers, strings, paths, types/shape, and state, each asserting
>     a specific value or exception rather than merely "it did not
>     crash";
> 
>   - keeping the assertion strong: never weaken an assertion to get
>     green;
> 
>   - how to run just the test you wrote, with and without coverage.
> 
> AI-Generated: codex/claude-opus 4.7 (xhigh)
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>

Is this here to help human test authors, or to help Claude? A lot of it
seems very generic, e.g. suggesting which boundaries to test is not
specific to wic.

If it's to guide the AI agent, then say that explicitly and reference it
from a top-level AGENTS.md file. I'm not a big fan of this stuff
littering repositories, but if you're going to be using AI code
generation heavily then if may be needed.

> ---
> changes in v3:
> - no change in this revision.
> changes in v2:
> - v1 submitted the entire test suite as a single commit; v2 breaks
>   the work into a reviewable series, and this patch is one step of it.
> ---
>  tests/docs/authoring.md | 124 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 124 insertions(+)
>  create mode 100644 tests/docs/authoring.md
> 
> diff --git a/tests/docs/authoring.md b/tests/docs/authoring.md
> new file mode 100644
> index 000000000000..c60cceb2d32d
> --- /dev/null
> +++ b/tests/docs/authoring.md
> @@ -0,0 +1,124 @@
> +# Authoring a unit test
> +
> +Unit tests live under `tests/unit/`, one module per area of wic, named
> +`test_<area>.py`. They import the wic module under test directly and
> +run in-process, so they need no host tools and no build environment.
> +
> +## Contents
> +
> +- [Where a test goes](#where-a-test-goes)
> +- [Shape of a test](#shape-of-a-test)
> +- [Assert the correct behaviour](#assert-the-correct-behaviour)
> +- [Probe the boundaries](#probe-the-boundaries)
> +- [Keep the assertion strong](#keep-the-assertion-strong)
> +- [Running what you wrote](#running-what-you-wrote)
> +
> +## Where a test goes
> +
> +Group tests by the wic module they exercise, one `test_<area>.py` per
> +module. Start a new file when you begin covering a module that has no
> +file yet; otherwise extend the existing one.
> +
> +## Shape of a test
> +
> +```python
> +import pytest
> +
> +from wic.ksparser import sizetype
> +
> +
> +class TestSizetype:
> +    # sizetype(default, size_in_bytes=False) returns a parser f(arg);
> +    # with default "M", a bare number is read as mebibytes-in-KiB.
> +    def test_plain_value_uses_default_unit(self):
> +        parse = sizetype("M")
> +        assert parse("100") == 100 * 1024
> +
> +    @pytest.mark.parametrize("arg,expected", [
> +        ("1K", 1),
> +        ("1G", 1 * 1024 * 1024),
> +        ("0", 0),
> +    ])
> +    def test_suffixes(self, arg, expected):
> +        assert sizetype("M")(arg) == expected
> +```
> +
> +Prefer `@pytest.mark.parametrize` to express each input/output pair as
> +its own case rather than looping inside one test, so a single bad
> +value shows up as one named failure.
> +
> +Use `pytest`'s `tmp_path` fixture for any scratch files; a passing
> +test's scratch directory is removed automatically (see the retention
> +policy in `pyproject.toml`), and a failing one is kept for inspection.
> +
> +## Assert the correct behaviour
> +
> +Every assertion states the behaviour wic is *expected* to provide,
> +never the behaviour it currently happens to have. A test that locks in
> +a wrong result is worse than no test: it gives false confidence and it
> +turns red exactly when someone repairs the code.
> +
> +When a test exposes a defect, the fix to the wic source lands in the
> +same change as the test, so the test asserts the correct behaviour and
> +passes. Do not commit a test that asserts a known-wrong result.
> +
> +Wrong -- bakes in the bug:
> +
> +```python
> +# value lost its comma; asserting the broken output
> +assert result["file"] == "s3://bucket/a"
> +```
> +
> +Right -- asserts the correct output (and the fix lands with it):
> +
> +```python
> +assert result["file"] == "s3://bucket/a,b.img"
> +```
> +
> +## Probe the boundaries
> +
> +For a parameter, probe the boundary and just past it rather than only
> +the comfortable middle of its range. The classes worth covering:
> +
> +- numbers: empty, zero, negative, the maximum and one beyond it,
> +  non-power-of-two and non-multiple-of-1024/1000 values, primes,
> +  fractional values where an integer is assumed, `0` versus `0.0`
> +  versus `"0"`, off-by-one at a block or sector boundary, and very
> +  large magnitudes near `2**31` and `2**63`
> +- strings: the empty string, whitespace only, embedded spaces, tabs,
> +  newlines and CRLF endings, non-ASCII characters, shell
> +  metacharacters in a value that becomes part of a command line,
> +  leading-dash values that look like options, over-delimited or
> +  malformed forms (stray commas, doubled or missing separators), and
> +  very long values
> +- paths: traversal (`../`), doubled slashes, a trailing slash or its
> +  absence, `.` and `..` components, broken or looping symlinks, and a
> +  non-existent path
> +- types and shape: each wrong type the parameter might receive (a
> +  string where an integer is expected, a list where a string is
> +  expected, `None`, and so on), the wrong argument count, and
> +  duplicate keys or entries
> +- state: a second call that must not see stale cached data, a
> +  `cache=False` path that must evict, calling an operation twice, and
> +  finalising a half-constructed object
> +
> +Each case asserts a specific outcome -- an exact value, or a specific
> +exception -- rather than merely "it did not crash." A test that only
> +checks for the absence of an exception will pass against badly wrong
> +output.
> +
> +## Keep the assertion strong
> +
> +If a test is hard to make pass, the answer is in the code or in
> +understanding the correct behaviour, never in weakening the assertion
> +to something vague enough to pass. Loosening an assertion to get green
> +is how a suite quietly stops catching regressions.
> +
> +## Running what you wrote
> +
> +```bash
> +tests/run-tests.sh tests/unit/test_<area>.py -v
> +tests/run-tests.sh --coverage tests/unit/test_<area>.py
> +```
> +
> +A new test should leave the suite green, with zero failures.

-- 
Paul Barker


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

* Re: [yocto-patches] [wic][PATCH v3 08/10] tests: ignore E402 in the test tree for the sys.path bootstrap
  2026-07-01  7:40 ` [wic][PATCH v3 08/10] tests: ignore E402 in the test tree for the sys.path bootstrap Trevor Woerner
@ 2026-07-06  8:55   ` Paul Barker
  0 siblings, 0 replies; 21+ messages in thread
From: Paul Barker @ 2026-07-06  8:55 UTC (permalink / raw)
  To: yocto-patches

[-- Attachment #1: Type: text/plain, Size: 4170 bytes --]

On Wed, 2026-07-01 at 03:40 -0400, Trevor Woerner via
lists.yoctoproject.org wrote:
> The unit tests that follow run against a plain checkout that has not
> been installed. To do that, each test module prepends the in-tree src/
> directory to sys.path and only then imports wic:
> 
>     _SRC = Path(__file__).resolve().parent.parent.parent / "src"
>     if str(_SRC) not in sys.path:
>         sys.path.insert(0, str(_SRC))
> 
>     from wic.bb.utils import mkdirhier
> 
> That bootstrap necessarily runs before the wic imports, which trips
> ruff's E402 (module-level import not at top of file). The ordering is
> required, not accidental: the import would fail without the sys.path
> adjustment ahead of it.
> 
> Rather than scatter per-line noqa comments through every test module,
> this commit relaxes E402 for the test tree once, via a
> [tool.ruff.lint.per-file-ignores] entry scoped to tests/**. It is the
> only rule relaxed for tests/; the suite is otherwise held to a clean
> bar by --lint-tests.
> 
> tests/docs/linting.md gains a section describing this single exception
> and why it exists.
> 
> AI-Generated: codex/claude-opus 4.7 (xhigh)
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> ---
> changes in v3:
> - no change in this revision.
> changes in v2:
> - v1 submitted the entire test suite as a single commit; v2 breaks
>   the work into a reviewable series, and this patch is one step of it.
> ---
>  pyproject.toml        |  8 ++++++++
>  tests/docs/linting.md | 20 ++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/pyproject.toml b/pyproject.toml
> index 656adcd4930a..fb42fe6dd135 100644
> --- a/pyproject.toml
> +++ b/pyproject.toml
> @@ -58,3 +58,11 @@ tmp_path_retention_count  = 1
>  # not yet ruff-clean and is left out until its findings are fixed (see
>  # tests/docs/linting.md). --lint-src can still be run to preview the
>  # source findings, but it is reported, not enforced.
> +
> +[tool.ruff.lint.per-file-ignores]
> +# Test modules prepend the in-tree src/ directory to sys.path before
> +# importing wic, so the suite runs against a checkout that has not been
> +# installed. That bootstrap necessarily runs before the wic imports,
> +# which trips E402 (module-level import not at top of file). The
> +# ordering is deliberate; ignore E402 for the test tree only.
> +"tests/**" = ["E402"]

LTGM.

> diff --git a/tests/docs/linting.md b/tests/docs/linting.md
> index 71b4de21c100..1c1bfcc06f82 100644
> --- a/tests/docs/linting.md
> +++ b/tests/docs/linting.md
> @@ -4,6 +4,7 @@
>  
>  - [Running the linter](#running-the-linter)
>  - [tests/ must be clean](#tests-must-be-clean)
> +- [The one intentional exception in tests/](#the-one-intentional-exception-in-tests)
>  - [src/ is not linted yet](#src-is-not-linted-yet)
>  
>  The test suite is linted with [ruff](https://docs.astral.sh/ruff/). It
> @@ -28,6 +29,25 @@ Our own test code is held to a clean bar: `tests/run-tests.sh
>  --lint-tests` reports nothing. If you add a test that trips a rule, fix
>  the test before the change lands.
>  
> +## The one intentional exception in tests/
> +
> +Test modules prepend the in-tree `src/` directory to `sys.path` before
> +importing `wic`, so the suite runs against a plain checkout that has not
> +been installed:
> +
> +```python
> +_SRC = Path(__file__).resolve().parent.parent.parent / "src"
> +if str(_SRC) not in sys.path:
> +    sys.path.insert(0, str(_SRC))
> +
> +from wic.bb.utils import mkdirhier
> +```
> +
> +That bootstrap necessarily runs before the `wic` imports, which trips
> +`E402` (module-level import not at top of file). The ordering is
> +required, so `E402` is ignored for the test tree via `per-file-ignores`
> +in `pyproject.toml`. This is the only rule relaxed for `tests/`.
> +
>  ## src/ is not linted yet
>  
>  `--lint-src` runs ruff over the wic source, but the source is **not**

Argh! Claude! No!

This is well documented by the comment in pyproject.toml, it doesn't
need detailed explanation in a documentation file.

Best regards,

-- 
Paul Barker


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

* Re: [yocto-patches] [wic][PATCH v3 09/10] tests/unit/test_bb_utils: test mkdirhier() and fix its missing errno import
  2026-07-01  7:40 ` [wic][PATCH v3 09/10] tests/unit/test_bb_utils: test mkdirhier() and fix its missing errno import Trevor Woerner
@ 2026-07-06  9:06   ` Paul Barker
  0 siblings, 0 replies; 21+ messages in thread
From: Paul Barker @ 2026-07-06  9:06 UTC (permalink / raw)
  To: yocto-patches

[-- Attachment #1: Type: text/plain, Size: 8989 bytes --]

On Wed, 2026-07-01 at 03:40 -0400, Trevor Woerner via
lists.yoctoproject.org wrote:
> mkdirhier() wraps os.makedirs() and, on OSError, re-checks the error to
> decide whether to swallow a benign "already exists" race or re-raise:
> 
>     except OSError as e:
>         if e.errno != errno.EEXIST or not os.path.isdir(directory):
>             raise e
> 
> but the module never imports errno. The reference to errno.EEXIST is
> only evaluated when os.makedirs() actually raises, so the defect is
> invisible on the happy path and on the unexpanded-${} guard. The moment
> a real OSError occurs -- a parent component that is a file, a permission
> error, anything -- the handler itself raises NameError: name 'errno' is
> not defined, masking the original error with a misleading one.
> 
> The fix is a one-line import. With errno available the handler behaves
> as intended: an EEXIST on an existing directory is swallowed, while any
> other OSError (and an EEXIST whose path is not a directory) propagates
> unchanged.
> 
> This commit adds tests/unit/test_bb_utils.py, covering mkdirhier() end
> to end:
> 
>   - the happy path (nested, single-level, idempotent, existing dir);
>   - the unexpanded-${} guard (rejected, nothing created, and that a
>     bare brace without a dollar is allowed);
>   - the OSError handler: a real mkdir under a file component surfaces an
>     OSError, an arbitrary OSError propagates with its errno intact, an
>     EEXIST on an existing directory is swallowed, and an EEXIST on a
>     regular file propagates.
> 
> The error-path tests fail with NameError without the import and pass
> with it, so the test and the fix belong together in this one change.
> 
> AI-Generated: codex/claude-opus 4.8 (xhigh)
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> ---
> changes in v3:
> - switch the tests from tempfile.mkdtemp() to pytest's tmp_path
>   fixture so each test's scratch directory is cleaned up instead of
>   leaking under /tmp; no change to what is tested.
> changes in v2:
> - v1 recorded this bug with an xfail marker in one large commit;
>   v2 drops the xfail, asserts the correct behaviour directly, and
>   lands the one-line errno-import fix in this same commit so the
>   test passes green.
> ---
>  src/wic/bb/utils.py         |   1 +
>  tests/unit/test_bb_utils.py | 116 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+)
>  create mode 100644 tests/unit/test_bb_utils.py
> 
> diff --git a/src/wic/bb/utils.py b/src/wic/bb/utils.py
> index 3750056ba563..0b40dd0c1d59 100644
> --- a/src/wic/bb/utils.py
> +++ b/src/wic/bb/utils.py
> @@ -1,6 +1,7 @@
>  """
>  Minimal subset of BitBake's bb.utils used by standalone wic.
>  """
> +import errno
>  import os
>  
>  # from bitbake/lib/bb/utils.py

Nitpick: It's better to fix the bug in a separate commit before the test
is added.

> diff --git a/tests/unit/test_bb_utils.py b/tests/unit/test_bb_utils.py
> new file mode 100644
> index 000000000000..aeca054109ff
> --- /dev/null
> +++ b/tests/unit/test_bb_utils.py
> @@ -0,0 +1,116 @@
> +r"""
> +Unit tests for wic.bb.utils -- a small standalone subset of BitBake's
> +bb.utils. mkdirhier() is a mkdir -p wrapper with two guards: an
> +unexpanded-${} check and an OSError handler that re-checks EEXIST.
> +"""

This doesn't need to be a raw string.

> +import errno
> +import os
> +import sys
> +import unittest.mock as mock
> +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))
> +
> +from wic.bb.utils import mkdirhier
> +
> +
> +# ---------------------------------------------------------------------------
> +# Happy path
> +# ---------------------------------------------------------------------------
> +
> +class TestMkdirhierHappyPath:
> +    def test_creates_nested_directory(self, tmp_path):
> +        target = os.path.join(str(tmp_path), "a", "b", "c")
> +        mkdirhier(target)
> +        assert os.path.isdir(target)
> +
> +    def test_idempotent_on_existing_directory(self, tmp_path):
> +        target = os.path.join(str(tmp_path), "x")
> +        mkdirhier(target)
> +        # Second call must not raise (exist_ok=True).
> +        mkdirhier(target)
> +        assert os.path.isdir(target)
> +
> +    def test_single_level(self, tmp_path):
> +        target = os.path.join(str(tmp_path), "single")
> +        mkdirhier(target)
> +        assert os.path.isdir(target)
> +
> +    def test_existing_root_directory(self, tmp_path):
> +        # An already-existing directory (the tmpdir itself) is a no-op.
> +        mkdirhier(str(tmp_path))
> +        assert os.path.isdir(str(tmp_path))

Are we testing mkdirhier() or testing Python's os.makedirs()
implementation?

There's no need to re-test Python's standard library. We only need to
test the functionality we add in mkdirhier(), and probably a single
basic "can it create a directory" test.

> +
> +
> +# ---------------------------------------------------------------------------
> +# Unexpanded bitbake-variable guard
> +# ---------------------------------------------------------------------------
> +
> +class TestMkdirhierUnexpandedVar:
> +    def test_unexpanded_var_rejected(self):
> +        with pytest.raises(Exception) as ei:
> +            mkdirhier("/tmp/${WORKDIR}/x")
> +        assert "unexpanded bitbake variable" in str(ei.value)
> +
> +    def test_unexpanded_var_not_created(self, tmp_path):
> +        bad = os.path.join(str(tmp_path), "${VAR}", "sub")
> +        with pytest.raises(Exception):
> +            mkdirhier(bad)
> +        # Nothing should have been created.
> +        assert not os.path.exists(os.path.join(str(tmp_path), "${VAR}"))

This assert could be merged into the previous test.

> +
> +    def test_brace_without_dollar_is_allowed(self, tmp_path):
> +        # Only the literal '${' marker triggers the guard; a bare '{' is a
> +        # legal (if unusual) directory name.
> +        target = os.path.join(str(tmp_path), "plain{brace")
> +        mkdirhier(target)
> +        assert os.path.isdir(target)
> +
> +
> +# ---------------------------------------------------------------------------
> +# OSError handler
> +# ---------------------------------------------------------------------------
> +
> +class TestMkdirhierErrorPath:
> +    def test_oserror_under_a_file_component(self, tmp_path):
> +        # Create a regular file, then try to mkdir a subdir *under* it. On
> +        # Linux os.makedirs raises NotADirectoryError (an OSError subclass);
> +        # the handler must let it surface rather than swallow it.
> +        f = os.path.join(str(tmp_path), "afile")
> +        Path(f).write_text("x")
> +        target = os.path.join(f, "sub")
> +        with pytest.raises(OSError):
> +            mkdirhier(target)
> +
> +    def test_generic_oserror_propagates(self):
> +        # An arbitrary OSError (EACCES != EEXIST) must propagate unchanged.
> +        err = OSError()
> +        err.errno = errno.EACCES
> +        with mock.patch("wic.bb.utils.os.makedirs", side_effect=err):
> +            with pytest.raises(OSError) as ei:
> +                mkdirhier("/whatever/path")
> +        assert ei.value.errno == errno.EACCES
> +
> +    def test_eexist_on_existing_dir_is_swallowed(self, tmp_path):
> +        # An EEXIST OSError on a path that is already a directory is the
> +        # benign race the handler exists to absorb; it must not propagate.
> +        err = OSError()
> +        err.errno = errno.EEXIST
> +        with mock.patch("wic.bb.utils.os.makedirs", side_effect=err):
> +            mkdirhier(str(tmp_path))  # exists and is a dir -> swallowed, no raise
> +
> +    def test_eexist_on_existing_file_propagates(self, tmp_path):
> +        # EEXIST but the path is a regular file, not a directory: the
> +        # handler's isdir() re-check fails, so the error must propagate.
> +        f = os.path.join(str(tmp_path), "afile")
> +        Path(f).write_text("x")
> +        err = OSError()
> +        err.errno = errno.EEXIST
> +        with mock.patch("wic.bb.utils.os.makedirs", side_effect=err):
> +            with pytest.raises(OSError) as ei:
> +                mkdirhier(f)
> +        assert ei.value.errno == errno.EEXIST

Claude loves to write test cases like this, that test specific little
details of the current implementation. This needs to be re-framed, test
the expected behaviour not the implementation details.

And to take a step back - if mkdirhier() is imported from bitbake, why
add test cases here anyway? If bitbake is missing test coverage then
it's better to add it there and plan to keep src/wic/bb in sync with
any future changes in bitbake.

Best regards,

-- 
Paul Barker


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

* Re: [yocto-patches] [wic][PATCH v3 10/10] tests/docs: add the review rubric
  2026-07-01  7:40 ` [wic][PATCH v3 10/10] tests/docs: add the review rubric Trevor Woerner
@ 2026-07-06  9:08   ` Paul Barker
  0 siblings, 0 replies; 21+ messages in thread
From: Paul Barker @ 2026-07-06  9:08 UTC (permalink / raw)
  To: yocto-patches

[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]

On Wed, 2026-07-01 at 03:40 -0400, Trevor Woerner via
lists.yoctoproject.org wrote:
> The suite already documents how to write a test (authoring.md) and how
> it is linted (linting.md), but not how a test change is judged. This
> adds tests/docs/reviewing.md, the review-time companion to authoring.md,
> so the standard a change is held to is written down where both
> reviewers and contributors can see it.
> 
> The rubric records the conventions the suite is built on:
> 
>   - one function per commit, with the subject keyed to the test file;
>   - a test asserts the correct behaviour, and when it exposes a defect
>     the source fix lands in the same commit rather than as an xfail or a
>     test that bakes in the wrong result;
>   - a fix must be proven to matter by backing it out and watching the
>     test go red;
>   - the suite is green and lint-clean at every commit, not only at the
>     tip of a series;
>   - assertions are specific, boundaries are probed, and an assertion is
>     never weakened to force a pass;
>   - coverage is read as a guide to untested branches, not a score;
>   - each commit message stands alone and references no other commit.
> 
> It closes with a short reviewer checklist that collects those points,
> and is linked from the docs table in the suite README.
> 
> AI-Generated: codex/claude-opus 4.8 (xhigh)
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>

The comments I've made earlier in the series apply here as well. Is this
for the AI agent? Does it need to explain such generic ideas as
one change per commit?

Best regards,

-- 
Paul Barker


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

end of thread, other threads:[~2026-07-06  9:08 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-01  7:40 [wic][PATCH v3 00/10] tests: standalone test-suite framework plus the first unit test Trevor Woerner
2026-07-01  7:40 ` [wic][PATCH v3 01/10] tests: add the standalone test-suite skeleton Trevor Woerner
2026-07-06  8:18   ` [yocto-patches] " Paul Barker
2026-07-01  7:40 ` [wic][PATCH v3 02/10] tests: add a session banner via conftest.py Trevor Woerner
2026-07-06  8:17   ` [yocto-patches] " Paul Barker
2026-07-01  7:40 ` [wic][PATCH v3 03/10] tests: add the run-tests.sh wrapper Trevor Woerner
2026-07-06  8:21   ` [yocto-patches] " Paul Barker
2026-07-01  7:40 ` [wic][PATCH v3 04/10] tests: add optional coverage reporting to run-tests.sh Trevor Woerner
2026-07-06  8:33   ` [yocto-patches] " Paul Barker
2026-07-01  7:40 ` [wic][PATCH v3 05/10] tests: add ruff linting " Trevor Woerner
2026-07-06  8:40   ` [yocto-patches] " Paul Barker
2026-07-01  7:40 ` [wic][PATCH v3 06/10] tests/docs: add the suite overview README Trevor Woerner
2026-07-06  8:47   ` [yocto-patches] " Paul Barker
2026-07-01  7:40 ` [wic][PATCH v3 07/10] tests/docs: add the test-authoring guide Trevor Woerner
2026-07-06  8:52   ` [yocto-patches] " Paul Barker
2026-07-01  7:40 ` [wic][PATCH v3 08/10] tests: ignore E402 in the test tree for the sys.path bootstrap Trevor Woerner
2026-07-06  8:55   ` [yocto-patches] " Paul Barker
2026-07-01  7:40 ` [wic][PATCH v3 09/10] tests/unit/test_bb_utils: test mkdirhier() and fix its missing errno import Trevor Woerner
2026-07-06  9:06   ` [yocto-patches] " Paul Barker
2026-07-01  7:40 ` [wic][PATCH v3 10/10] tests/docs: add the review rubric Trevor Woerner
2026-07-06  9:08   ` [yocto-patches] " Paul Barker

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.