feat: support game-ci test --docker on Windows containers - #140
Conversation
`game-ci test --docker` rejected every non-Linux host outright. The container-side Windows entrypoint.ps1 had no RUN_TESTS branch (it always ran build.ps1), so allowing it through would have silently run a BUILD instead of a test - the guard was correct for the code as it stood. The original unity-test-runner action did support this, via its own dist/platforms/windows/run_tests.ps1; the capability was lost in the port to this CLI, not deliberately dropped. Rather than resurrect that script, entrypoint.ps1 now reuses steps/test.ps1 - the "native host" Windows test script - which is already container-safe: the only container/host difference that ever mattered is how the Unity Editor is located, and resolve_unity_path.ps1's Get-UnityEditorRoot already returns the image-baked $Env:UNITY_PATH when set. Docker.getWindowsCommand mounts the whole platforms/windows tree at c:\steps, so it needs no new volume. That also keeps the far better implementation (package mode, coverage gating, array-based argv) instead of the original's string-interpolated one, and avoids a ~250-line duplicate. Two further bugs found and fixed while verifying this end to end: - dist/test-standalone-scripts was never mounted into the container, so --testPlatforms=standalone died on `cp -R /UnityTestRunnerAction/...`. This affected LINUX too, not just Windows - the scripts survived the port but the mount didn't. Now mounted on both platforms, and only for test runs. - The Windows container entrypoint never exited with the build/test step's exit code; it just fell off the end, so the container's status was whatever return_license.ps1 happened to leave. Builds were shielded by UnityBuildValidation parsing the log, but a test run has no equivalent output check, so a failing test suite could have reported success. macOS stays rejected, with a clearer message - there are no Unity Editor Docker images for macOS at all, so there is nothing to run in. Verified: the PowerShell scripts parse under Windows PowerShell 5.1 (what the container actually runs, not pwsh 7), and steps/test.ps1 was executed against a stubbed UNITY_PATH to confirm it resolves its sibling resolve_unity_path.ps1 via $PSScriptRoot, honours $Env:UNITY_PATH, and surfaces $global:TEST_RUNNER_EXIT_CODE across the dot-source boundary the way entrypoint.ps1 consumes it. Full suite: 244 pre-existing failures before and after, +5 new passing tests.
|
Warning Review limit reachedNext included review available in 35 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (5)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughDocker-based Unity test execution now supports Windows containers. Linux and Windows commands mount standalone test scripts when ChangesWindows Docker test execution
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR adds Windows container test execution and related mounting and exit-code handling; no actionable merge-blocking risk remains based on the supplied evidence. Sequence Diagram(s)sequenceDiagram
participant UnityTestCommand
participant Docker.run
participant WindowsContainer
UnityTestCommand->>Docker.run: run windows-il2cpp with runTests
Docker.run->>WindowsContainer: mount test-standalone-scripts
WindowsContainer->>WindowsContainer: execute batchmode tests
WindowsContainer-->>UnityTestCommand: return execution result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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. Comment |
game-ci/unity-test-runner#310's Linux jobs ran their tests successfully - the log ends with `<test-run id="2" result="Passed" total="5" passed="5">` - and the CLI then reported: [ERROR] Error: There was an error building the project. at validateBuild (...) at async executeDocker (...) validateBuild() scrapes the container log for a "# Build results #" section, which only a real build ever emits. `game-ci test --docker` produces NUnit XML instead, so every passing test run was turned into a build failure. This is the same defect already fixed for activate-only runs (game-ci/unity-activate#111) - the guard just never learned about runTests. Test outcomes are validated from the results XML by the caller, so there is nothing for validateBuild to do on this path.
Added: a third bug, found by running this against a real matrixAfter fixing the stale Unity license on unity-test-runner#310, its Linux jobs got far enough to actually run the tests and pass them: …and the CLI then reported the run as a failure:
This is the same defect already fixed for activate-only runs (game-ci/unity-activate#111) — the guard simply never learned about Fixed in 1d51bb8, with a regression test. Note on rolloutunity-test-runner consumes the released CLI binary ( |
Releases the fixes from #140, which game-ci/unity-test-runner cannot pick up until a release exists - it downloads the published binary (cliVersion: latest), so it is still running v0.1.14. Both of unity-test-runner#310's remaining failure modes are fixed by this release, and nothing else: - Linux: `game-ci test --docker` ran the tests, passed them (<test-run result="Passed" total="5" passed="5">), and then failed the run anyway - validateBuild() scrapes the log for a "# Build results #" section that only a real build emits. - Windows: rejected outright by the hostPlatform guard, which existed because the container entrypoint.ps1 had no RUN_TESTS branch. It has one now, reusing the shared steps/test.ps1. Also carries the missing /UnityTestRunnerAction mount, without which --testPlatforms=standalone fails on `cp -R` (on Linux too, not just Windows).
1 participant
What
game-ci test --dockerrejected every non-Linux host outright:That guard was correct for the code as it stood —
dist/platforms/windows/entrypoint.ps1had noRUN_TESTSbranch and unconditionally ranbuild.ps1, so letting a test run through would have silently performed a build and reported success.This is what fails every Windows job on game-ci/unity-test-runner#310 (each dies in ~25s, well before Unity starts).
Why it regressed
The original unity-test-runner action did support this, via its own
dist/platforms/windows/run_tests.ps1. The capability was lost in the port to this CLI — not deliberately dropped.Approach
Rather than resurrect
run_tests.ps1,entrypoint.ps1now reusessteps/test.ps1— the "native host" Windows test script — which is already container-safe. The only container/host difference that ever mattered is how the Unity Editor is located, andresolve_unity_path.ps1'sGet-UnityEditorRootalready returns the image-baked$Env:UNITY_PATHwhen set, before falling back to the Unity Hub default.Docker.getWindowsCommandalready mounts the wholeplatforms/windowstree atc:\steps, so the shared script is reachable atc:\steps\steps\test.ps1with no new volume.This also keeps the strictly better implementation — package mode,
coverageEnabledgating, array-based argv — instead of the original's string-interpolated one, and avoids a ~250-line duplicate.Two further bugs found while verifying end to end
1.
--testPlatforms=standalonewas broken in Docker mode — on Linux too.dist/test-standalone-scriptswas never mounted into the container, soubuntu/steps/test.sh'scp -R "/UnityTestRunnerAction/Assets/..."failed outright. The scripts survived the port; the mount didn't (the original action mounted the same directory as/UnityStandaloneScripts). Now mounted on both platforms, and only for test runs.2. The Windows container never propagated its exit code.
entrypoint.ps1just fell off the end, so the container's status was whateverreturn_license.ps1happened to leave behind. Builds were shielded byUnityBuildValidationparsing the log output, but a test run has no equivalent output check — a failing suite could have reported success.macOS stays rejected, with a clearer message: there are no Unity Editor Docker images for macOS at all, so there is nothing to run the container scripts in.
Verification
powershell, notpwsh7). Both runtimes were also checked for the[int]""coercion the new exit-code casts rely on.steps/test.ps1was executed against a stubbedUNITY_PATHto confirm it resolves its siblingresolve_unity_path.ps1via$PSScriptRoot, honours$Env:UNITY_PATH(it attempted exactly the container's<UNITY_PATH>\Editor\Unity.exeshape), and surfaces$global:TEST_RUNNER_EXIT_CODEacross the dot-source boundaryentrypoint.ps1consumes it through.windows-il2cppimage; standalone mount present for test runs and absent for builds, both platforms).vi.mockunder bun's runner.)Note on unity-test-runner#310
This unblocks the Windows half of that PR's matrix. The Linux half fails for an unrelated reason — Unity licensing (
Code 500 ... No ULF license found/Code 10 while verifying Licensing Client signature), i.e. an expired or invalidUNITY_LICENSEsecret, not a code defect. Both halves need to be resolved for that matrix to go green.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation