feat: Add configurable init_timeout for PlayMode test initialization#1021
Merged
Scriptwonder merged 4 commits intoCoplayDev:betafrom Apr 29, 2026
Merged
Conversation
Contributor
Reviewer's GuideAdds a configurable initialization timeout for Unity PlayMode test jobs, threading an optional init_timeout parameter from the Python run_tests tool through the C# RunTests handler into TestJobManager, which now supports per-job init timeouts with persistence across domain reloads while keeping the default at 15s. Sequence diagram for configurable init_timeout through test job lifecyclesequenceDiagram
actor Developer
participant PythonRunTests as Python_run_tests_tool
participant MCPServer as MCP_Server
participant RunTestsHandler as RunTests_HandleCommand
participant TestJobManager as TestJobManager
Developer->>PythonRunTests: run_tests(mode=PlayMode, init_timeout=120000)
PythonRunTests->>MCPServer: Request with init_timeout
MCPServer->>RunTestsHandler: HandleCommand(params)
RunTestsHandler->>RunTestsHandler: initTimeoutMs = Params.GetInt(initTimeout) ?? 0
RunTestsHandler->>TestJobManager: StartJob(mode, filterOptions, initTimeoutMs)
TestJobManager->>TestJobManager: Create TestJob with InitTimeoutMs = initTimeoutMs
TestJobManager->>TestJobManager: PersistToSessionState(init_timeout_ms)
loop Poll job status
MCPServer->>TestJobManager: GetJob(jobId)
TestJobManager->>TestJobManager: initTimeout = InitTimeoutMs > 0 ? InitTimeoutMs : DefaultInitializationTimeoutMs
TestJobManager->>TestJobManager: Check now - StartedUnixMs > initTimeout
alt Exceeded initTimeout
TestJobManager->>TestJobManager: Status = Failed, Error = initialization timeout
else Within initTimeout
TestJobManager-->>MCPServer: Running or Completed status
end
end
TestJobManager->>TestJobManager: PersistToSessionState(init_timeout_ms)
Class diagram for updated TestJobManager and RunTests init timeout handlingclassDiagram
class TestJob {
string JobId
TestJobStatus Status
TestMode Mode
long StartedUnixMs
long FinishedUnixMs
long LastProgressUnixMs
long? LastFinishedUnixMs
List~TestJobFailure~ FailuresSoFar
string Error
TestRunResult Result
long InitTimeoutMs
}
class PersistedJob {
string job_id
string status
string mode
long started_unix_ms
long? finished_unix_ms
string last_finished_test_full_name
long? last_finished_unix_ms
List~TestJobFailure~ failures_so_far
string error
long init_timeout_ms
}
class TestJobManager {
<<static>>
const int FailureCap
const long StuckThresholdMs
const long DefaultInitializationTimeoutMs
const int MaxJobsToKeep
const long MinPersistIntervalMs
static string StartJob(TestMode mode, TestFilterOptions filterOptions, long initTimeoutMs)
static TestJob GetJob(string jobId)
static void TryRestoreFromSessionState()
static void PersistToSessionState(bool force)
}
class RunTests {
static Task~object~ HandleCommand(JObject params)
}
class ParamsHelper {
bool GetBool(string key)
int? GetInt(string key)
string GetString(string key)
}
RunTests --> TestJobManager : calls StartJob(mode, filterOptions, initTimeoutMs)
TestJobManager o-- TestJob : manages
TestJobManager o-- PersistedJob : serializes to SessionState
RunTests --> ParamsHelper : uses to read initTimeout
PersistedJob --> TestJob : maps init_timeout_ms to InitTimeoutMs
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPer-job initialization timeout support added: Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(220,240,255,0.5)
Client->>Server: POST /run_tests (mode, filter, init_timeout?)
end
rect rgba(240,255,220,0.5)
Server->>MCP Tool: send "run_tests" command (params include initTimeout if >0)
end
rect rgba(255,240,220,0.5)
MCP Tool / Unity Editor->>TestJobManager: StartJob(mode, filterOptions, initTimeoutMs)
Note right of TestJobManager: clamp initTimeoutMs -> set TestJob.InitTimeoutMs -> Persist to SessionState
TestJobManager->>TestJobManager: Periodic GetJob checks init timeout using per-job InitTimeoutMs (or default)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider validating/sanitizing the
initTimeoutMsvalue inRunTests.HandleCommand/TestJobManager.StartJob(e.g., clamp to a reasonable min/max and treat non-positive values as0) so that non-Python callers or malformed input can't set an accidentally huge or negative initialization timeout.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider validating/sanitizing the `initTimeoutMs` value in `RunTests.HandleCommand`/`TestJobManager.StartJob` (e.g., clamp to a reasonable min/max and treat non-positive values as `0`) so that non-Python callers or malformed input can't set an accidentally huge or negative initialization timeout.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MCPForUnity/Editor/Services/TestJobManager.cs (1)
301-325: 🛠️ Refactor suggestion | 🟠 MajorAdd Unity tests for
initTimeoutMsoverride and domain-reload restore functionality.The feature is implemented (parameter passing, persistence, restoration, timeout enforcement) but completely lacks test coverage. Tests should verify:
- Override applied when
initTimeoutMs > 0is passed- Default fallback to
DefaultInitializationTimeoutMs(15 seconds) wheninitTimeoutMs == 0- Persistence and restoration of
initTimeoutMsafter domain reloadAdd tests in
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RunTestsTests.csor create a dedicatedTestJobManagerTests.csfile.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Services/TestJobManager.cs` around lines 301 - 325, Add EditMode unit tests exercising StartJob's initTimeoutMs behavior: one test calls TestJobManager.StartJob(mode, filterOptions, initTimeoutMs>0) and asserts the returned/created TestJob has InitTimeoutMs equal to the passed value; a second test calls StartJob with initTimeoutMs==0 and asserts the TestJob.InitTimeoutMs equals TestJobManager.DefaultInitializationTimeoutMs (15s). Add a third test for persistence by starting a job with a nonzero initTimeoutMs, forcing the manager's persistence path to run, then simulate a domain-reload restore by invoking the TestJobManager restoration method (the same method used on domain reload) and assert the restored TestJob still has the original InitTimeoutMs. Reference StartJob, TestJob.InitTimeoutMs, DefaultInitializationTimeoutMs and the manager's persistence/restore methods in your new tests.
🧹 Nitpick comments (1)
Server/src/services/tools/run_tests.py (1)
170-172: Validate explicit invalidinit_timeoutvalues instead of silently dropping them.If callers pass
init_timeout <= 0, the current path ignores it and proceeds. Returning a clear error makes misconfiguration obvious.Proposed adjustment
- if init_timeout is not None and init_timeout > 0: - params["initTimeout"] = init_timeout + if init_timeout is not None: + if init_timeout <= 0: + return MCPResponse( + success=False, + error="init_timeout must be a positive integer (milliseconds)", + ) + params["initTimeout"] = init_timeoutAlso applies to: 203-204
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/services/tools/run_tests.py` around lines 170 - 172, The init_timeout parameter should be validated so invalid values (<= 0) are not silently ignored; in the function signature/parameter handling for init_timeout (and the other occurrence around lines 203-204 where init_timeout is used) add a guard that if init_timeout is not None and init_timeout <= 0 you raise a ValueError (or return an explicit error) with a clear message like "init_timeout must be > 0 or None" instead of dropping the value—update validation in the run_tests entrypoint/handler that declares init_timeout to enforce this rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@MCPForUnity/Editor/Services/TestJobManager.cs`:
- Around line 301-325: Add EditMode unit tests exercising StartJob's
initTimeoutMs behavior: one test calls TestJobManager.StartJob(mode,
filterOptions, initTimeoutMs>0) and asserts the returned/created TestJob has
InitTimeoutMs equal to the passed value; a second test calls StartJob with
initTimeoutMs==0 and asserts the TestJob.InitTimeoutMs equals
TestJobManager.DefaultInitializationTimeoutMs (15s). Add a third test for
persistence by starting a job with a nonzero initTimeoutMs, forcing the
manager's persistence path to run, then simulate a domain-reload restore by
invoking the TestJobManager restoration method (the same method used on domain
reload) and assert the restored TestJob still has the original InitTimeoutMs.
Reference StartJob, TestJob.InitTimeoutMs, DefaultInitializationTimeoutMs and
the manager's persistence/restore methods in your new tests.
---
Nitpick comments:
In `@Server/src/services/tools/run_tests.py`:
- Around line 170-172: The init_timeout parameter should be validated so invalid
values (<= 0) are not silently ignored; in the function signature/parameter
handling for init_timeout (and the other occurrence around lines 203-204 where
init_timeout is used) add a guard that if init_timeout is not None and
init_timeout <= 0 you raise a ValueError (or return an explicit error) with a
clear message like "init_timeout must be > 0 or None" instead of dropping the
value—update validation in the run_tests entrypoint/handler that declares
init_timeout to enforce this rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f29e7e8-145c-4496-887a-6f3b43f421bc
📒 Files selected for processing (5)
MCPForUnity/Editor/Services/TestJobManager.csMCPForUnity/Editor/Tools/RunTests.csServer/src/services/tools/run_tests.pydocs/development/README-DEV-zh.mddocs/development/README-DEV.md
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs (2)
23-24: Remove unused field_originalJobs.This field is declared but never assigned or used anywhere in the class.
🧹 Remove dead code
private string _originalJobId; - private object _originalJobs;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs` around lines 23 - 24, The private field _originalJobs in the TestJobManagerInitTimeoutTests class is declared but never used; remove the declaration "private object _originalJobs;" from the class to eliminate dead code and any unused field warning, ensuring you only change the field declaration and not other test logic or setup/teardown methods.
97-124: Potential test flakiness if Editor is compiling or updating.The
GetJobauto-fail logic inTestJobManager.cs(line 505) checks!EditorApplication.isCompiling && !EditorApplication.isUpdatingbefore applying the timeout. If either condition is true during test execution, the job won't auto-fail and this test will fail unexpectedly.This is unlikely in a controlled EditMode test environment, but worth noting. If flakiness is observed, consider adding a brief delay or explicit check.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs` around lines 97 - 124, The test can be flaky because TestJobManager.GetJob checks EditorApplication.isCompiling and EditorApplication.isUpdating before applying the timeout; modify the test GetJob_WithDefaultTimeout_AutoFailsAfter15Seconds to wait until EditorApplication.isCompiling and EditorApplication.isUpdating are false before invoking _getJobMethod (or explicitly poll with a short timeout and small delays), so the auto-fail path in TestJobManager.GetJob will be exercised deterministically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs`:
- Around line 23-24: The private field _originalJobs in the
TestJobManagerInitTimeoutTests class is declared but never used; remove the
declaration "private object _originalJobs;" from the class to eliminate dead
code and any unused field warning, ensuring you only change the field
declaration and not other test logic or setup/teardown methods.
- Around line 97-124: The test can be flaky because TestJobManager.GetJob checks
EditorApplication.isCompiling and EditorApplication.isUpdating before applying
the timeout; modify the test GetJob_WithDefaultTimeout_AutoFailsAfter15Seconds
to wait until EditorApplication.isCompiling and EditorApplication.isUpdating are
false before invoking _getJobMethod (or explicitly poll with a short timeout and
small delays), so the auto-fail path in TestJobManager.GetJob will be exercised
deterministically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27b7cef6-d43c-4a0a-b4df-05acf680c2a7
📒 Files selected for processing (5)
MCPForUnity/Editor/Services/TestJobManager.csServer/src/services/tools/run_tests.pyServer/tests/integration/test_run_tests_async.pyTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs.meta
✅ Files skipped from review due to trivial changes (1)
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs.meta
🚧 Files skipped from review as they are similar to previous changes (1)
- Server/src/services/tools/run_tests.py
Emerix
changed the title
PlayMode tests require entering play mode which triggers a domain reload. On large projects this can take >15s, causing the hardcoded 15s init timeout to auto-fail the test job before tests actually start. This adds an `init_timeout` parameter to `run_tests` that flows through the Python server → C# RunTests handler → TestJobManager. When set, the per-job timeout overrides the 15s default. The value is persisted across domain reloads via SessionState. Changes: - Python: Add `init_timeout` param to `run_tests()` function signature - C# RunTests: Read `initTimeout` param and pass to `StartJob()` - C# TestJobManager: Per-job `InitTimeoutMs` field with fallback to `DefaultInitializationTimeoutMs` (15s), persisted in SessionState Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Clamp initTimeoutMs in StartJob: negative values → 0, cap at 600s - Python: reject init_timeout <= 0 with explicit error before calling Unity - Add 3 C# EditMode tests for per-job InitTimeoutMs behavior (custom timeout, default timeout auto-fail, persist/restore) - Add 4 Python tests for init_timeout forwarding and validation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove unused _originalJobs field - Add Assume.That guards for EditorApplication.isCompiling/isUpdating so the test is skipped (inconclusive) rather than producing misleading results when the editor is mid-compilation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Emerix
force-pushed
the
feature/configurable-init-timeout
branch
from
4475218 to
3df5a84
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MCPForUnity/Editor/Services/TestJobManager.cs (1)
498-507:⚠️ Potential issue | 🟡 MinorUpdate the timeout comment to match the per-job behavior.
The code now resolves either the per-job timeout or the default, but the comment still says the job auto-fails after 15 seconds. That will mislead anyone debugging the stuck-job path.
✏️ Suggested comment fix
- // After 15 seconds without initialization, auto-fail the job to prevent hanging. + // After the resolved initialization timeout without initialization, auto-fail the job to prevent hanging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Services/TestJobManager.cs` around lines 498 - 507, The existing comment in TestJobManager incorrectly states a fixed 15-second timeout while the code uses a per-job timeout (job.InitTimeoutMs) falling back to DefaultInitializationTimeoutMs; update the comment above the check that uses job.Status, job.TotalTests, job.InitTimeoutMs and DefaultInitializationTimeoutMs to explain that the initialization timeout is per-job (job.InitTimeoutMs) or the default (DefaultInitializationTimeoutMs) and that the job will be auto-failed after whichever timeout value is resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs`:
- Around line 66-162: Tests that assert auto-fail behavior are flaking because
GetJob suppresses timeouts while the editor is compiling/updating; add a guard
at the start of the three tests (GetJob_WithCustomInitTimeout_UsesPerJobTimeout,
GetJob_WithDefaultTimeout_AutoFailsAfter15Seconds,
InitTimeoutMs_SurvivesPersistAndRestore) to skip or defer timeout assertions if
EditorApplication.isCompiling or EditorApplication.isUpdating is true (or re-run
the check after a short wait), so the assertions about TestJobStatus (the
Assert.AreEqual checks against TestJobStatus.Running/Failed) only run when the
editor is idle and GetJob's timeout logic will behave deterministically.
- Around line 24-64: The TearDown currently clears only the in-memory Jobs
dictionary but leaves the editor SessionState populated; after removing the test
keys from the _jobsField dictionary and restoring _currentJobIdField, invoke the
manager's PersistToSessionState (use the _persistMethod) so the cleaned/restored
state is written back to SessionState (alternatively call
TryRestoreFromSessionState if intended) to ensure no synthetic jobs remain in
the shared editor SessionState across tests.
---
Outside diff comments:
In `@MCPForUnity/Editor/Services/TestJobManager.cs`:
- Around line 498-507: The existing comment in TestJobManager incorrectly states
a fixed 15-second timeout while the code uses a per-job timeout
(job.InitTimeoutMs) falling back to DefaultInitializationTimeoutMs; update the
comment above the check that uses job.Status, job.TotalTests, job.InitTimeoutMs
and DefaultInitializationTimeoutMs to explain that the initialization timeout is
per-job (job.InitTimeoutMs) or the default (DefaultInitializationTimeoutMs) and
that the job will be auto-failed after whichever timeout value is resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 86061baf-f56a-49a9-9aa6-a19406939fea
📒 Files selected for processing (8)
MCPForUnity/Editor/Services/TestJobManager.csMCPForUnity/Editor/Tools/RunTests.csServer/src/services/tools/run_tests.pyServer/tests/integration/test_run_tests_async.pyTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs.metadocs/development/README-DEV-zh.mddocs/development/README-DEV.md
✅ Files skipped from review due to trivial changes (3)
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs.meta
- docs/development/README-DEV.md
- docs/development/README-DEV-zh.md
🚧 Files skipped from review as they are similar to previous changes (1)
- MCPForUnity/Editor/Tools/RunTests.cs
The persist/restore test writes synthetic jobs to SessionState, which survives domain reloads and would be re-hydrated by TestJobManager's [InitializeOnLoadMethod] hook. Flush the cleaned in-memory state back to SessionState in TearDown so test artifacts don't leak across runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs (2)
54-68:⚠️ Potential issue | 🟠 MajorRestore the original SessionState, not just the in-memory jobs.
Persisting the cleaned dictionary still overwrites whatever was already in editor SessionState before the test ran. Snapshot the SessionState strings in
SetUpand write them back here after removing the synthetic jobs.🧹 Suggested cleanup
private string _originalJobId; + private string _originalJobsSessionState; + private string _originalCurrentJobIdSessionState; @@ _originalJobId = _currentJobIdField.GetValue(null) as string; + _originalJobsSessionState = UnityEditor.SessionState.GetString("MCPForUnity.TestJobsV1", string.Empty); + _originalCurrentJobIdSessionState = UnityEditor.SessionState.GetString("MCPForUnity.CurrentTestJobIdV1", string.Empty); @@ _currentJobIdField.SetValue(null, _originalJobId); // Clean up any test jobs we inserted var jobs = _jobsField.GetValue(null) as System.Collections.IDictionary; jobs?.Remove("test-init-timeout-job"); jobs?.Remove("test-init-timeout-default"); jobs?.Remove("test-init-timeout-persist"); - // Flush cleaned state to SessionState so synthetic jobs don't survive domain reloads. - // The persist test writes to SessionState; without this, the stub job would be - // restored on the next [InitializeOnLoadMethod] and pollute later test runs. - _persistMethod.Invoke(null, new object[] { true }); + UnityEditor.SessionState.SetString("MCPForUnity.TestJobsV1", _originalJobsSessionState); + UnityEditor.SessionState.SetString("MCPForUnity.CurrentTestJobIdV1", _originalCurrentJobIdSessionState);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs` around lines 54 - 68, TearDown currently restores only in-memory fields (_currentJobIdField, _jobsField) and calls _persistMethod, which overwrites any pre-existing editor SessionState; modify the test to snapshot the relevant SessionState string(s) during SetUp and restore that snapshot in TearDown instead of blindly persisting the cleaned dictionary: in SetUp capture the original SessionState value(s) that the tested code reads/writes, store them on a private field (e.g. _originalSessionStateString), and in TearDown invoke _persistMethod or the appropriate SessionState API to write back that original string after removing the synthetic jobs and resetting _currentJobIdField so the editor SessionState is exactly restored.
100-127:⚠️ Potential issue | 🟡 MinorScope the auto-fail assertion to an idle editor.
GetJobsuppresses init-timeout failures while Unity is compiling or updating, so this test can still be flaky unless it only runs when the editor is idle.🧪 Suggested guard
[Test] public void GetJob_WithDefaultTimeout_AutoFailsAfter15Seconds() { + Assume.That( + !UnityEditor.EditorApplication.isCompiling && !UnityEditor.EditorApplication.isUpdating, + "Timeout auto-fail is undefined while the editor is compiling or updating."); + // Arrange: insert a job with InitTimeoutMs=0 (use default) and start time 20s ago🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs` around lines 100 - 127, The test GetJob_WithDefaultTimeout_AutoFailsAfter15Seconds calls GetJob which suppresses init-timeout failures while the editor is busy; scope the assertion to an idle editor by checking EditorApplication.isCompiling and EditorApplication.isUpdating (or EditorApplication.isPlaying/other relevant busy flags) inside the TestJobManagerInitTimeoutTests.GetJob_WithDefaultTimeout_AutoFailsAfter15Seconds test and either skip the assertion (Assert.Ignore) when the editor is busy or wait/poll until both are false before invoking GetJob, so the auto-fail check runs only when the editor is idle.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs`:
- Around line 54-68: TearDown currently restores only in-memory fields
(_currentJobIdField, _jobsField) and calls _persistMethod, which overwrites any
pre-existing editor SessionState; modify the test to snapshot the relevant
SessionState string(s) during SetUp and restore that snapshot in TearDown
instead of blindly persisting the cleaned dictionary: in SetUp capture the
original SessionState value(s) that the tested code reads/writes, store them on
a private field (e.g. _originalSessionStateString), and in TearDown invoke
_persistMethod or the appropriate SessionState API to write back that original
string after removing the synthetic jobs and resetting _currentJobIdField so the
editor SessionState is exactly restored.
- Around line 100-127: The test
GetJob_WithDefaultTimeout_AutoFailsAfter15Seconds calls GetJob which suppresses
init-timeout failures while the editor is busy; scope the assertion to an idle
editor by checking EditorApplication.isCompiling and
EditorApplication.isUpdating (or EditorApplication.isPlaying/other relevant busy
flags) inside the
TestJobManagerInitTimeoutTests.GetJob_WithDefaultTimeout_AutoFailsAfter15Seconds
test and either skip the assertion (Assert.Ignore) when the editor is busy or
wait/poll until both are false before invoking GetJob, so the auto-fail check
runs only when the editor is idle.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59c4bedf-6a1a-43cb-8d2e-1631af993300
📒 Files selected for processing (1)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TestJobManagerInitTimeoutTests.cs
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
PlayMode tests require entering play mode which triggers a domain reload. On large Unity projects this can take >15s, causing the hardcoded 15s init timeout to
auto-fail the test job before tests actually start. This PR adds a configurable
init_timeoutparameter torun_testsso callers can specify a longerinitialization timeout for PlayMode tests.
Type of Change
Changes Made
Server/src/services/tools/run_tests.py): Addedinit_timeoutparameter torun_tests()function signature. When set, forwarded to Unity asinitTimeoutin the params dict.MCPForUnity/Editor/Tools/RunTests.cs): ReadsinitTimeoutparam from the request and passes it toTestJobManager.StartJob().MCPForUnity/Editor/Services/TestJobManager.cs):InitTimeoutMsfield toTestJobfor per-job timeout overrideInitializationTimeoutMstoDefaultInitializationTimeoutMs(15s, unchanged default)StartJob()accepts optionalinitTimeoutMsparameterGetJob()uses per-job timeout when set, falls back to 15s defaultInitTimeoutMspersisted/restored across domain reloads via SessionStateTesting/Screenshots/Recordings
Tested on a large Unity project (~18s domain reload time for PlayMode):
init_timeout=120000init_timeout(default 15s is sufficient)Documentation Updates
tools/UPDATE_DOCS_PROMPT.md(recommended)tools/UPDATE_DOCS.mdRelated Issues
None
Additional Notes
The default timeout remains 15s (no breaking change). The
init_timeoutparameter is optional and only needed for projects where PlayMode domain reload exceeds15s. Recommended value for PlayMode: 120000 (120s).
Summary by Sourcery
Add configurable per-job initialization timeout for Unity test runs and propagate it from the Python run_tests tool through the editor pipeline.
New Features:
Enhancements:
Documentation:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests