Skip to content

Comments

fix: resolve UE5 audio test race condition#28

Merged
AndrewAltimit merged 2 commits intomainfrom
fix/ci-audio-test-race-condition
Feb 21, 2026
Merged

fix: resolve UE5 audio test race condition#28
AndrewAltimit merged 2 commits intomainfrom
fix/ci-audio-test-race-condition

Conversation

@AndrewAltimit
Copy link
Owner

Summary

  • Fix flaky unload_track_fires_callback test that broke main CI by adding a Mutex to serialize tests sharing global callback atomics (LAST_EVENT/LAST_TRACK/LAST_VALUE)
  • Fix unused variable warning (port -> _port) in oasis-app test
  • Main CI and PR validation pipelines already run identical test suites -- the root cause was the race condition itself, not a pipeline gap

Root Cause

The UE5 audio tests use static AtomicU32 globals written by an extern "C" callback. When Rust runs tests in parallel, one test's reset_globals() can overwrite another's callback values between the action and the assertion. The failing assertion saw u32::MAX (reset sentinel) instead of 6 (TrackUnloaded).

Fix

init_backend() now acquires a Mutex guard that serializes all callback-dependent tests. The guard lives for the test's lifetime via destructuring: let (_guard, mut b) = init_backend();. Tests that don't use callbacks (e.g. default_state, has_audio_*) are unaffected.

Test plan

  • cargo test -p oasis-backend-ue5 -- audio -- all 22 tests pass
  • cargo test --workspace -- full suite passes
  • cargo clippy --workspace -- -D warnings -- clean
  • cargo fmt --all -- --check -- clean

Generated with Claude Code

The `unload_track_fires_callback` test was flaky due to shared global
static atomics (`LAST_EVENT`, `LAST_TRACK`, `LAST_VALUE`) being raced
by parallel test threads. One test's `reset_globals()` could clobber
another test's callback values mid-assertion.

Fix: add a `Mutex` to serialize all tests that use the shared callback
globals via `init_backend()`. The guard is held for the test's lifetime,
preventing interleaving. Also prefix unused `port` variable with `_`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

Gemini AI Code Review

Issues (if any)

(none)

Previous Issues (for incremental reviews)

(none)

Suggestions (if any)

(none)

Notes

  • The unused variable warning fix in oasis-app/src/commands.rs is correct.
  • Lock poisoning is properly handled via unwrap_or_else(|e| e.into_inner()), which correctly prevents one panicking test from cascading failures to other tests.
  • The let (_guard, mut b) binding correctly ensures the MutexGuard lives until the end of the test scope (unlike let _ = ... which would drop immediately), successfully preventing the race condition on the shared atomic variables.

Reaction


Generated by Gemini AI (gemini-3.1-pro-preview). Supplementary to human reviews.

@github-actions
Copy link

Codex AI Code Review

Issues (if any)

(none)

Previous Issues (for incremental reviews)

(none)

Suggestions (if any)

  • crates/oasis-app/src/commands.rs:599 - Remove let _port = 0; entirely since it is no longer used and the simulation is already represented by state.listener = Some(...).

Notes

  • Verified both changed files and line references in the provided diff; no actionable bugs or security issues were identified.
  • The mutex-guard lifetime pattern in crates/oasis-backend-ue5/src/audio.rs correctly serializes callback-atomic tests and addresses the stated race condition.

Reaction


Generated by Codex AI (gpt-5.3-codex). Supplementary to human reviews.

@AndrewAltimit
Copy link
Owner Author

Review Response Agent (Iteration 1)

Status: No changes needed

Fixed Issues

  • Removed dead let _port = 0; variable in crates/oasis-app/src/commands.rs:599 per Codex suggestion - the binding was entirely unused and should be deleted rather than suppressed with an underscore prefix

Ignored Issues

  • (none - both Gemini and Codex found no bugs or security issues)

Deferred to Human

  • (none)

Notes

  • Both Gemini and Codex gave the PR a clean review with no issues found
  • Gemini explicitly validated the mutex-guard lifetime pattern and poison handling as correct
  • The only actionable item was Codex's minor suggestion to remove the dead variable entirely, which has been applied and verified with clippy + test compilation

The agent reviewed feedback but determined no code changes were required.

Remove the entirely unused `let _port = 0;` binding rather than just
suppressing the warning with an underscore prefix. The listener config
constructs its own port value independently.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

Gemini AI Incremental Review

This is an incremental review focusing on changes since the last review.

Previous Issues

  • [RESOLVED] crates/oasis-app/src/commands.rs:596 - Unused port variable removed entirely based on previous feedback.

Issues

(none)

Suggestions

(none)

Notes

  • The Mutex in crates/oasis-backend-ue5/src/audio.rs serializes access to shared global callback atomics, effectively addressing the test race condition.
  • Capturing the MutexGuard as _guard correctly holds the lock for the full duration of each test.
  • Using unwrap_or_else(|e| e.into_inner()) to handle lock poisoning is good defensive practice — prevents a single failing test from deadlocking or cascading failures across the suite.

Reaction


Generated by Gemini AI (gemini-3.1-pro-preview). Supplementary to human reviews.

@github-actions
Copy link

Codex AI Incremental Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none)

Previous Issues (for incremental reviews)

  • [RESOLVED] crates/oasis-app/src/commands.rs:596 - Unused port test variable was removed.

Suggestions (if any)

(none)

Notes

  • Verified the two changed files in the diff; no new actionable bugs, security issues, or required fixes were identified.
  • The test-lock change in crates/oasis-backend-ue5/src/audio.rs correctly serializes callback-global assertions and addresses the reported race condition pattern.

Reaction


Generated by Codex AI (gpt-5.3-codex). Supplementary to human reviews.

@AndrewAltimit
Copy link
Owner Author

Review Response Agent (Iteration 2)

Status: No changes needed

Fixed Issues

  • (none needed - all previous feedback was already addressed in iteration 1)

Ignored Issues

  • (none - both Gemini and Codex incremental reviews found zero issues or suggestions)

Deferred to Human

  • (none)

Notes

  • Both Gemini (gemini-3.1-pro-preview) and Codex (gpt-5.3-codex) gave clean incremental reviews with no issues, no suggestions, and confirmed the previous Codex suggestion (remove dead port variable) as resolved
  • The mutex-guard pattern and poison handling in crates/oasis-backend-ue5/src/audio.rs were validated as correct by both reviewers
  • No code changes required for this iteration

The agent reviewed feedback but determined no code changes were required.

@AndrewAltimit AndrewAltimit merged commit 1f4755f into main Feb 21, 2026
7 checks passed
@AndrewAltimit AndrewAltimit deleted the fix/ci-audio-test-race-condition branch February 21, 2026 17:58
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