diff mbox series

[wic,v2,9/9] tests/unit/test_bb_utils: test mkdirhier() and fix its missing errno import

Message ID 20260630160612.1005451-10-twoerner@gmail.com
State New
Headers show
Series tests: standalone test-suite framework plus the first unit test | expand

Commit Message

Trevor Woerner June 30, 2026, 4:06 p.m. UTC
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 mbox series

Patch

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