You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[tests] pass the sbx-mgr and template-manager suites on any test host - #1717
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.
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: 6
Check name
Status
Explanation
Title check
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
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
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
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
The diff changes only Rust test code. It adds no agent tooling configuration or executable agent content.
Public-Safe Content
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.
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Merging this makes
cargo testpass the sbx-mgr and template-manager suites on any test host. Nine tests failed onmainon 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.archdisagrees with the binding ("runtime/arch columns disagree with execution metadata"). Each fixture now namesx86_64along with its binding.protocol_test_environmentrebuilt 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:qualified_test_policy);selected_create_persists_the_binding_used_by_first_start, which passes on aarch64 too.with_cluster_apipoints the process-wide registration and backup logs at its own temporary files. Tests that share a process, ascargo 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
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_gaponce andimmediate_reclaim_removes_ordinary_entries_and_leaves_the_rest_in_the_laneonce. Both pass every time on their own; see "Not covered".main, the same suites fail these nine tests every time.cargo clippy --all-targets -D warningson both crates is clean.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.