Skip to content

[tests] pass the sbx-mgr and template-manager suites on any test host - #1717

Closed
zibi-pplx wants to merge 1 commit into
mainfrom
zibi/tests-any-host
Closed

zibi-pplx wants to merge 1 commit into
mainfrom
zibi/tests-any-host

Conversation

@zibi-pplx

Copy link
Copy Markdown
Collaborator

Summary

Merging this makes cargo test pass the sbx-mgr and template-manager suites on any test host. Nine tests failed on main on an Apple-silicon Mac, as they would on any aarch64 host or in any run that shares one process between tests. CI didn't see it, because it runs each test in its own process (nextest) on x86-64. Only test code changes.

  • template-manager, five startup-recovery tests. The fixtures bind an x86-64 execution but name no architecture. Registration then falls back to the test host's architecture and stores a row whose arch disagrees with the binding ("runtime/arch columns disagree with execution metadata"). Each fixture now names x86_64 along with its binding.
  • sbx-mgr, two create tests. On a host other than x86-64, protocol_test_environment rebuilt E0 for the host's architecture. That gives it a new reference no compiled catalog qualifies, so Create refused it ("unknown execution boot contract"). The node-manager in these tests is a fake, so they now use E0 as compiled on every host. Two divergences go with it:
    • a per-host auto-lifecycle policy (qualified_test_policy);
    • the x86-64-only gate on selected_create_persists_the_binding_used_by_first_start, which passes on aarch64 too.
  • sbx-mgr, two cluster-api stub tests. Each with_cluster_api points the process-wide registration and backup logs at its own temporary files. Tests that share a process, as cargo test's threads do, recorded into each other's files, so a test could find its own log empty ("registration was never admitted: recorded []"). A process-wide async mutex now serializes these tests. Under nextest, with one test per process, it never waits.

Validation

  • Locally on macOS (Apple silicon), at this head:
    • cargo test -p pplx-space-template-mgr: 418 passed.
    • cargo test -p pplx-space-sbx-mgr: 1,761 passed, in six of eight full runs. The other two each had one different failure: waits_through_a_node_manager_restart_gap once and immediate_reclaim_removes_ordinary_entries_and_leaves_the_rest_in_the_lane once. Both pass every time on their own; see "Not covered".
    • On main, the same suites fail these nine tests every time.
  • cargo clippy --all-targets -D warnings on both crates is clean.
  • CI on this head covers Linux.

Severity and risk

Severity: low. Developers on Macs saw these tests fail without a product bug behind them.

Risk: test-only. The mutex can only make the cluster-api stub tests wait for each other within one process. Roll back by reverting.

Not covered

Two other sbx-mgr tests failed once each under the full in-process run and pass alone. They look load-related, and I haven't diagnosed them.

AI assistance: an AI coding agent diagnosed the failures and drafted the fixes and this description; the author reviewed them.

Nine tests failed under `cargo test` on an Apple-silicon Mac, and so would
on any aarch64 host or in any single-process run. CI passes them because
it runs each test in its own process on x86-64.

- template-manager: five startup-recovery fixtures bind an x86-64
  execution but name no architecture, so registration fell back to the
  test host's and stored a row whose arch disagrees with the binding.
  Each fixture now names `x86_64` with its binding.
- sbx-mgr create tests: on a host other than x86-64, the protocol test
  environment was rebuilt for the host's architecture, which gives it a
  reference no compiled catalog qualifies, so Create refused it. The
  node-manager is a fake, so the tests now use E0 as compiled on every
  host. That also drops a per-host auto-lifecycle policy and the x86-64
  gate on `selected_create_persists_the_binding_used_by_first_start`,
  which passes on aarch64 too.
- sbx-mgr cluster-api stub tests: each `with_cluster_api` points the
  process-wide registration and backup logs at its own files. Tests
  sharing a process, as `cargo test`'s threads do, recorded into each
  other's files, so a test found its own log empty. A process-wide async
  mutex now serializes them; under nextest it never waits.
@coderabbitai

coderabbitai Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Sandbox create tests now use the compiled x86-64 environment on every host and no longer adjust auto-lifecycle based on host architecture. Cluster API stub test runs are serialized with a process-wide mutex. Startup recovery fixtures set or copy the x86-64 manifest architecture.

Merge Risk: Minimal · up to 5f3b5

The test changes leave production behavior unchanged and have no identified merge-blocking risk. The two reported intermittent suite failures remain undiagnosed but are not tied to these changes.

Pre-merge checks: Passed 6

Check name Status Explanation
Title check Passed The title "[tests] pass the sbx-mgr and template-manager suites on any test host" uses the bracketed area prefix and a lowercase summary, as the title requirements call for. It accurately describes th…
Description Follows The Pr Template Passed The description contains all four required items. It states the change, validation results, severity and risk, and the AI coding agent's contributions.
Bug Fixes Name A Regression Test Passed The pull request changes test fixtures and test synchronization only. It does not fix an application behavior defect, so this check does not apply.
Weakened Safeguards Are Justified Passed The description and adjacent comments explain the host-independent fixture changes. The diff removes no assertions or tests. It also removes an architecture gate, which expands test coverage.
Agent Tooling Runs No New Commands Passed The diff changes only Rust test code. It adds no agent tooling configuration or executable agent content.
Public-Safe Content Passed The diff, title, and description contain no content prohibited by the check. The quoted errors describe test failures, not production logs.
Full details: Title check

Explanation

The title "[tests] pass the sbx-mgr and template-manager suites on any test host" uses the bracketed area prefix and a lowercase summary, as the title requirements call for. It accurately describes the changeset, which edits test code in sbx-mgr and template-manager to remove host-specific behavior and serialize cluster-api stub tests. The title does not end with a period and contains no ticket ID or link. The one deviation is that the title says "any test host" while the changes are aimed at specific host-dependent failures, so the scope is somewhat broad but still a real part of the change.

Recent review info

Automatic approval policy

Commit: 5f3b52bcb0cf.

Setting Effective value Source
Automatic approvals Enabled Global configuration
Merge risk Moderate Global configuration
Security risk Not set Global configuration

No overrides matched. The global limits apply.

Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 2f19d5f and 5f3b52b.

Files selected for processing: 3

  • crates/l40_api/sbx/mgr/server/src/service/grpc_core/create_after_lookup_tests.rs
  • crates/l40_api/sbx/mgr/server/src/service/grpc_lifecycle/retired_teardown_tests/cluster_api_stub_tests.rs
  • crates/l40_api/template/mgr/server/src/service/startup_recovery/tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review.

@zibi-pplx

Copy link
Copy Markdown
Collaborator Author

Folded into #1709, which now carries these test fixes along with the stack.

@zibi-pplx zibi-pplx closed this Nov 8, 2026
@zibi-pplx
zibi-pplx deleted the zibi/tests-any-host branch November 8, 2026 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant