Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DeusData merged 1 commit into from
Oct 11, 2026
+172 −46
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
218 changes: 172 additions & 46 deletions tests/test_cli.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1572,25 +1572,30 @@ 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
* full 2 x CLI_SCOPE_TIMEOUT_MS, which is where a wedged teardown is real. */
#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
Expand Down Expand Up @@ -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;

Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}
}

/* 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;
}
cbm_usleep(2000);
}
}

/* 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;
Expand Down Expand Up @@ -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);
Expand All @@ -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"
Expand All @@ -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,
Expand Down Expand Up @@ -1917,22 +1970,28 @@ 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);
fixture->endpoint = NULL;
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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Loading