mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-09 01:35:18 +02:00
tests: matrix harness uses with_defaults() workload, not from_env()
Round 4 CI failure: Test Workspace and server-aws both red on
`concurrent_branch_ops_morphological_matrix` cell b
("merge × merge: same-target-distinct-sources") — second merge
returned 429 instead of 200. The matrix passes locally.
Root cause: cargo test runs tests in parallel by default. The admission
test `ingest_per_actor_admission_cap_returns_429` is wrapped with
`#[serial]` and an EnvGuard that sets
`OMNIGRAPH_PER_ACTOR_INFLIGHT_MAX=1` for its duration. Process-wide
env vars are visible to concurrently-running tests; the matrix's
`Harness::new()` called `AppState::open()` which delegates to
`WorkloadController::from_env()`, picking up cap=1 if it ran while
the admission test held the EnvGuard. With cap=1 + 2 concurrent
merges in cell b, one merge waits behind merge_exclusive while the
other is admitted; the waiter holds its admission permit, but a
fresh actor permit is needed when admission is per-actor — the
second merge's permit acquisition fails because the first hasn't
released yet, and 429 fires.
Fix (correct by design, AGENTS.md rule 9): the matrix harness builds
the WorkloadController explicitly via
`WorkloadController::with_defaults()` and passes it to
`AppState::new_with_workload`, the constructor added in commit
22d76db. Closes the bug class "tests pick up another concurrent test's
env override at construction time" — the matrix is now insulated from
any env-var manipulation in the rest of the test suite.
Verified locally: with `OMNIGRAPH_PER_ACTOR_INFLIGHT_MAX=1` set in the
environment, the matrix passes (it ignores env entirely now).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
99b0941478
commit
8bd9a5ff14
1 changed files with 21 additions and 3 deletions
|
|
@ -2436,9 +2436,27 @@ mod matrix {
|
|||
pub async fn new() -> Self {
|
||||
let temp = init_loaded_repo().await;
|
||||
let repo = repo_path(temp.path());
|
||||
let state = AppState::open(repo.to_string_lossy().to_string())
|
||||
.await
|
||||
.unwrap();
|
||||
// Build the WorkloadController explicitly with defaults rather
|
||||
// than letting `AppState::open` call
|
||||
// `WorkloadController::from_env()`. The admission-gate test
|
||||
// (`ingest_per_actor_admission_cap_returns_429`) sets
|
||||
// OMNIGRAPH_PER_ACTOR_INFLIGHT_MAX=1 inside an EnvGuard while
|
||||
// it runs. Process-wide env vars are visible to
|
||||
// concurrently-running tests; if a matrix cell reads env at
|
||||
// AppState construction time during that window it picks up
|
||||
// cap=1 and the second concurrent merge in cell b surfaces
|
||||
// 429 instead of the expected 200. Constructing the
|
||||
// controller here with explicit defaults makes cells
|
||||
// independent of any env mutation other tests perform.
|
||||
let db = Omnigraph::open(repo.to_str().unwrap()).await.unwrap();
|
||||
let workload =
|
||||
omnigraph_server::workload::WorkloadController::with_defaults();
|
||||
let state = AppState::new_with_workload(
|
||||
repo.to_string_lossy().to_string(),
|
||||
db,
|
||||
Vec::new(),
|
||||
workload,
|
||||
);
|
||||
let app = build_app(state);
|
||||
Self {
|
||||
_temp: temp,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue