Skip to content

fix: Test runs no longer race Play Mode cleanup#1347

Merged
hatayama merged 1 commit into
v3-betafrom
fix/run-tests-cleanup-wait
Jun 15, 2026
Merged

fix: Test runs no longer race Play Mode cleanup#1347
hatayama merged 1 commit into
v3-betafrom
fix/run-tests-cleanup-wait

Conversation

@hatayama

@hatayama hatayama commented Jun 15, 2026

Copy link
Copy Markdown
Owner

Summary

  • Test runs now give Unity Test Runner cleanup a short window before returning control to the next command.
  • Immediate Play Mode entry after a test run no longer trips delayed Test Runner cleanup work.

User Impact

  • Before this change, running tests and immediately starting Play Mode could leave Unity busy or surface delayed Test Runner cleanup errors.
  • After this change, command chains such as run-tests followed by Play Mode are more stable.

Changes

  • Wait briefly after successful Unity Test Runner execution before building the run-tests response.
  • Keep fail-fast paths unchanged so validation errors and missing Test Framework responses do not wait.
  • Add focused coverage for cleanup wait and skip paths.

Verification

  • cli\dist\windows-amd64\uloop.exe compile
  • cli\dist\windows-amd64\uloop.exe run-tests --test-mode EditMode --filter-type regex --filter-value 'io.github.hatayama.UnityCliLoop.Tests.Editor.RunTestsUseCaseTests'
  • cli\dist\windows-amd64\uloop.exe run-tests --test-mode EditMode --filter-type exact --filter-value 'io.github.hatayama.UnityCliLoop.Tests.Editor.CliPathSetupProfileResolverTests.ResolvePlan_WhenBashProfileExistsUsesExistingProfile' && cli\dist\windows-amd64\uloop.exe control-play-mode --action Play && cli\dist\windows-amd64\uloop.exe get-logs --include-stack-trace
  • codex review --commit HEAD

Review in cubic

RunFinished fires before Unity Test Framework has completed its editor cleanup tasks. Wait briefly after real test execution so immediate PlayMode entry does not race delayed Test Runner cleanup.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 2 files

Re-trigger cubic

@hatayama
hatayama merged commit 6b53da5 into v3-beta Jun 15, 2026
8 checks passed
@hatayama
hatayama deleted the fix/run-tests-cleanup-wait branch June 15, 2026 15:57
@github-actions github-actions Bot mentioned this pull request Jun 15, 2026
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 76f29db1-17fc-4e29-a68d-318bcc7e6098

📥 Commits

Reviewing files that changed from the base of the PR and between 188ccd7 and 7ec7ad3.

📒 Files selected for processing (2)
  • Assets/Tests/Editor/RunTestsUseCaseTests.cs
  • Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs

📝 Walkthrough

Walkthrough

RunTestsUseCase gains an optional Func<CancellationToken, Task> constructor parameter that defaults to a new WaitForTestRunnerCleanupAsync static method introducing a 3000ms TimerDelay.Wait. ExecuteAsync now awaits this delegate after test execution but before returning the result. Existing tests are updated to pass a no-op helper, and three new tests verify the delegate is called only on the successful path.

Changes

Post-test cleanup wait in RunTestsUseCase

Layer / File(s) Summary
Cleanup delegate field, wiring, and invocation
Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs
Adds _waitForTestRunnerCleanupAsync field and TestRunnerCleanupFallbackDelayMilliseconds constant; extends constructor with the optional delegate defaulting to null; assigns it with a fallback to the new static WaitForTestRunnerCleanupAsync; awaits it in ExecuteAsync after error handling; implements WaitForTestRunnerCleanupAsync via TimerDelay.Wait with the 3000ms constant.
Test updates and new cleanup-invocation tests
Assets/Tests/Editor/RunTestsUseCaseTests.cs
Updates all existing test constructors to pass NoCleanupWait; adds ExecuteAsync_AfterTestExecution_ShouldWaitForCleanup (asserts delegate invoked once), ExecuteAsync_WhenValidationFails_ShouldNotWaitForCleanup, and ExecuteAsync_WhenTestFrameworkUnavailable_ShouldNotWaitForCleanup (both assert delegate not invoked); introduces the NoCleanupWait private helper.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • hatayama/unity-cli-loop#940: Modifies RunTestsUseCase/RunTestsUseCaseTests around ExecuteAsync control flow, adding fail-fast validation for invalid EditMode-in-play-mode requests in the same use-case entry point touched here.
  • hatayama/unity-cli-loop#1192: Modifies RunTestsUseCase/RunTestsUseCaseTests around ExecuteAsync control flow for test-framework availability and cancellation, overlapping directly with the paths affected by the cleanup-wait change.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/run-tests-cleanup-wait

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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