Skip to content

test(docker): poll for settled dashboard 'down' slot instead of racing boot - #63523

Closed
jethac wants to merge 2 commits into
NousResearch:mainfrom
jethac:fix/dashboard-slot-down
Closed

test(docker): poll for settled dashboard 'down' slot instead of racing boot#63523
jethac wants to merge 2 commits into
NousResearch:mainfrom
jethac:fix/dashboard-slot-down

Conversation

@jethac

@jethac jethac commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Problem

tests/docker/test_dashboard.py::test_dashboard_slot_reports_down_when_disabled is flaky in the Docker build/test job:

AssertionError: Dashboard slot should be 'down' without HERMES_DASHBOARD;
svstat reports: 'up (pid 155 pgid 155) 0 seconds'
assert 'down' in 'up (pid 155 pgid 155) 0 seconds'
tests/docker/test_dashboard.py:51

Note the 0 seconds uptime — svstat is catching the service in its first moments.

Root cause

The dashboard is an always-declared supervised s6 longrun (docker/s6-rc.d/dashboard/). When HERMES_DASHBOARD is unset:

  • run exits 0 immediately — no dashboard process is ever exec'd;
  • finish exits 125, s6's "permanent failure, do not restart" marker (≈ s6-svc -O), so s6-supervise leaves the slot down and never restarts it.

So the slot's steady state is down, with no process running — corroborated by the sibling test_dashboard_not_running_by_default (a pgrep check) which passes.

But s6-rc exec's the run wrapper once when it brings the user-services bundle up, so s6-svstat can briefly report up (...) 0 seconds before run exits and finish settles the slot to down. The failing test did a single immediate s6-svstat check with no retry — unlike its sibling test_dashboard_slot_reports_up_when_enabled, which polls — so it intermittently reads that one-time boot flap.

Fix

Poll for the settled down state (mirroring the "up" test), anchored on ^down so a transitioning up (...) want down line can't false-match a bare down. Also corrects the docstring, which still described the deleted cont-init.d/03-dashboard-toggle down-marker mechanism (the real mechanism is the run-exit-0 / finish-exit-125 pair).

Test-only change — no s6 service or product code touched; it only relaxes an immediate check into a bounded poll for the same condition the service already guarantees in steady state.

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure area/docker Docker image, Compose, packaging sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation P3 Low — cosmetic, nice to have labels Jul 13, 2026
@jethac
jethac force-pushed the fix/dashboard-slot-down branch from 40c19e6 to 29a7f39 Compare July 13, 2026 04:00
@jethac

jethac commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Depends on #63560 (adds jethachan@gmail.com to AUTHOR_MAP). The first commit on this branch is that mapping — the check-attribution job needs it present to pass. Once #63560 merges to main, this rebases and that commit drops out, leaving just the test fix.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating this Docker-test race. Current main still performs the single immediate s6-svstat assertion at tests/docker/test_dashboard.py:44-53, while the disabled service takes the run exit-0 path (docker/s6-rc.d/dashboard/run:9-18) and the permanent-down finish path (docker/s6-rc.d/dashboard/finish:19-28).

The proposed poll_container(... | grep -q '^down') use is bounded by the existing helper in tests/docker/conftest.py:224-246, checks the returned success value, and mirrors the enabled-slot polling pattern at tests/docker/test_dashboard.py:74-75. The change is test-only apart from the explained attribution mapping dependency; GitHub reports PR head 879c4832548d as mergeable.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 16, 2026
@jethac
jethac force-pushed the fix/dashboard-slot-down branch from 879c483 to 63b658b Compare July 18, 2026 02:02
jethac and others added 2 commits July 29, 2026 14:53
My commits on this PR use my personal email (no longer at Google). Add it to
AUTHOR_MAP alongside the existing jetha@google.com entry (which still attributes
an earlier commit) — both resolve to the same GitHub user. Fixes check-attribution.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g boot

test_dashboard_slot_reports_down_when_disabled did a single immediate
s6-svstat check with no retry, unlike its sibling
test_dashboard_slot_reports_up_when_enabled which polls. The dashboard is
an always-declared supervised s6 longrun; s6-rc exec's its run wrapper once
when it brings the user bundle up, so svstat can briefly report
`up (...) 0 seconds` before run exits 0 and finish exits 125 (permanent-down)
settle the slot to `down`. The immediate check races that one-time boot flap
and intermittently reads the transient `up`:

    AssertionError: Dashboard slot should be 'down' without HERMES_DASHBOARD;
    svstat reports: 'up (pid 155 pgid 155) 0 seconds'

The service is correctly gated (run exits 0, no daemon; finish exits 125,
no restart — verified, and the sibling pgrep test confirms no process runs),
so the slot's steady state is down. Poll for it, anchored on `^down` so a
transitioning `up (...) want down` line can't false-match. Also correct the
docstring, which still described the deleted cont-init.d/03-dashboard-toggle
down-marker mechanism.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jethac
jethac force-pushed the fix/dashboard-slot-down branch from 63b658b to d4af8e7 Compare July 29, 2026 06:03
@jethac

jethac commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Closing — the prune in #74383 removed test_dashboard_slot_reports_down_when_disabled (and its siblings) entirely, so there's nothing left for this fix to stabilize. Thanks for landing #63522 via #74517!

@jethac jethac closed this Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docker Docker image, Compose, packaging P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants