test(cli): no timeout decides a cli_scope test; finish keeps the environment

Follow-up to #2589.

No timeout decides a cli_scope test any more:
- The status probe and the parent's client connect repeat an attempt that
  reaches a live but mute endpoint (a loaded runner answering slowly). A
  reply or a definite refusal decides. Each attempt keeps the API's finite
  5 s budget. This replaces the 15 s status budget and the single 5 s
  connect.
- The host child is reaped without a deadline (it used to get SIGKILL after
  60 s).
- The host child joins its version cohort with UINT64_MAX, which waits for
  locks but answers a conflicting holder at once (it used to have a 45 s
  deadline).
A wedged child is left to the harness's per-suite ceiling.

cli_scope_fixture_finish restored HOME, CBM_CACHE_DIR and SHELL even when
start had failed before saving them, which unset them for every later test
in the suite. It now restores only what start saved.

New tests:
- cli_scope_fixture_finish_keeps_env_after_an_early_failure fails on main
  at ASSERT(home_kept);
- cli_scope_retries_a_mute_endpoint_until_it_answers checks the retry rule
  on scripted attempt outcomes.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel
2026-10-11 13:16:44 +02:00
parent f216a10c7c
commit bf6063663f
+173 -47
View File
@@ -1572,15 +1572,19 @@ TEST(cli_install_recovers_markerless_stale_rendezvous) {
* install under test and asks the daemon itself afterwards. */
#define CLI_SCOPE_HOST_DRAINED 3
#define CLI_SCOPE_TIMEOUT_MS 5000U
/* The host child's readiness has NO deadline (USER RULE: no timeout decides a
* test). The child answers 'R' or 'E', or the pipe reaches EOF when it dies,
* and the fixture names the failing step in every case. A child that wedges
* before answering is left to the harness's per-suite ceiling. A 90 s bound
* used to decide this wait, and on a starved machine (2026-10-09, during a
* kernel index) ASSERT(ready) failed with nothing in the log to say why.
* Generous, bounded waits remain for reaping and status probes below. */
#define CLI_SCOPE_HOST_REAP_TIMEOUT_MS 60000U
#define CLI_SCOPE_HOST_SERVING_TIMEOUT_MS 15000U
/* No timeout decides a cli_scope test (USER RULE). That covers the host child's
* readiness, its cohort join, the parent's client connect, the "still serving"
* probe, and the reap:
* - the child answers 'R' or 'E', or the pipe reaches EOF when it dies, and the
* fixture names the failing step in every case;
* - each IPC attempt keeps the API's finite CLI_SCOPE_TIMEOUT_MS budget, but an
* attempt that reaches a live, mute endpoint (a loaded runner answering
* slowly) is repeated, and only a reply or a definite refusal decides
* (cli_scope_until_answered);
* - a child that wedges is left to the harness's per-suite ceiling.
* A 90 s readiness bound, a 15 s status budget and a 60 s reap used to decide
* these; on a starved machine (2026-10-09, during a kernel index)
* ASSERT(ready) failed with nothing in the log to say why. */
/* Teardown budget for a child the activation ALREADY drained: its service is
* gone, so each free/release succeeds immediately and this is only the
* give-up point if one unexpectedly does not. The undrained path keeps the
@@ -1588,9 +1592,10 @@ TEST(cli_install_recovers_markerless_stale_rendezvous) {
#define CLI_SCOPE_CLEANUP_DRAINED_MS 1000U
/* Budget for probing that an ALREADY-drained host has stopped serving.
*
* The generous CLI_SCOPE_HOST_SERVING_TIMEOUT_MS exists for the POSITIVE
* question -- "is this daemon still up?" -- where a slow reply on a loaded
* runner must not be misread as drained. Asked in the negative it inverts:
* The positive question -- "is this daemon still up?" -- repeats a mute
* attempt until the daemon answers (cli_scope_host_serving), because a slow
* reply on a loaded runner must not be misread as drained. Asked in the
* negative it inverts:
* there is no reply coming, so the whole budget is spent proving silence, and
* the assertion is decided by a timeout expiring rather than by the system's
* own behaviour. That was 15 s of the drain test's wall clock and the largest
@@ -1620,6 +1625,7 @@ typedef struct {
char *old_home;
char *old_cache;
char *old_shell;
bool env_saved; /* HOME, CBM_CACHE_DIR and SHELL were saved for finish */
char failed_step[160]; /* the setup step that failed, "" when ready */
} cli_scope_fixture_t;
@@ -1634,8 +1640,10 @@ static _Noreturn void cli_scope_host_child(const cli_scope_fixture_t *fixture, i
cbm_daemon_runtime_service_t *service = NULL;
int acquire_status = -1;
if (endpoint && manager) {
acquire_status = (int)cbm_version_cohort_acquire(manager, &fixture->identity,
cbm_now_ms() + 45000U, &lease, &conflict);
/* UINT64_MAX: wait as long as a lock is held, never for a conflicting
* holder (that is answered at once), so no deadline decides the join. */
acquire_status = (int)cbm_version_cohort_acquire(manager, &fixture->identity, UINT64_MAX,
&lease, &conflict);
}
bool admitted = acquire_status == (int)CBM_VERSION_COHORT_OK;
if (admitted) {
@@ -1745,32 +1753,60 @@ static bool cli_scope_setup_failed(cli_scope_fixture_t *fixture, const char *tag
return false;
}
/* Reap the host child within a bound: the release signal makes a healthy child
* break within ~50 ms and finish teardown in a few seconds, so a child still
* alive past the deadline is wedged — SIGKILL it and reap so the suite always
* makes progress. Returns the child's exit code, or -1 when it had to be
* killed or did not exit cleanly; a -1 fails the caller's host_exit assertion
* cleanly rather than hanging. */
static int cli_scope_reap_host(pid_t host, uint32_t timeout_ms) {
uint64_t deadline = cbm_now_ms() + timeout_ms;
/* Reap the host child, with no deadline: the release signal makes a healthy
* child exit within seconds, and a wedged one is left to the harness's
* per-suite ceiling (see CLI_SCOPE_TIMEOUT_MS). Returns the child's exit code,
* or -1 when it did not exit normally. */
static int cli_scope_reap_host(pid_t host) {
int status = 0;
for (;;) {
pid_t reaped = waitpid(host, &status, WNOHANG);
pid_t reaped = waitpid(host, &status, 0);
if (reaped == host) {
return WIFEXITED(status) ? WEXITSTATUS(status) : -1;
}
if (reaped < 0 && errno != EINTR) {
return -1;
}
if (cbm_now_ms() >= deadline) {
(void)kill(host, SIGKILL);
(void)waitpid(host, &status, 0);
return -1;
}
cbm_usleep(2000);
}
}
/* One IPC attempt's outcome for cli_scope_until_answered. */
typedef enum {
CLI_SCOPE_ANSWERED, /* a reply arrived */
CLI_SCOPE_REFUSED, /* definite: no endpoint, a refusal, or a conflict */
CLI_SCOPE_MUTE, /* a live process holds the endpoint but did not answer in time */
} cli_scope_attempt_t;
typedef cli_scope_attempt_t (*cli_scope_attempt_fn)(void *context);
/* Repeat an IPC attempt while it reaches a live but mute endpoint, so a slow
* reply on a loaded runner is waited for instead of failing the test. A reply
* or a definite refusal ends it. Each attempt keeps the API's finite budget.
* Returns true when an attempt was answered. */
static bool cli_scope_until_answered(cli_scope_attempt_fn attempt, void *context) {
for (;;) {
cli_scope_attempt_t outcome = attempt(context);
if (outcome != CLI_SCOPE_MUTE) {
return outcome == CLI_SCOPE_ANSWERED;
}
}
}
/* One attempt to connect the parent's committed client to the host. */
static cli_scope_attempt_t cli_scope_connect_attempt(void *context) {
cli_scope_fixture_t *fixture = context;
cbm_daemon_runtime_connect_result_t result = {0};
fixture->client = cbm_daemon_runtime_client_connect(fixture->endpoint, &fixture->identity,
CLI_SCOPE_TIMEOUT_MS, &result);
if (fixture->client) {
return CLI_SCOPE_ANSWERED;
}
return result.status == CBM_DAEMON_RUNTIME_CONNECT_ERROR &&
result.muted_endpoint_holder_pid != 0
? CLI_SCOPE_MUTE
: CLI_SCOPE_REFUSED;
}
static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *tag) {
memset(fixture, 0, sizeof(*fixture));
fixture->host = -1;
@@ -1842,6 +1878,7 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta
cli_activation_save_env(&fixture->old_home, &fixture->old_cache);
const char *shell = getenv("SHELL");
fixture->old_shell = shell ? strdup(shell) : NULL;
fixture->env_saved = true;
cbm_setenv("SHELL", "/bin/zsh", 1);
if (child < 0) {
return cli_scope_setup_failed(fixture, tag, "forking the host", fork_error);
@@ -1854,11 +1891,9 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta
0);
}
fixture->endpoint = cbm_daemon_bootstrap_endpoint_new(fixture->runtime_parent);
cbm_daemon_runtime_connect_result_t connect_result = {0};
fixture->client = fixture->endpoint
? cbm_daemon_runtime_client_connect(fixture->endpoint, &fixture->identity,
CLI_SCOPE_TIMEOUT_MS, &connect_result)
: NULL;
if (fixture->endpoint) {
(void)cli_scope_until_answered(cli_scope_connect_attempt, fixture);
}
if (!fixture->client) {
return cli_scope_setup_failed(fixture, tag,
fixture->endpoint ? "connecting the parent's client"
@@ -1876,15 +1911,33 @@ static bool cli_scope_host_serving_within(const cli_scope_fixture_t *fixture, ui
!status.stopping && status.committed_clients == 1;
}
typedef struct {
const cli_scope_fixture_t *fixture;
cbm_daemon_runtime_status_t status;
} cli_scope_status_probe_t;
/* One status request. A failed request still reports the PID of a live but
* mute endpoint holder, which is what tells slow apart from gone. */
static cli_scope_attempt_t cli_scope_status_attempt(void *context) {
cli_scope_status_probe_t *probe = context;
memset(&probe->status, 0, sizeof(probe->status));
if (cbm_daemon_runtime_request_status(probe->fixture->endpoint, &probe->fixture->identity,
CLI_SCOPE_TIMEOUT_MS, &probe->status)) {
return CLI_SCOPE_ANSWERED;
}
return probe->status.muted_endpoint_holder_pid != 0 ? CLI_SCOPE_MUTE : CLI_SCOPE_REFUSED;
}
/* Ask the host daemon itself: still running, not stopping, and the parent's
* committed client still admitted. */
* committed client still admitted. A foreign-namespace install leaves this
* daemon serving, so a slow reply on a loaded runner must not be misread as
* "drained" (the flaky failure this fixture showed): a mute attempt is
* repeated until the daemon answers, while a drained daemon is unreachable or
* reports stopping, which decides at once. */
static bool cli_scope_host_serving(const cli_scope_fixture_t *fixture) {
/* A generous, bounded status deadline: a foreign-namespace install leaves
* this daemon serving, so a slow response on a loaded runner must not be
* misread as "drained" (the flaky failure this fixture showed). The call
* still fails cleanly — a genuinely drained daemon is unreachable or
* reports stopping — it just no longer decides survival on a 5 s budget. */
return cli_scope_host_serving_within(fixture, CLI_SCOPE_HOST_SERVING_TIMEOUT_MS);
cli_scope_status_probe_t probe = {.fixture = fixture};
return fixture->endpoint && cli_scope_until_answered(cli_scope_status_attempt, &probe) &&
!probe.status.stopping && probe.status.committed_clients == 1;
}
static int cli_scope_install(cli_scope_fixture_t *fixture, const char *home, const char *cache,
@@ -1917,7 +1970,7 @@ static int cli_scope_fixture_finish(cli_scope_fixture_t *fixture) {
}
int host_exit = -1;
if (fixture->host > 0) {
host_exit = cli_scope_reap_host(fixture->host, CLI_SCOPE_HOST_REAP_TIMEOUT_MS);
host_exit = cli_scope_reap_host(fixture->host);
fixture->host = -1;
}
cbm_daemon_ipc_endpoint_free(fixture->endpoint);
@@ -1925,14 +1978,20 @@ static int cli_scope_fixture_finish(cli_scope_fixture_t *fixture) {
if (fixture->previous_supervisor_build[0]) {
cbm_index_supervisor_set_build_fingerprint_for_test(fixture->previous_supervisor_build);
}
if (fixture->old_shell) {
cbm_setenv("SHELL", fixture->old_shell, 1);
} else {
cbm_unsetenv("SHELL");
/* Restore only what start saved: a setup that failed before saving left
* HOME, CBM_CACHE_DIR and SHELL untouched, and restoring "nothing saved"
* would unset them for every later test in the suite. */
if (fixture->env_saved) {
if (fixture->old_shell) {
cbm_setenv("SHELL", fixture->old_shell, 1);
} else {
cbm_unsetenv("SHELL");
}
cli_activation_restore_env(fixture->old_home, fixture->old_cache);
fixture->env_saved = false;
}
free(fixture->old_shell);
fixture->old_shell = NULL;
cli_activation_restore_env(fixture->old_home, fixture->old_cache);
fixture->old_home = NULL;
fixture->old_cache = NULL;
test_rmdir_r(fixture->tmpdir);
@@ -2080,6 +2139,71 @@ TEST(cli_scope_fixture_names_its_failing_setup_step) {
PASS();
}
/* Every cli_scope test calls finish even when start failed. A start that
* failed before saving HOME, CBM_CACHE_DIR and SHELL used to have finish
* "restore" them to nothing, unsetting them for every later test. */
TEST(cli_scope_fixture_finish_keeps_env_after_an_early_failure) {
char *saved_home = save_test_env("HOME");
char *saved_cache = save_test_env("CBM_CACHE_DIR");
char *saved_shell = save_test_env("SHELL");
cbm_setenv("HOME", "/tmp/cli-scope-sentinel-home", 1);
cbm_setenv("CBM_CACHE_DIR", "/tmp/cli-scope-sentinel-cache", 1);
cbm_setenv("SHELL", "/bin/sentinel-shell", 1);
cli_scope_fixture_t fixture;
bool ready = cli_scope_fixture_start(&fixture, "no-such-parent/y");
(void)cli_scope_fixture_finish(&fixture);
const char *home = getenv("HOME");
const char *cache = getenv("CBM_CACHE_DIR");
const char *shell = getenv("SHELL");
bool home_kept = home && strcmp(home, "/tmp/cli-scope-sentinel-home") == 0;
bool cache_kept = cache && strcmp(cache, "/tmp/cli-scope-sentinel-cache") == 0;
bool shell_kept = shell && strcmp(shell, "/bin/sentinel-shell") == 0;
restore_test_env("HOME", saved_home);
restore_test_env("CBM_CACHE_DIR", saved_cache);
restore_test_env("SHELL", saved_shell);
ASSERT_FALSE(ready);
ASSERT_TRUE(home_kept);
ASSERT_TRUE(cache_kept);
ASSERT_TRUE(shell_kept);
PASS();
}
/* A scripted sequence of attempt outcomes for cli_scope_until_answered. */
typedef struct {
const cli_scope_attempt_t *outcomes;
int count;
int calls;
} cli_scope_scripted_attempts_t;
static cli_scope_attempt_t cli_scope_scripted_attempt(void *context) {
cli_scope_scripted_attempts_t *script = context;
int index = script->calls < script->count ? script->calls : script->count - 1;
script->calls++;
return script->outcomes[index];
}
/* The rule that replaced the status and connect timeouts: a mute endpoint is
* asked again, and only a reply or a definite refusal decides. */
TEST(cli_scope_retries_a_mute_endpoint_until_it_answers) {
static const cli_scope_attempt_t slow_then_answer[] = {CLI_SCOPE_MUTE, CLI_SCOPE_MUTE,
CLI_SCOPE_MUTE, CLI_SCOPE_ANSWERED};
cli_scope_scripted_attempts_t slow = {slow_then_answer, 4, 0};
ASSERT_TRUE(cli_scope_until_answered(cli_scope_scripted_attempt, &slow));
ASSERT_EQ(slow.calls, 4);
static const cli_scope_attempt_t slow_then_refused[] = {CLI_SCOPE_MUTE, CLI_SCOPE_REFUSED};
cli_scope_scripted_attempts_t refused = {slow_then_refused, 2, 0};
ASSERT_FALSE(cli_scope_until_answered(cli_scope_scripted_attempt, &refused));
ASSERT_EQ(refused.calls, 2);
static const cli_scope_attempt_t answered_at_once[] = {CLI_SCOPE_ANSWERED};
cli_scope_scripted_attempts_t at_once = {answered_at_once, 1, 0};
ASSERT_TRUE(cli_scope_until_answered(cli_scope_scripted_attempt, &at_once));
ASSERT_EQ(at_once.calls, 1);
PASS();
}
/* cli_scope_install with the activation's stdout captured into out. */
static int cli_scope_install_captured(cli_scope_fixture_t *fixture, const char *home,
const char *cache, const char *bin_dir, bool skip_binary,
@@ -17940,6 +18064,8 @@ SUITE(cli) {
RUN_TEST(cli_install_into_host_namespace_still_drains_host_cohort);
RUN_TEST(cli_install_skip_binary_unchanged_in_host_namespace_quiesces_nothing);
RUN_TEST(cli_scope_fixture_names_its_failing_setup_step);
RUN_TEST(cli_scope_fixture_finish_keeps_env_after_an_early_failure);
RUN_TEST(cli_scope_retries_a_mute_endpoint_until_it_answers);
RUN_TEST(cli_install_foreign_home_unreadable_cohort_cache_keeps_host_daemon);
RUN_TEST(cli_install_foreign_home_busy_cohort_keeps_host_daemon);
#endif