Patches for Yocto layers and components that do not have their own list
 help / color / mirror / Atom feed
From: Trevor Woerner <twoerner@gmail.com>
To: yocto-patches@lists.yoctoproject.org
Subject: [wic][PATCH v2 9/9] tests/unit/test_bb_utils: test mkdirhier() and fix its missing errno import
Date: Tue, 30 Jun 2026 12:06:12 -0400	[thread overview]
Message-ID: <20260630160612.1005451-10-twoerner@gmail.com> (raw)
In-Reply-To: <20260630160612.1005451-1-twoerner@gmail.com>

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.7 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
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 | 126 ++++++++++++++++++++++++++++++++++++
 2 files changed, 127 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..5cd6035ca453
--- /dev/null
+++ b/tests/unit/test_bb_utils.py
@@ -0,0 +1,126 @@
+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 tempfile
+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):
+        d = tempfile.mkdtemp()
+        target = os.path.join(d, "a", "b", "c")
+        mkdirhier(target)
+        assert os.path.isdir(target)
+
+    def test_idempotent_on_existing_directory(self):
+        d = tempfile.mkdtemp()
+        target = os.path.join(d, "x")
+        mkdirhier(target)
+        # Second call must not raise (exist_ok=True).
+        mkdirhier(target)
+        assert os.path.isdir(target)
+
+    def test_single_level(self):
+        d = tempfile.mkdtemp()
+        target = os.path.join(d, "single")
+        mkdirhier(target)
+        assert os.path.isdir(target)
+
+    def test_existing_root_directory(self):
+        # An already-existing directory (the tmpdir itself) is a no-op.
+        d = tempfile.mkdtemp()
+        mkdirhier(d)
+        assert os.path.isdir(d)
+
+
+# ---------------------------------------------------------------------------
+# 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):
+        d = tempfile.mkdtemp()
+        bad = os.path.join(d, "${VAR}", "sub")
+        with pytest.raises(Exception):
+            mkdirhier(bad)
+        # Nothing should have been created.
+        assert not os.path.exists(os.path.join(d, "${VAR}"))
+
+    def test_brace_without_dollar_is_allowed(self):
+        # Only the literal '${' marker triggers the guard; a bare '{' is a
+        # legal (if unusual) directory name.
+        d = tempfile.mkdtemp()
+        target = os.path.join(d, "plain{brace")
+        mkdirhier(target)
+        assert os.path.isdir(target)
+
+
+# ---------------------------------------------------------------------------
+# OSError handler
+# ---------------------------------------------------------------------------
+
+class TestMkdirhierErrorPath:
+    def test_oserror_under_a_file_component(self):
+        # 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.
+        d = tempfile.mkdtemp()
+        f = os.path.join(d, "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):
+        # An EEXIST OSError on a path that is already a directory is the
+        # benign race the handler exists to absorb; it must not propagate.
+        d = tempfile.mkdtemp()
+        err = OSError()
+        err.errno = errno.EEXIST
+        with mock.patch("wic.bb.utils.os.makedirs", side_effect=err):
+            mkdirhier(d)  # d exists and is a dir -> swallowed, no raise
+
+    def test_eexist_on_existing_file_propagates(self):
+        # EEXIST but the path is a regular file, not a directory: the
+        # handler's isdir() re-check fails, so the error must propagate.
+        d = tempfile.mkdtemp()
+        f = os.path.join(d, "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



      parent reply	other threads:[~2026-06-30 16:07 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-30 16:06 [wic][PATCH v2 0/9] tests: standalone test-suite framework plus the first unit test Trevor Woerner
2026-06-30 16:06 ` [wic][PATCH v2 1/9] tests: add the standalone test-suite skeleton Trevor Woerner
2026-06-30 16:06 ` [wic][PATCH v2 2/9] tests: add a session banner via conftest.py Trevor Woerner
2026-06-30 16:06 ` [wic][PATCH v2 3/9] tests: add the run-tests.sh wrapper Trevor Woerner
2026-06-30 16:06 ` [wic][PATCH v2 4/9] tests: add optional coverage reporting to run-tests.sh Trevor Woerner
2026-06-30 16:06 ` [wic][PATCH v2 5/9] tests: add ruff linting " Trevor Woerner
2026-06-30 16:06 ` [wic][PATCH v2 6/9] tests/docs: add the suite overview README Trevor Woerner
2026-06-30 16:06 ` [wic][PATCH v2 7/9] tests/docs: add the test-authoring guide Trevor Woerner
2026-06-30 16:06 ` [wic][PATCH v2 8/9] tests: ignore E402 in the test tree for the sys.path bootstrap Trevor Woerner
2026-06-30 16:06 ` Trevor Woerner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260630160612.1005451-10-twoerner@gmail.com \
    --to=twoerner@gmail.com \
    --cc=yocto-patches@lists.yoctoproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox