diff --git a/CLAUDE.md b/CLAUDE.md index a4392e1..c7e20e1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -355,28 +355,52 @@ Source layout (all under `src/`): right after the `stop_tap_relay()` loop) for `--clean-processes`'s own crash-orphan sweep (`network_tap_relay.h`'s `clean_stale_tap_relays()`, see below). -- `self_test.{h,cpp}` — `run_self_tests(args)` implements `-t/--test` - (`args` is `ParsedArgs::test_args`, `cli_args.h` — everything on the - command line after `-t`, captured the same trailing-argv way `-r`/`-x` - capture their own command). Purely plumbing: builds a synthetic argv - (`{"slocker-lite -t"} + args`) and hands it straight to Catch2's - `Catch::Session().run(argc, argv)` — no test logic of its own lives here - at all, that's all under `tests/` (see below); this file is `#if +- `self_test.{h,cpp}` — `run_self_tests(args, config)` implements + `-t/--test` (`args` is `ParsedArgs::test_args`, `cli_args.h` — everything + on the command line after `-t`, captured the same trailing-argv way + `-r`/`-x` capture their own command; `config` is the same effective, + `-c/--config-file`-resolved `AppConfig` any other command gets, threaded + through from `dispatch_command()`'s own `Mode::test` case). Mostly + plumbing: builds a synthetic argv (`{"slocker-lite -t"} + args`) and + hands it straight to Catch2's `Catch::Session().run(argc, argv)` — no + test logic of its own lives here at all, that's all under `tests/` (see + below) — but first stashes `config` into `tests/support/fixtures.h`'s + own `g_test_app_config` global, *before* Catch2 ever runs a single + `TEST_CASE`. **Why**: without this, test code that creates a real + container (`test_rootless_run.cpp`'s own `run_in_fixture()`, below) had + no way to reflect `-c`'s settings at all — it always built a fresh, + hardcoded default `AppConfig{}` for every container regardless of what + `-c`/the real config file said, since Catch2 `TEST_CASE`s are just plain + functions with no way to receive parameters from the harness that invoked + them. Confirmed by direct testing (per the user's own request): a + hand-written config with every `unshare-*`/`with-*` key set to `false`, + used via `-c -t -- "[integration][net]~[root]"`, correctly + flips all 3 of `test_rootless_run.cpp`'s container-creating tests to + failing (the two namespace-isolation checks, since nothing's actually + isolated anymore, and the nohup-straggler regression test, since with no + pid namespace *and* no cgroup — rootless, on this dev machine — nothing + reaps the backgrounded process) while leaving every other category + (`[unit]`, `[integration]~[net]`) completely unaffected, exactly as + expected. This file is `#if ENABLE_TESTS`-guarded (`config.h`, from Meson's `enable_tests` option, default on — the same macro that already gated whether `catch2_dep` gets linked at all) so a `-Denable_tests=false` build prints a clear "not compiled into this build" message and returns nonzero instead of - failing to link. `-t`'s own leftover-args capture requires a literal - `--` before any Catch2 option that looks like one of slocker-lite's own - (`-r/--reporter` collides with `-r/--run` — `-c/--section` no longer - collides with anything now that `--cleanup` dropped its own `-c` short - form) — a bare tag expression like `-t -- "[unit]"` needs it - too by convention, though `getopt_long`'s own permutation happens to let - a `-t "[unit]"` without `--` work anyway, since `"[unit]"` doesn't start - with `-`. Distinct from the Meson-driven fixture smoke test under - `tests/` (`tests/gen_fixture.py`/`tests/run_test.py`, described in - "Build & test commands" below), which stays a separate, always-on, - Python-driven mount/unmount/cleanup check. + failing to link — the `#include "fixtures.h"`/`g_test_app_config` usage + is confined to the `#if ENABLE_TESTS` branch specifically because + `tests/support/fixtures.cpp` (where it's defined) is itself only + compiled into the binary when `enable_tests` is on (`meson.build`'s + `test_sources`), so referencing it unconditionally would break that + build with a link error. `-t`'s own leftover-args capture requires a + literal `--` before any Catch2 option that looks like one of + slocker-lite's own (`-r/--reporter` collides with `-r/--run`; + `-c/--section` again collides too, now with `-c/--config-file`) — a bare + tag expression like `-t -- "[unit]"` needs it too by convention, though + `getopt_long`'s own permutation happens to let a `-t "[unit]"` without + `--` work anyway, since `"[unit]"` doesn't start with `-`. Distinct from + the Meson-driven fixture smoke test under `tests/` (`tests/gen_fixture.py`/ + `tests/run_test.py`, described in "Build & test commands" below), which + stays a separate, always-on, Python-driven mount/unmount/cleanup check. **Test organization under `tests/`** (all in-process — every category calls this project's own already-header-exposed functions directly, no @@ -500,7 +524,19 @@ Source layout (all under `src/`): expected shutdown `SIGTERM` via the inherited handler and report a spurious failure) — fixed the same way, resetting `SIGTERM` to `SIG_DFL` right before forking the straggler. - - `tests/support/fixtures.{h,cpp}` — `find_busybox_fixture()` (searches + - `tests/support/fixtures.{h,cpp}` — `g_test_app_config` (a plain `AppConfig` + global, default-constructed) is set once by `run_self_tests()` + (`self_test.cpp`, see above) from the current `-t` run's own effective, + possibly `-c/--config-file`-resolved config, before Catch2 runs anything; + `test_rootless_run.cpp`'s own `run_in_fixture()` reads a copy of it for + every container it creates instead of a hardcoded default, so `-c` + actually reaches that test's own namespace-policy resolution. Declared + here (not `self_test.{h,cpp}`) specifically because `self_test.cpp` is + always compiled (test or not), while this file — and thus this + global's actual definition — only exists in the binary at all when + `enable_tests` is on; `self_test.cpp`'s own `#include "fixtures.h"`/ + write to it is confined to its `#if ENABLE_TESTS` branch for exactly + that reason. `find_busybox_fixture()` (searches `images/busybox.tar` relative to cwd, this project's own established manual-testing convention; `nullopt` if absent, so `[net]` tests `SKIP()` rather than fail — see `tests/setup-tests.py`, below), diff --git a/src/commands.cpp b/src/commands.cpp index f4e4c55..09e3e17 100644 --- a/src/commands.cpp +++ b/src/commands.cpp @@ -889,7 +889,7 @@ int dispatch_command(const ParsedArgs& args, const std::filesystem::path& config case Mode::clean_processes: return clean_processes_command(); case Mode::test: - return run_self_tests(args.test_args); + return run_self_tests(args.test_args, config); case Mode::exec: return exec_in_session(*args.exec_pid, args.command, args.user_flag, args.group_flag); case Mode::kill: diff --git a/src/self_test.cpp b/src/self_test.cpp index d61ebc1..f662ae8 100644 --- a/src/self_test.cpp +++ b/src/self_test.cpp @@ -20,12 +20,21 @@ #if ENABLE_TESTS #include + +#include "fixtures.h" #else #include #endif -int run_self_tests(const std::vector& args) { +int run_self_tests(const std::vector& args, const AppConfig& config) { #if ENABLE_TESTS + // Stashed into a shared test-support global (tests/support/fixtures.h) + // *before* Catch2 ever runs a TEST_CASE, so test code that creates a + // real container (test_rootless_run.cpp's own run_in_fixture()) can + // consult the same effective, -c/--config-file-resolved settings a + // real -r/--run invocation would, instead of hardcoded defaults. + g_test_app_config = config; + // args[0] is conventionally the program name in Catch2's eyes (used in // some of its own diagnostic/help output) -- it never sees the real // argv[0], since ParsedArgs::test_args only ever holds what came after @@ -45,6 +54,7 @@ int run_self_tests(const std::vector& args) { return session.run(static_cast(argv.size()), argv.data()); #else (void)args; + (void)config; fmt::print(stderr, "tests were not compiled into this build (reconfigure with " "-Denable_tests=true and rebuild)\n"); diff --git a/src/self_test.h b/src/self_test.h index b254505..2cdc1c9 100644 --- a/src/self_test.h +++ b/src/self_test.h @@ -19,16 +19,26 @@ #include #include +#include "config_file.h" + // Implements -t/--test, this project's own built-in Catch2-driven test // suite (distinct from the Meson-driven fixture smoke test under tests/, // which stays a separate, always-on mount/unmount/cleanup check). // `args` is `ParsedArgs::test_args` (cli_args.h) -- everything on the // command line after `-t`, forwarded to Catch2's own Session::run() // unmodified (tag expressions, --list-tests, --reporter, etc.); a bare -// `-t` (empty `args`) runs every registered TEST_CASE. Only compiled to +// `-t` (empty `args`) runs every registered TEST_CASE. `config` is the +// same effective (`-c/--config-file`-resolved) AppConfig any other command +// gets, given to test code so a test that creates a real container can +// reflect the same global settings a real `-r/--run` would (see +// `tests/support/fixtures.h`'s `g_test_app_config`, which this stashes it +// into) -- letting e.g. `-c -t -- "[integration][net]"` +// exercise what happens with pid/cgroup isolation actually disabled, +// instead of every test always creating containers under hardcoded +// defaults regardless of `-c`. Only compiled to // actually run Catch2 when this build has ENABLE_TESTS set (config.h, // from Meson's `enable_tests` option, default on) -- otherwise prints a // clear "not compiled into this build" message and returns nonzero, // since a -Denable_tests=false build has no TEST_CASEs (or Catch2 itself) // linked in at all. -int run_self_tests(const std::vector& args); +int run_self_tests(const std::vector& args, const AppConfig& config); diff --git a/tests/integration/test_rootless_run.cpp b/tests/integration/test_rootless_run.cpp index 992dbf4..6abd069 100644 --- a/tests/integration/test_rootless_run.cpp +++ b/tests/integration/test_rootless_run.cpp @@ -125,7 +125,12 @@ std::string run_in_fixture(const std::filesystem::path& image, const std::vector args.mode_arg = image.string(); args.command = command; - AppConfig config; + // A fresh copy of the current -t run's effective config (g_test_app_config, + // fixtures.h -- set once by run_self_tests() from whatever main() resolved, + // respecting -c/--config-file) rather than a hardcoded AppConfig{}, so this + // helper's containers reflect the same global settings a real -r/--run + // invocation would. + AppConfig config = g_test_app_config; CapturedStdout capture; // config_path is only ever consulted by modes that read/write a config // file (-w/--write-config, -v/--volume, -n/--network); Mode::run diff --git a/tests/support/fixtures.cpp b/tests/support/fixtures.cpp index 7dde570..95718e2 100644 --- a/tests/support/fixtures.cpp +++ b/tests/support/fixtures.cpp @@ -26,6 +26,8 @@ #include #include +AppConfig g_test_app_config; + std::optional find_busybox_fixture() { std::error_code ec; auto path = std::filesystem::current_path(ec) / "images" / "busybox.tar"; diff --git a/tests/support/fixtures.h b/tests/support/fixtures.h index 86a70ec..1d342b2 100644 --- a/tests/support/fixtures.h +++ b/tests/support/fixtures.h @@ -20,6 +20,20 @@ #include #include +#include "config_file.h" + +// The effective AppConfig for the current -t/--test run -- set once by +// run_self_tests() (self_test.cpp) from whatever main() resolved (respecting +// -c/--config-file, same as any other command), before Catch2 ever runs a +// TEST_CASE. Test code that creates a real container (test_rootless_run.cpp's +// own run_in_fixture()) reads this instead of hardcoding a fresh, default +// AppConfig{}, so e.g. `-c -t -- "[integration][net]"` +// actually exercises what happens with pid/cgroup isolation disabled. Plain +// default-constructed AppConfig{} otherwise (matching every test's prior, +// unconditional behavior) -- a bare `-t` with no `-c` and no real config file +// changes nothing. +extern AppConfig g_test_app_config; + // Path to a real, runnable OCI Image Layout tar (something with an actual // /bin/sh, unlike tests/gen_fixture.py's minimal single-file fixture used // by the plain mount/unmount smoke test) for [integration][net] tests that