diff --git a/dist/platforms/windows/entrypoint.ps1 b/dist/platforms/windows/entrypoint.ps1 index 00188994..dd327707 100644 --- a/dist/platforms/windows/entrypoint.ps1 +++ b/dist/platforms/windows/entrypoint.ps1 @@ -39,10 +39,64 @@ if ($Env:ACTIVATE_ONLY -eq "true") { exit $LASTEXITCODE } -# Build the project -& "c:\steps\build.ps1" +# RUN_TESTS=true (used by `game-ci test --docker`, see game-ci/cli's +# UnityTestCommand) runs the classic batchmode test flow instead of a build - +# same activation/license-return steps either way, only the middle step +# differs. Mirrors ubuntu/steps/runsteps.sh's own RUN_TESTS branch. +# +# The test implementation is deliberately NOT duplicated into this +# container script set. steps/test.ps1 (the native-host set, one directory +# down) is already container-safe: the only container/host difference that +# ever mattered is how the Unity Editor is located, and its +# resolve_unity_path.ps1 already honours the image-baked $Env:UNITY_PATH +# as-is (see Get-UnityEditorRoot) before falling back to the Unity Hub +# default. Docker.getWindowsCommand mounts the whole +# dist/platforms/windows directory at c:\steps, so that script is already +# present at c:\steps\steps\test.ps1 - no extra volume needed. The doubled +# "steps\steps" path is that mount's artifact, not a typo. +# +# Dot-sourced rather than called with & so the $global:TEST_RUNNER_EXIT_CODE +# it sets is visible here; build.ps1 communicates via $Env: instead, which +# crosses the & call boundary on its own. +if ($Env:RUN_TESTS -eq "true") { + . "c:\steps\steps\test.ps1" + $StepExitCode = [int]$global:TEST_RUNNER_EXIT_CODE +} else { + & "c:\steps\build.ps1" + $StepExitCode = [int]$Env:BUILD_EXIT_CODE +} # Free the seat for the activated license if ($Env:SKIP_ACTIVATION -ne "true") { & "c:\steps\return_license.ps1" } + +# +# Instructions for debugging - matches ubuntu/steps/runsteps.sh's own block. +# + +if ($StepExitCode -gt 0) { + Write-Host "" + Write-Host "###########################" + Write-Host "# Failure #" + Write-Host "###########################" + Write-Host "" + Write-Host "Please note that the exit code is not very descriptive." + Write-Host "Most likely it will not help you solve the issue." + Write-Host "" + Write-Host "To find the reason for failure: please search for errors in the log above." + Write-Host "" +} + +# +# Exit with the code from the build/test step. +# +# Previously this script just fell off the end, so the container's exit code +# was whatever the last command (return_license.ps1) happened to leave +# behind - a build/test failure could therefore surface as a *successful* +# container run. Builds were saved from that by +# UnityBuildValidation.validateBuild parsing the log output, but a test run +# has no equivalent output check, so propagate the real code explicitly. +# + +exit $StepExitCode diff --git a/dist/platforms/windows/steps/runsteps.ps1 b/dist/platforms/windows/steps/runsteps.ps1 index dbbb8f46..9e40c482 100644 --- a/dist/platforms/windows/steps/runsteps.ps1 +++ b/dist/platforms/windows/steps/runsteps.ps1 @@ -5,8 +5,13 @@ # HostRunner (src/model/host-runner.ts, see its class doc comment) against # a self-hosted Windows machine with Unity already installed via Unity Hub # - NOT the dist/platforms/windows/*.ps1 Docker-container script set one -# directory up, which assumes a container-baked $Env:UNITY_PATH and has no -# RUN_TESTS support at all. +# directory up, which assumes a container-baked $Env:UNITY_PATH. +# +# Note that test.ps1 in this directory is shared with that container set: +# entrypoint.ps1's RUN_TESTS branch dot-sources it directly rather than +# duplicating the test flow, since $Env:UNITY_PATH is precisely what +# resolve_unity_path.ps1's Get-UnityEditorRoot checks first. Keep it free +# of host-only assumptions. # # $PSScriptRoot is this script's own directory, so sibling steps are always # resolved correctly regardless of where dist/ was copied to - STEPS_DIR is diff --git a/dist/platforms/windows/steps/test.ps1 b/dist/platforms/windows/steps/test.ps1 index e99051d3..e3343f4d 100644 --- a/dist/platforms/windows/steps/test.ps1 +++ b/dist/platforms/windows/steps/test.ps1 @@ -1,5 +1,14 @@ -# Native Windows host-mode equivalent of ../../ubuntu/steps/test.sh - see -# runsteps.ps1's doc comment. +# Windows equivalent of ../../ubuntu/steps/test.sh - see runsteps.ps1's doc +# comment. +# +# Shared by BOTH Windows test paths, deliberately: HostRunner's native +# host mode (via runsteps.ps1) and the Docker container flow (via +# ../entrypoint.ps1's RUN_TESTS branch, which dot-sources this file at +# c:\steps\steps\test.ps1). Everything container-specific is already +# handled by environment: resolve_unity_path.ps1 returns the image-baked +# $Env:UNITY_PATH when set, and $TestRunnerActionDir below falls back to +# the c:\UnityTestRunnerAction mount. Do not add host-only assumptions +# here without giving the container an equivalent. # # Standalone sub-flow: the Linux version wraps the built standalone test # player in `xvfb-run` to give it a virtual X display. Windows has a real @@ -165,7 +174,21 @@ foreach ($Platform in $Platforms) { New-Item -ItemType Directory -Force -Path $EditorDir | Out-Null New-Item -ItemType Directory -Force -Path $PlayerDir | Out-Null - $TestRunnerActionDir = if ($Env:TEST_RUNNER_ACTION_DIR) { $Env:TEST_RUNNER_ACTION_DIR } else { Join-Path $Env:ACTION_FOLDER 'test-standalone-scripts' } + # Host mode (HostRunner) sets TEST_RUNNER_ACTION_DIR outright; the mac + # script set sets ACTION_FOLDER instead. In Docker mode neither is set, + # and Docker.getWindowsCommand mounts dist/test-standalone-scripts at + # c:\UnityTestRunnerAction - the Windows counterpart of the + # /UnityTestRunnerAction that ubuntu/steps/test.sh already defaults to. + $TestRunnerActionDir = + if ($Env:TEST_RUNNER_ACTION_DIR) { $Env:TEST_RUNNER_ACTION_DIR } + elseif ($Env:ACTION_FOLDER) { Join-Path $Env:ACTION_FOLDER 'test-standalone-scripts' } + else { 'c:\UnityTestRunnerAction' } + + if (-not (Test-Path $TestRunnerActionDir)) { + Write-Host "Standalone test scripts not found at `"$TestRunnerActionDir`". Set TEST_RUNNER_ACTION_DIR to the directory containing Assets\Editor and Assets\Player." + $global:TEST_RUNNER_EXIT_CODE = 1 + return + } Copy-Item -Path (Join-Path $TestRunnerActionDir 'Assets\Editor\*') -Destination $EditorDir -Recurse -Force Copy-Item -Path (Join-Path $TestRunnerActionDir 'Assets\Player\*') -Destination $PlayerDir -Recurse -Force diff --git a/src/command/test/unity-test-command.test.ts b/src/command/test/unity-test-command.test.ts index e3e5cfdf..12f7d906 100644 --- a/src/command/test/unity-test-command.test.ts +++ b/src/command/test/unity-test-command.test.ts @@ -113,4 +113,30 @@ describe('UnityTestCommand', () => { command.execute({ docker: true, hostPlatform: 'darwin', engineVersion: '2022.3.20f1' } as any), ).rejects.toThrow(/macOS/i); }); + + // Windows Docker test runs used to be rejected outright, because the + // container entrypoint.ps1 had no RUN_TESTS branch and would silently run + // a BUILD instead. It has one now (reusing the shared steps/test.ps1), so + // the flow is allowed through the same way Linux is. + it('--docker on Windows runs the batchmode flow instead of being rejected', async () => { + PlatformSetup.setup = mock(() => Promise.resolve()); + const dockerRunMock = mock(() => Promise.resolve()); + Docker.run = dockerRunMock; + + const command = new UnityTestCommand('test'); + const result = await command.execute({ + docker: true, + hostPlatform: 'win32', + hostOS: 'windows', + engineVersion: '2022.3.20f1', + } as any); + + expect(result).toBe(true); + expect(dockerRunMock).toHaveBeenCalledTimes(1); + const [image, options] = dockerRunMock.mock.calls[0] as unknown as [string, any]; + // Windows' own native Standalone target, which can only resolve to the + // windows-il2cpp module (see RunnerImageTag) - never the Linux one. + expect(image).toContain('windows-il2cpp'); + expect(options.runTests).toBe(true); + }); }); diff --git a/src/command/test/unity-test-command.ts b/src/command/test/unity-test-command.ts index b79a927c..b7330344 100644 --- a/src/command/test/unity-test-command.ts +++ b/src/command/test/unity-test-command.ts @@ -47,6 +47,10 @@ function defaultTestTargetPlatform(hostPlatform: string = process.platform): str * container, for self-hosted runners with Unity already installed (see * HostRunner) - mirrors orchestrator's own local (host) provider vs its * docker provider. + * + * Supported on Linux and Windows containers alike; macOS has no Unity + * Editor Docker images to run in, so it is rejected (use --local, or the + * default `unity test` CLI path, there). */ export class UnityTestCommand extends CommandBase implements CommandInterface { public async execute(options: Options): Promise { @@ -97,25 +101,24 @@ export class UnityTestCommand extends CommandBase implements CommandInterface { return true; } - // Docker (container) test mode is currently only wired up for Linux - // containers - dist/platforms/ubuntu/steps/test.sh + runsteps.sh's - // RUN_TESTS branch. Windows' entrypoint.ps1 (the unityci/editor - // Windows *container* image's entrypoint - see HostRunner's doc comment - // for why that's a different script set from HostRunner's own native - // dist/platforms/windows/steps/) doesn't know about RUN_TESTS yet (it - // always runs build.ps1), so running this on a Windows host today would - // silently attempt a BUILD instead of a test rather than failing - // loudly - reject it explicitly instead. macOS has no Unity Editor - // Docker images at all. Checked before PlatformSetup.setup runs, so - // this fails fast instead of after prompting for credentials. - if (hostPlatform !== 'linux') { + // Docker (container) test mode is wired up for Linux containers + // (dist/platforms/ubuntu/steps/test.sh, via runsteps.sh's RUN_TESTS + // branch) and for Windows containers (dist/platforms/windows/ + // entrypoint.ps1's own RUN_TESTS branch, which reuses steps/test.ps1 - + // container-safe because its resolve_unity_path.ps1 honours the + // image-baked $Env:UNITY_PATH). + // + // macOS is still rejected, and always will be for this flow: there are + // no Unity Editor Docker images for macOS at all, so there is nothing + // to run the container-side scripts in. Checked before + // PlatformSetup.setup runs, so this fails fast instead of after + // prompting for credentials. + if (hostPlatform === 'darwin') { throw new Error( - `--docker's classic batchmode test flow is currently only supported on Linux hosts/containers ` + - `(got hostPlatform=${hostPlatform}). ${ - hostPlatform === 'darwin' - ? 'No Unity Editor Docker images exist for macOS - omit --docker to use the native `unity test` CLI instead.' - : 'Windows Docker test support is tracked separately (the container-side scripts only handle builds so far).' - }`, + `--docker's classic batchmode test flow is not supported on macOS hosts ` + + `(got hostPlatform=${hostPlatform}). No Unity Editor Docker images exist for macOS - omit ` + + '--docker to use the native `unity test` CLI instead, or add --local to run the same batchmode ' + + 'flow directly against a locally installed Unity.', ); } diff --git a/src/model/docker.test.ts b/src/model/docker.test.ts index 45cf1ca5..d1532198 100644 --- a/src/model/docker.test.ts +++ b/src/model/docker.test.ts @@ -38,6 +38,32 @@ describe("Docker", () => { expect(validateBuildMock).not.toHaveBeenCalled(); }); + // Real bug (game-ci/unity-test-runner#310): a test run produces NUnit + // results, never a "# Build results #" section, so validateBuild turned a + // fully passing suite into "There was an error building the project". + it("skips build-output validation for test runs", async () => { + System.run = mock(() => + Promise.resolve({ output: '', error: "" }), + ); + const validateBuildMock = mock(() => {}); + UnityBuildValidation.validateBuild = validateBuildMock; + + await Docker.run("game-ci/unity-editor-stub:latest", { + hostOS: "linux", + hostPlatform: "linux", + currentWorkDir: "/home/runner/work/cli/cli", + homeDir: "/home/runner", + cliDistPath: "/home/runner/work/cli/cli/dist", + sshAgent: "", + gitPrivateToken: "", + dockerWorkspacePath: "/github/workspace", + engine: "unity", + runTests: true, + } as any); + + expect(validateBuildMock).not.toHaveBeenCalled(); + }); + it("still validates build output for real (non-activate-only) builds", async () => { System.run = mock(() => Promise.resolve({ output: "# Build results #\nErrors: 0\nSize:", error: "" })); const validateBuildMock = mock(() => {}); @@ -341,6 +367,80 @@ describe("Docker", () => { expect(command).not.toContain('"C:/Program Files/Microsoft Visual Studio"'); }); + // Regression test for a real bug: dist/test-standalone-scripts holds the + // Editor/Player helper scripts that --testPlatforms=standalone copies into + // the project, and ubuntu/steps/test.sh reads them from + // /UnityTestRunnerAction - but nothing ever mounted them there, so a + // standalone Docker test run died on `cp -R`. The original + // unity-test-runner action mounted the same directory; only the mount was + // lost in the port to this CLI. + it("mounts the standalone test helper scripts for a Linux test run", () => { + const command = (Docker as any).getLinuxCommand("game-ci/unity-editor-stub:latest", { + hostOS: "linux", + currentWorkDir: "/home/runner/work/cli/cli", + homeDir: "/home/runner", + cliDistPath: "/home/runner/work/cli/cli/dist", + sshAgent: "", + gitPrivateToken: "", + dockerWorkspacePath: "/github/workspace", + engine: "unity", + runTests: true, + }); + + expect(command).toContain( + '--volume "/home/runner/work/cli/cli/dist/test-standalone-scripts:/UnityTestRunnerAction:z"', + ); + }); + + it("does not mount the standalone test helper scripts for a plain Linux build", () => { + const command = (Docker as any).getLinuxCommand("game-ci/unity-editor-stub:latest", { + hostOS: "linux", + currentWorkDir: "/home/runner/work/cli/cli", + homeDir: "/home/runner", + cliDistPath: "/home/runner/work/cli/cli/dist", + sshAgent: "", + gitPrivateToken: "", + dockerWorkspacePath: "/github/workspace", + engine: "unity", + }); + + expect(command).not.toContain("UnityTestRunnerAction"); + }); + + it("mounts the standalone test helper scripts for a Windows test run", () => { + const command = (Docker as any).getWindowsCommand("game-ci/unity-editor-stub:latest", { + currentWorkDir: "C:/work/cli", + homeDir: "C:/Users/runner", + cliDistPath: "C:/work/cli/dist", + cliStoragePath: "C:/work/.game-ci", + unitySerial: "", + gitPrivateToken: "", + dockerWorkspacePath: "/github/workspace", + engine: "unity", + runTests: true, + }); + + expect(command).toContain('--volume="C:/work/cli/dist/test-standalone-scripts":"c:/UnityTestRunnerAction"'); + // The whole platforms/windows tree is mounted at c:/steps, which is what + // puts the shared steps/test.ps1 entrypoint.ps1 dot-sources in reach. + expect(command).toContain('--volume="C:/work/cli/dist/platforms/windows":"c:/steps"'); + }); + + it("does not mount the standalone test helper scripts for a plain Windows build", () => { + const command = (Docker as any).getWindowsCommand("game-ci/unity-editor-stub:latest", { + currentWorkDir: "C:/work/cli", + homeDir: "C:/Users/runner", + cliDistPath: "C:/work/cli/dist", + cliStoragePath: "C:/work/.game-ci", + unitySerial: "", + gitPrivateToken: "", + dockerWorkspacePath: "/github/workspace", + engine: "unity", + }); + + expect(command).not.toContain("UnityTestRunnerAction"); + }); + it.skip("runs", async () => { const image = "unity-builder:2019.2.11f1-webgl"; const parameters = { diff --git a/src/model/docker.ts b/src/model/docker.ts index df501e84..68ab6f5d 100644 --- a/src/model/docker.ts +++ b/src/model/docker.ts @@ -12,7 +12,7 @@ function engineEnvVars(options: Options) { class Docker { static async run(image: string, options: Options) { - const { hostPlatform, hostOS, engine, activateOnly } = options; + const { hostPlatform, hostOS, engine, activateOnly, runTests } = options; log.warning(`running docker process for ${hostOS} (${hostPlatform})`); @@ -40,9 +40,17 @@ class Docker { // build. An activate-only run never produces one - it was throwing // "There was an error building the project" on every successful // activation, because there's no build to validate in the first place. + // + // A test run (game-ci/unity-test-runner#310) has exactly the same + // shape and was missed by that fix: `game-ci test --docker` produces + // NUnit results, never a "# Build results #" section, so a fully + // passing suite ("result=Passed total=5 passed=5") was still being + // reported as `There was an error building the project`. Test + // outcomes are validated from the results XML by the caller, not from + // build-log scraping, so there is nothing for validateBuild to do here. switch (engine) { case "unity": - if (!activateOnly) { + if (!activateOnly && !runTests) { UnityBuildValidation.validateBuild(dockerRun.output); } break; @@ -84,6 +92,7 @@ class Docker { dockerMemoryLimit, dockerShmSize, engineLaunchWrapper, + runTests, } = options as Options & { commands?: string }; const home = homeDir; @@ -125,6 +134,16 @@ class Docker { isUnityDefaultFlow ? `--volume "${cliDistPath}/platforms/ubuntu/steps:/steps:z"` : "", isUnityDefaultFlow ? `--volume "${cliDistPath}/platforms/ubuntu/entrypoint.sh:/entrypoint.sh:z"` : "", isUnityDefaultFlow ? `--volume "${cliDistPath}/unity-config:/usr/share/unity3d/config:z"` : "", + // --testPlatforms=standalone copies these Editor/Player helper scripts + // into the project before building the standalone test player. Without + // this mount, test.sh's `cp -R "/UnityTestRunnerAction/Assets/..."` + // fails outright, so standalone was silently unrunnable in Docker mode. + // The original unity-test-runner action mounted the same directory (as + // /UnityStandaloneScripts) - only the mount was lost in the port to the + // CLI, not the scripts themselves. + isUnityDefaultFlow && runTests + ? `--volume "${cliDistPath}/test-standalone-scripts:/UnityTestRunnerAction:z"` + : "", sshAgent ? `--volume ${sshAgent}:/ssh-agent` : "", sshAgent && !sshPublicKeysDirectoryPath ? "--volume /home/runner/.ssh/known_hosts:/root/.ssh/known_hosts:ro" : "", sshPublicKeysDirectoryPath ? `--volume ${sshPublicKeysDirectoryPath}:/root/.ssh:ro` : "", @@ -151,6 +170,7 @@ class Docker { dockerShmSize, dockerIsolationMode, engineLaunchWrapper, + runTests, } = options as Options & { commands?: string }; // Same "don't force Unity's flow onto a non-Unity engine" fix as @@ -208,6 +228,12 @@ class Docker { isUnityDefaultFlow ? ` --volume="${cliDistPath}/platforms/windows":"c:/steps" \`` : "", isUnityDefaultFlow ? ` --volume="${cliDistPath}/BlankProject":"c:/BlankProject" \`` : "", isUnityDefaultFlow ? ` --volume="${cliDistPath}/unity-config":"c:/ProgramData/Unity/config" \`` : "", + // Windows counterpart of getLinuxCommand's own + // /UnityTestRunnerAction mount - see the comment there. Consumed by + // platforms/windows/steps/test.ps1's $TestRunnerActionDir fallback. + isUnityDefaultFlow && runTests + ? ` --volume="${cliDistPath}/test-standalone-scripts":"c:/UnityTestRunnerAction" \`` + : "", ` ${image} \``, isUnityDefaultFlow ? " powershell c:/steps/entrypoint.ps1" : ` ${wrappedCommands}`, ] diff --git a/src/model/host-runner.ts b/src/model/host-runner.ts index 56b28a15..49ef688f 100644 --- a/src/model/host-runner.ts +++ b/src/model/host-runner.ts @@ -25,12 +25,16 @@ import { path, fsSync as fs } from '../dependencies.ts'; * Windows note: dist/platforms/windows/*.ps1 (activate.ps1, build.ps1, * entrypoint.ps1, ...) are the *Docker container* scripts for * `unityci/editor` Windows images - they assume a baked-in $Env:UNITY_PATH - * and container-only setup, and have no RUN_TESTS support at all. This - * class instead uses a separate, genuinely native script set under - * dist/platforms/windows/steps/ (mirroring dist/platforms/ubuntu/steps/), - * which resolve Unity's install location dynamically from Unity Hub's - * default install directory (or $Env:UNITY_PATH if set) rather than - * assuming a container-baked path. + * and container-only setup. This class instead uses a separate, genuinely + * native script set under dist/platforms/windows/steps/ (mirroring + * dist/platforms/ubuntu/steps/), which resolve Unity's install location + * dynamically from Unity Hub's default install directory (or + * $Env:UNITY_PATH if set) rather than assuming a container-baked path. + * + * The two sets are not fully disjoint: entrypoint.ps1's RUN_TESTS branch + * deliberately reuses steps/test.ps1 rather than duplicating it, since + * $Env:UNITY_PATH is exactly the case Get-UnityEditorRoot already handles + * first. Only the build/activate halves remain genuinely separate. */ class HostRunner { private static buildEnv(options: Options): Record {