Skip to content

fix: give client GameTests a virtual clock/dispatcher instead of real threads - #10

Merged
KP2048 merged 3 commits into
1.21.xfrom
test-frame-clock
Aug 7, 2026
Merged

fix: give client GameTests a virtual clock/dispatcher instead of real threads#10
KP2048 merged 3 commits into
1.21.xfrom
test-frame-clock

Conversation

@KP2048

@KP2048 KP2048 commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #9's "known open issue" note: gives client GameTests a virtual clock/dispatcher instead of real threads/wall-clock time, fixing the ~1-in-5 intermittent failure rate documented there.

Root cause

ComposeScreen/ComposeContainerScreen back their coroutine scope with Dispatchers.Default and real wall-clock time. A composable's delay(...) (e.g. a dialog's close animation) genuinely raced real thread scheduling and frame delivery against the harness's tick-based polling - a different test failing each run, no logged exception.

Fix

AClientGameTestHarness.run() now installs a ComposeTestClockOverride (common/.../gui/ComposeScreen.kt) around each @ClientGameTest: a StandardTestDispatcher plus a per-frame scheduler.advanceTimeBy(50) pump called from renderNodes(), matching how real Jetpack Compose's own test tooling (ComposeTestRule/runComposeUiTest) avoids this class of race by construction.

Real gotcha hit and fixed along the way: the first attempt used advanceUntilIdle() instead of advanceTimeBy(bounded). Composables can run legitimately infinite delay() loops (TextFieldCore's blinking-cursor LaunchedEffect), and advanceUntilIdle() only returns once truly nothing is scheduled anywhere - never, for an unboundedly-recurring loop. That hung the render thread permanently the moment any text field was on screen, and every subsequent test in that run then failed too (client's main-thread executor queue never got to run again). Switched to a fixed, bounded per-pump increment instead.

Keeping the dependency out of the shipped jar

kotlinx-coroutines-test is dev/test-only: compileOnly in common/build.gradle.kts, runtimeLibrary(...) (present for local runs like runGametestClient, never bundled - and the config that correctly handles NeoForge's classloader-layer artifact transform, not plain runtimeOnly) in the loader modules. ComposeScreen itself never references kotlinx.coroutines.test.* symbols directly - it only holds a plain CoroutineDispatcher? and a (() -> Unit)? pump callback (already-bundled core/stdlib types) - so a real player's game, which never has the test dependency on its classpath, never needs to resolve it.

Also in this PR

Test plan

  • :common:compileKotlin, :fabric:compileKotlin, :neoforge:compileKotlin all clean, no warnings
  • kotlinx-coroutines-test confirmed resolving correctly via runtimeLibraryConfiguration
  • Full local neoforge:runGametestClient: 8 consecutive clean runs, 131 individual tests total, zero failures - including the previously-hanging text field test and the originally-flaky ConfirmDialog/RadioGroup tests
  • Caught and fixed a real regression before landing: the first advanceUntilIdle() attempt hung the entire suite on the text field test; verified the advanceTimeBy fix resolves it before re-running the full batch

🤖 Generated with Claude Code

KP2048 and others added 3 commits August 6, 2026 22:21
- ArchieFluidSlot.Serializer's encodeNullableSerializableElement/
  decodeNullableSerializableElement (PR #9's structured-encoding fix) need
  @OptIn(ExperimentalSerializationApi::class), matching the existing pattern
  elsewhere in the codebase (NetworkChannel.kt, NBT.kt, ...).
- AClientGameTestHarness.run()'s new TestCoroutineScheduler.advanceTimeBy(...)
  call needs @OptIn(ExperimentalCoroutinesApi::class).

Also merges origin/1.21.x (now includes the just-merged PR #9) into this
branch, resolving one AGENTS.md conflict: replaced the "known open issue,
cause not confirmed" flakiness note PR #9 landed with this branch's own
"confirmed fixed" note, rather than keeping both.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@KP2048
KP2048 marked this pull request as ready for review August 7, 2026 02:36
Copilot AI lite review requested due to automatic review settings August 7, 2026 02:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@KP2048
KP2048 merged commit 273799e into 1.21.x Aug 7, 2026
5 of 7 checks passed
@KP2048
KP2048 deleted the test-frame-clock branch August 8, 2026 15:37
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.

2 participants