Skip to content

Improve test harness code introduced during sidecar migration - #283

Merged
jserv merged 5 commits into
sysprog21:mainfrom
henrybear327:sysroot/test-harness
Aug 12, 2026
Merged

Improve test harness code introduced during sidecar migration#283
jserv merged 5 commits into
sysprog21:mainfrom
henrybear327:sysroot/test-harness

Conversation

@henrybear327

@henrybear327 henrybear327 commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Follow up improvement effort to improve code introduce in #256

Mainly focusing on improving the test harness makefile, and refactoring the helper functions.


Summary by cubic

Modernizes the test harness by generating check lanes with canned make rules, centralizing shared helpers, and adding header dependency tracking for guest tests. This reduces duplication, fixes rebuild correctness, and removes stale sidecar references.

  • Refactors

    • Generate check and sanitizer lanes via canned recipes in mk/tests.mk (shared scratch setup, absock leak check, banners preserved; single '+' run line so make -n expands the lane).
    • Track shared header deps for cross-compiled guest tests via CROSS_TEST_CFLAGS (-MMD/-MP; driver injects -MQ for the -o target).
    • Consolidate helpers into tests/test-util.h and tests/host-test-util.h (dirfd file I/O, dir scans, UNIX socket helpers with cleanup on failure, host pass/fail summary; unify openat2 ABI via linux-openat2.h); update tests to use them.
    • Fold the no-sysroot variant into test-sysroot-outside-names via a mode arg; remove tests/test-nosysroot-literal-names.c.
    • Remove stale sidecar mentions from docs and test headers; clarify resolver behavior (clamp '..' only at guest root; case-insensitive sysroot may store escaped leaves).
  • Bug Fixes

    • Bounds-check the stored-name probe reply and zero the buffer; accept SMB/NFS attr layouts; mirror the check in host-only lanes.

Written for commit 83aa927. Summary will update on new commits.

Review in cubic

@henrybear327 henrybear327 self-assigned this Aug 11, 2026
@henrybear327
henrybear327 requested a review from jserv August 11, 2026 12:53
cubic-dev-ai[bot]

This comment was marked as resolved.

jserv

This comment was marked as resolved.

@henrybear327
henrybear327 marked this pull request as draft August 11, 2026 15:41
@henrybear327
henrybear327 marked this pull request as ready for review August 11, 2026 15:42
@henrybear327
henrybear327 requested a review from jserv August 11, 2026 15:42

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 19 files

Re-trigger cubic

src/syscall/sidecar.c was deleted in the casefold redesign, but three
pieces of prose still cite it: an internals paragraph explaining the
climbed-create rule by comparison to sidecar_walk_parent_at(), and two
test headers describing sidecar collapsing and sidecar-mangled names.

The paragraph drops the comparison, and each test header states the
behavior that actually holds: the resolvers clamp '..' only at the guest
root, and a case-insensitive sysroot may store a leaf under its escaped
spelling. A citation that resolves to no file misleads the reader who
tries to follow it.
The guest cross rules compiled and linked in one step with no
dependency output, so editing a shared test header rebuilt nothing and
every guest binary kept running its stale code; host tests were covered
by DEPFLAGS all along. One shared flag spelling now emits a .d beside
each binary, which mk/common.mk already includes. In a compile-and-link
step the gcc driver injects -MQ with the -o argument, so -MMD alone
names the binary as the rule target and writes the .d next to it.

The dynamic-glibc bench keeps its own line: it includes no shared
header, and the shared spelling's -static would defeat the dynamic link
its measurement exists to exercise.
The stored-name probe used getattrlistat's reply without validating it.
Every field it consumed comes from the filesystem, and a sysroot can
name an SMB or NFS mount, which need not fill attr_dataoffset and
attr_length the way APFS does: a reply too short for the reference, a
negative offset, a length past what the volume wrote, or a name with no
terminating NUL each made the "%s" print read bytes the kernel never
set. Zeroing the buffer bounds what a partial reply can print.

The check mirrors casefold_attr_stored_name in casefold-walk.c rather
than calling it: of the three lanes including this header, only
test-casefold-walk-host links casefold-walk.o.
The suite carried the same code several times over: five private
helpers re-implementing test-util.h functions their own files already
include, the openat2 ABI declared in two headers that each claimed to
be its only home, a pathname-socket bind helper under two names and a
third spelling without the listen, a pass/fail harness copied into
three host binaries, and a whole lane that was another lane's
assertions with different banner strings.

Each moves to the header that already serves its siblings, keeping
every assertion and every matrix column; the duplicated lane becomes a
mode argument to the binary it was copied from. test-util.h gains the
dirfd-relative siblings the name-relative lane was hand-rolling.

Two details of the shared spellings are deliberate: unix_bind unlinks
the socket file when listen fails, which no copy did, so a rerun does
not inherit EADDRINUSE from a bind the failed call left behind; and the
scratch-root helper takes the lane name as an argument, keeping the
failure line each copy printed under its own name, the only name a CI
log can trace back to a lane.
check and check-sanitizer spelled forty-six lanes as banner-plus-invoke
pairs by hand, thirteen of them twice, and seventeen lane recipes
opened with the same three lines of scratch-directory setup. Two canned
recipes now carry each banner beside its invoke, and three fragments
carry the shared setup and the absock leak check, so a lane-list edit
cannot leave a banner naming the wrong lane. Order and banner text are
preserved verbatim; recipes that clean up more than the scratch
directory replace the trap once the extra paths are named.

The lane recipe is a single line marked '+'. Make decides what still
runs under -n by looking for a literal $(MAKE) in the unexpanded recipe
line, which a canned recipe hides, so without the marker a dry run
prints the sub-make instead of expanding the lane behind it. One line
rather than a banner-then-invoke pair because once a '+' line appears
in an expansion, every later line of that same expansion also runs for
real under -n; split banners would behave unlike each other, and a
non-lane line appended after a run-lane call would execute during a dry
build.
@jserv
jserv merged commit 7b51ea7 into sysprog21:main Aug 12, 2026
21 checks passed
@henrybear327
henrybear327 deleted the sysroot/test-harness branch August 12, 2026 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants