From 9464879a8025dd5d80f08269de8b0c6cc876b97c Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Mon, 28 Sep 2026 10:48:02 -0400 Subject: [PATCH] test(marmot): harness checks that could pass without testing anything - Removal checks read "not in the member list" off a pipeline where a failed wn query (dead daemon, ok:false) also produced no match. Tests 06 and 34 now require wn's own positive signal (self_membership "removed"); the leave and avatar checks use wn_lists, which tells "answered, not listed" from "no answer". - setup.sh fell through to the old amy binary when all four builds failed; it now fails. - Test 27's "tombstone published" check compared a count to false, so it never fired; it requires published > 0. - The short wnd socket dir was random per run: dirs piled up in /tmp and start_daemon could not find a wnd a killed run left behind. It is now derived from the path (0700, reused only when ours). Headless run: 32 passed, 0 failed, 2 skipped. Co-Authored-By: Claude Opus 5.5 --- cli/tests/headless/helpers.sh | 17 +++++++++++++++-- cli/tests/lib.sh | 23 +++++++++++++++++++++++ cli/tests/marmot/setup.sh | 12 ++++++++---- cli/tests/marmot/tests-manage.sh | 27 ++++++++++----------------- cli/tests/marmot/tests-media.sh | 2 +- 5 files changed, 57 insertions(+), 24 deletions(-) diff --git a/cli/tests/headless/helpers.sh b/cli/tests/headless/helpers.sh index 1a9b97e3e1..124f13e5c2 100644 --- a/cli/tests/headless/helpers.sh +++ b/cli/tests/headless/helpers.sh @@ -72,13 +72,26 @@ assert_eq() { # checkout under an ordinary home dir already crosses it: ~85 bytes failed. Move # such a socket into a short private temp dir (0700, since wnd also refuses a # socket dir others can read). +# +# The dir is derived from the long path, not random: the same checkout gets the same +# socket every run, so `start_daemon` still finds a wnd a killed run left behind +# instead of starting a second one on the same data, and runs stop piling up dirs. short_socket_path() { local path="$1" if [[ ${#path} -lt 70 ]]; then printf '%s' "$path"; return; fi # Resolved, not /tmp itself: on macOS /tmp is a symlink to /private/tmp and wnd # refuses a socket path through an alias ("untrusted directory alias"). - local dir - dir="$(mktemp -d "$(cd /tmp && pwd -P)/wnd.XXXXXX")" && chmod 700 "$dir" + local tmp dir + tmp="$(cd /tmp && pwd -P)" + dir="$tmp/wnd-$(id -u)-$(printf '%s' "$path" | cksum | cut -d' ' -f1)" + if ! mkdir -m 700 "$dir" 2>/dev/null; then + # Reuse only a dir that is ours and private: /tmp is shared, and a socket in a + # dir someone else controls could be swapped under wnd. + if [[ ! -d "$dir" || -L "$dir" || ! -O "$dir" ]]; then + dir="$(mktemp -d "$tmp/wnd.XXXXXX")" + fi + chmod 700 "$dir" + fi printf '%s/%s.sock' "$dir" "$(basename "$(dirname "$path")")" } diff --git a/cli/tests/lib.sh b/cli/tests/lib.sh index 2bb3f00627..c0f55b5f70 100644 --- a/cli/tests/lib.sh +++ b/cli/tests/lib.sh @@ -199,6 +199,29 @@ jq_list() { ' 2>/dev/null || true } +# Whether 's wn lists in the group's . Three answers, because a +# query that fails is not evidence of absence: piped straight into `jq -e select`, a dead +# daemon or an `ok:false` reply read exactly like "not listed" and passed removal tests. +# 0 listed · 1 wn answered and is not listed · 2 no usable answer +wn_lists() { + local who="$1" gid="$2" list="$3" hex="$4" out + out=$("wn_$who" --json groups members "$gid" 2>/dev/null) || return 2 + printf '%s' "$out" | jq -e '.ok == true' >/dev/null 2>&1 || return 2 + if printf '%s' "$out" | jq_list "$list" | jq -e --arg p "$hex" \ + 'select((.member_id // .admin_id // .pubkey // .public_key) == $p)' >/dev/null 2>&1; then + return 0 + fi + return 1 +} + +# Whether 's wn reports that it was itself removed from the group: MDK's +# `groups show` carries `self_membership: "removed"` once the removal commit applied. +wn_self_removed() { + local who="$1" gid="$2" + "wn_$who" --json groups show "$gid" 2>/dev/null \ + | jq -e '.ok == true and .result.group.self_membership == "removed"' >/dev/null 2>&1 +} + # npub or hex pubkey of one member/admin entry. MDK names the field per # collection: members carry `member_id`, admins carry `admin_id`. jq_member_ids() { diff --git a/cli/tests/marmot/setup.sh b/cli/tests/marmot/setup.sh index 8cbdc01d97..163d0a0f89 100644 --- a/cli/tests/marmot/setup.sh +++ b/cli/tests/marmot/setup.sh @@ -33,15 +33,19 @@ preflight() { [[ -x "$AMY_BIN" ]] || { fail_msg "amy not found at $AMY_BIN and --no-build set"; exit 1; } warn "--no-build: using the existing $AMY_BIN, which may be older than this checkout" else - local attempt max_attempts=4 + local attempt max_attempts=4 built=0 for attempt in $(seq 1 $max_attempts); do step "building :cli:installDist (attempt $attempt/$max_attempts)" - if ( cd "$REPO_ROOT" && ./gradlew :cli:installDist ) 2>&1 | tee -a "$LOG_FILE" \ - && [[ -x "$AMY_BIN" ]]; then - break + ( cd "$REPO_ROOT" && ./gradlew :cli:installDist ) 2>&1 | tee -a "$LOG_FILE" + # gradle's status, not tee's: a failed build must not fall through to the old binary. + if [[ "${PIPESTATUS[0]}" -eq 0 && -x "$AMY_BIN" ]]; then + built=1; break fi [[ "$attempt" -lt "$max_attempts" ]] && warn "gradle build failed (likely transient jitpack/Google 503) — retrying" done + # The binary may still exist from an earlier build; running the suite on it would test + # code this checkout no longer has. + [[ "$built" -eq 1 ]] || { fail_msg "amy build failed after $max_attempts attempts"; exit 1; } fi [[ -x "$AMY_BIN" ]] || { fail_msg "amy still missing after build"; exit 1; } info "amy: $AMY_BIN" diff --git a/cli/tests/marmot/tests-manage.sh b/cli/tests/marmot/tests-manage.sh index 2517915ce3..f201392fa6 100644 --- a/cli/tests/marmot/tests-manage.sh +++ b/cli/tests/marmot/tests-manage.sh @@ -27,10 +27,7 @@ test_06_member_removal() { # C should no longer see the group on its own member view. local deadline=$(( $(date +%s) + 120 )) removed=0 while [[ $(date +%s) -lt $deadline ]]; do - if ! wn_c --json groups members "$mls_gid" 2>/dev/null \ - | jq_list members | jq -e --arg p "$C_HEX" \ - 'select((.member_id // .pubkey // .public_key) == $p)' \ - >/dev/null 2>&1; then + if wn_self_removed c "$mls_gid"; then removed=1; break fi sleep 3 @@ -180,10 +177,9 @@ test_11_leave_group() { local deadline=$(( $(date +%s) + 120 )) gone=0 while [[ $(date +%s) -lt $deadline ]]; do - if ! wn_b --json groups members "$mls_gid" 2>/dev/null \ - | jq_list admins | jq -e --arg p "$A_HEX" \ - 'select((.admin_id // .pubkey // .public_key) == $p)' \ - >/dev/null 2>&1; then + local rc=0 + wn_lists b "$mls_gid" admins "$A_HEX" || rc=$? + if [[ "$rc" -eq 1 ]]; then gone=1; break fi sleep 3 @@ -222,10 +218,9 @@ test_17_group_image_commit() { # Skip cleanly if A is no longer a member of GROUP_02 (a later test may have removed # A) — this test only makes sense while A can still commit to the group. - if ! wn_b --json groups members "$mls_gid" 2>/dev/null \ - | jq_list members | jq -e --arg p "$A_HEX" \ - 'select((.member_id // .pubkey // .public_key) == $p)' \ - >/dev/null 2>&1; then + local listed=0 + wn_lists b "$mls_gid" members "$A_HEX" || listed=$? + if [[ "$listed" -eq 1 ]]; then record_result "$id" skip "A not in GROUP_02"; return fi @@ -423,12 +418,10 @@ test_34_amy_removes_last_other_member() { local deadline=$(( $(date +%s) + 120 )) gone=0 view while [[ $(date +%s) -lt $deadline ]]; do wn_b sync >/dev/null 2>&1 || true - # `groups members`, as test 06 reads it: `groups show` carries no member list, and - # reading one there made this pass before wn had processed anything. + # A positive signal: wn marks its own copy removed once it applied the commit. Reading + # "B is not in the member list" instead passed whenever the query itself failed. view=$(wn_b_json groups show "$mls_gid" 2>/dev/null || true) - if ! wn_b --json groups members "$mls_gid" 2>/dev/null \ - | jq_list members | jq -e --arg p "$B_HEX" \ - 'select((.member_id // .pubkey // .public_key) == $p)' >/dev/null 2>&1; then + if wn_self_removed b "$mls_gid"; then gone=1; break fi sleep 3 diff --git a/cli/tests/marmot/tests-media.sh b/cli/tests/marmot/tests-media.sh index 620ab6ba86..ad5e9b92c9 100644 --- a/cli/tests/marmot/tests-media.sh +++ b/cli/tests/marmot/tests-media.sh @@ -446,7 +446,7 @@ test_27_deletion_wn_to_amy() { record_result "$id" fail "wn messages delete failed"; return } printf 'wn messages delete %s -> %s\n' "$target" "$del" >>"$LOG_FILE" - if ! printf '%s' "$del" | jq -e '.result.published != false' >/dev/null 2>&1; then + if ! printf '%s' "$del" | jq -e '(.result.published // 0) > 0' >/dev/null 2>&1; then record_result "$id" fail "wn did not publish the delete tombstone: $del"; return fi