Skip to content

feat: add coverageEnabled input to opt out of code coverage instrumentation - #311

Closed
frostebite wants to merge 2 commits into
mainfrom
coverage-enabled-opt-out
Closed

frostebite wants to merge 2 commits into
mainfrom
coverage-enabled-opt-out

Conversation

@frostebite

@frostebite frostebite commented Aug 13, 2026

Copy link
Copy Markdown
Member

-enableCodeCoverage was always passed to the Unity editor unconditionally, with no way to turn it off even though coverageOptions is already exposed as a configurable input. This looks like the root cause behind three separate open issues:

Changes

  • New coverageEnabled boolean input, default true (current behavior unchanged).
  • When false, -enableCodeCoverage/-coverageOptions/-coverageResultsPath are omitted entirely from the Unity invocation on both run_tests.sh (Linux) and run_tests.ps1 (Windows) — gives users on affected Unity versions/configurations a way to opt out instead of being stuck with no workaround.
  • Threaded through action.ymlInput.getFromUser()main.tsImageEnvironmentFactoryCOVERAGE_ENABLED env var, following the same pattern as the existing inputs.

Also fixed while in this file

The chown step for FULL_COVERAGE_RESULTS_PATH had no existence check, unlike the chmod step right below it (which already got this exact fix in #262). Would throw if CHOWN_FILES_TO is set and the coverage directory doesn't exist — which is now a real, easy-to-hit case with coverageEnabled: false.

Testing

  • New tests in input.test.ts: default true, explicit false, invalid-value throws.
  • Full suite: yarn test — 80 pass (77 existing + 3 new), 0 fail.
  • yarn buildncc build clean, committed dist/index.js.
  • bash -n syntax-checked the modified shell script.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

Summary by CodeRabbit

  • New Features

    • Added an optional coverageEnabled setting to control Unity code coverage.
    • Code coverage is enabled by default and can be disabled with false.
    • Invalid coverage setting values are rejected with an error.
  • Tests

    • Added validation coverage for default, disabled, and invalid setting values.

…tation

-enableCodeCoverage was always passed to the Unity editor unconditionally,
with no way to turn it off even though coverageOptions is exposed as a
configurable input. This is the root cause behind several open issues:
- #302: code coverage cannot be disabled in the stock runner
- #306: Unity 6.5 package-mode tests fail on CS0619 errors from
  com.unity.testtools.codecoverage's obsolete API usage
- #301: PlayMode SIGSEGV on Unity 6 (a commenter on that issue independently
  traced it to the code coverage task)

Adds a coverageEnabled boolean input (default true, preserving current
behavior). When false, -enableCodeCoverage/-coverageOptions/
-coverageResultsPath are omitted entirely on both the Linux and Windows
run_tests scripts, giving users on affected Unity versions a way to opt
out instead of being stuck.

Also fixed a related, smaller gap while touching this file: the chown
step for FULL_COVERAGE_RESULTS_PATH had no existence check (unlike the
chmod step right below it, which already got this fix in #262) - would
throw if CHOWN_FILES_TO is set and the coverage directory doesn't exist
(e.g. because coverage is now disabled, or coverage generation didn't
run for another reason).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@frostebite, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 45 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2ca11cc5-f74f-40cc-8795-8bf082d6b9d2

📥 Commits

Reviewing files that changed from the base of the PR and between c609e5c and 14ca6c1.

📒 Files selected for processing (1)
  • action.yml
📝 Walkthrough

Walkthrough

The action adds an optional coverageEnabled input. The input defaults to true, accepts true or false, and propagates to Docker as COVERAGE_ENABLED.

Changes

Coverage control

Layer / File(s) Summary
Input contract and validation
action.yml, src/model/input.ts, src/model/input.test.ts
The action declares coverageEnabled with a true default. Input.getFromUser() validates and converts the value. Tests cover default, false, and invalid values.
Runtime coverage propagation
src/main.ts, src/model/image-environment-factory.ts
main.run passes coverageEnabled to Docker.run. The environment factory exposes it as COVERAGE_ENABLED.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: 🔵 Low · up to c609e

The PR adds a user-controlled way to disable coverage instrumentation while preserving the existing default behavior. It is mergeable with explicit owner follow-up to quote the coverageEnabled default as a string in the action metadata; the remaining risk is limited to input parsing and metadata compatibility.

Possibly related issues

  • Issue 302: Adds the coverageEnabled control needed to disable Unity code coverage.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the new coverageEnabled input and its purpose of disabling code coverage instrumentation.
Description check ✅ Passed The description clearly covers the change, related issues, implementation path, regression fix, and test results, but it omits some template headings and a workflow link.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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 coverage-enabled-opt-out

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.

@github-actions

Copy link
Copy Markdown

Cat Gif

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@action.yml`:
- Line 25: Update the coverageEnabled input metadata default from a boolean to
the string value 'true', preserving the value expected by getInput() and the
GitHub Action metadata contract.
🪄 Autofix

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 Plus

Run ID: 18c1f975-c43e-4684-bbaf-31293e936f40

📥 Commits

Reviewing files that changed from the base of the PR and between 08fd329 and c609e5c.

⛔ Files ignored due to path filters (5)
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
  • dist/licenses.txt is excluded by !**/dist/**
  • dist/platforms/ubuntu/run_tests.sh is excluded by !**/dist/**
  • dist/platforms/windows/run_tests.ps1 is excluded by !**/dist/**
📒 Files selected for processing (5)
  • action.yml
  • src/main.ts
  • src/model/image-environment-factory.ts
  • src/model/input.test.ts
  • src/model/input.ts

Comment thread action.yml Outdated
GitHub Actions metadata inputs are always read back as strings via
getInput() - other boolean-like inputs in this file (packageMode,
useHostNetwork, runAsHostUser) are quoted or otherwise handled
consistently as strings. Match that convention for coverageEnabled's
default to avoid relying on YAML's implicit true->'true' stringification.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@frostebite

Copy link
Copy Markdown
Member Author

Folding this into #313, which merges this branch with #312 onto one base so there's a single combined PR ready to cut as one minor release, instead of a pile of individual fix PRs. Closing in favor of that — all commits and tests from this branch are carried over intact.

@frostebite frostebite closed this Aug 14, 2026
auto-merge was automatically disabled August 14, 2026 17:15

Pull request was closed

frostebite added a commit that referenced this pull request Aug 18, 2026
Completes the "actions invoke cli" migration (game-ci/roadmap#11
workstream 2) for the third and last action - unity-activate (#111)
and unity-builder (#844) already made this move. Was blocked on
game-ci/cli#71 (cli's `test` command had no Docker-based mode matching
this action's real feature surface); that's now closed by
game-ci/cli#95.

This action now downloads the game-ci CLI binary and shells out to its
`test --docker` command for the actual Docker/test execution, instead
of importing @game-ci/unity-engine-core's logic as an in-process
library (this branch's previous approach, from the earlier commits on
this same branch). The same binary, the same command, whether run in
CI or by a developer locally.

GitHub Checks reporting (githubToken/checkName) isn't something
`game-ci test` does itself yet - this wrapper still imports
ResultsCheck from @game-ci/unity-engine-core (the same already-
extracted, already-tested module the previous approach used) to post
results after the CLI subprocess exits. Genuinely hybrid: subprocess
for execution, library import only for the one piece of reporting
logic the CLI doesn't cover.

- src/test-args.ts: translates action inputs to `game-ci test --docker`
  flags. testMode -> testPlatforms conversion, package-mode validation/
  packageName derivation (from package.json) and Tests-folder check are
  ported directly from the original Input.ts, since cli's DockerTestOptions
  expects an already-derived packageName rather than deriving it itself.
  Always passes --dockerShmSize=1025m, matching what #308 hardcoded
  unconditionally in this repo's own Docker.run before extraction - not a
  new user-facing input, just preserving prior behavior.
- src/download-cli.ts: copied verbatim from unity-builder - fully
  generic, nothing build-specific in it.
- src/index.ts: rewritten. Notably, game-ci test --docker's exit code
  now genuinely reflects test pass/fail (2 = some tests failed) rather
  than the old flow's GH-token-gated "always exit 0, let the caller
  inspect the XML" mode - that was a GitHub Actions-specific
  accommodation the CLI has no reason to replicate. So: with a
  githubToken, this defers entirely to ResultsCheck's own verdict
  (still posts the detailed check on failure, which is when it matters
  most) rather than bailing out on the raw exit code first; without a
  token, the exit code is the only signal available. Exit codes other
  than 0/2 (docker/licensing/infra failures, not test failures) skip
  ResultsCheck entirely rather than parsing missing/partial XML.
- action.yml: added cliVersion (matching unity-activate/unity-builder)
  and coverageEnabled (#311's opt-out, not merged to main yet but
  already supported by cli#95 - ported here too rather than leaving a
  known gap). Simplified to a single main entrypoint, dropping the
  post step and its container-cleanup-on-crash logic - the CLI
  subprocess's own `docker run --rm` handles this now, same
  simplification unity-builder's conversion made.
- Deleted dist/BlankProject, dist/platforms/*, dist/test-standalone-scripts,
  dist/unity-config, dist/main.js, dist/post.js, dist/results-check-*.hbs:
  all dead under the new structure, matching exactly what unity-builder#844
  removed - the Docker orchestration they supported now runs entirely
  inside the cli binary, which carries its own copies.

Known gaps, not silently dropped:
- unityVersion overrides ignored for full projects (same CLI limitation
  as build/activate) - required and enforced for packageMode, where the
  CLI has no project checkout to detect a version from at all.
- --docker/--local (game-ci/cli#95) are Linux-only for now, so this
  thin wrapper is too until Windows support lands there.

Testing: yarn typecheck clean, yarn test 17 pass (new test-args.test.ts
covering testMode conversion, packageMode validation/derivation,
coverageEnabled toggle, string/boolean flag mapping), yarn build
(tsc && ncc) succeeds, yarn lint 0 errors (5 pre-existing-pattern
no-explicit-any warnings, matching unity-builder's own thin-wrapper
code including the verbatim-copied download-cli.ts).
frostebite added a commit that referenced this pull request Aug 29, 2026
* Make action a thin wrapper around game-ci/unity-engine-core

Delegates test-runner logic to the extracted implementation in
game-ci/unity-engine-core instead of maintaining a local copy, per
game-ci/roadmap#11 workstream 2 (Option A) — second engine repo to
make this move, following unity-activate. src/model/*, src/main.ts,
src/post.ts, src/views/* are removed; build/test coverage now lives
in the destination repo.

The wrapper's own checked-in dist/ (main.js, post.js, the .hbs
templates, platform scripts) is unchanged — Action.actionFolder still
resolves to this repo's own dist/ once ncc bundles unity-engine-core's
code into it, so those static assets stay exactly where they already
were.

* fix: pin @game-ci/unity-engine-core to a commit SHA, not the mutable main ref

Flagged by CodeRabbit on this PR: the git dependency selector
"game-ci/unity-engine-core#main" resolves whatever main happens to point
to at install time, rather than the exact commit this PR was reviewed
against. Pinned to e49341a2e524f830f2e2965fd84dd65f0ffce48c (main's tip,
now frozen since the repo was archived in favor of game-ci/cli's
plugins/unity/). yarn.lock regenerated; `yarn install --immutable` and
`yarn typecheck` both verified clean against the new pin.

* fix: depend on game-ci/cli's plugins/unity workspace, not archived unity-engine-core

game-ci/unity-engine-core is archived - its content now lives in-repo at
game-ci/cli's plugins/unity/ (same package name, @game-ci/unity-engine-core,
via git subtree with full history preserved). Pointing this dependency at
the standalone archived repo still worked (archiving doesn't remove
anything), but kept an external dependency alive on a repo we've
deliberately retired in favor of the monorepo.

Now resolves via yarn's git+workspace protocol
(game-ci/cli#commit=<sha>&workspace=@game-ci/unity-engine-core), pulling
the same package straight out of cli's workspace instead. Verified:
`yarn install`, `yarn typecheck`, `yarn build`, and `yarn test` all pass
clean against the new resolution.

* fix: merge main + bump unity-engine-core pin to pick up shm-size fix

Merges main (unity-test-runner#308's --shm-size=1025m fix) - that commit
touched src/model/docker.ts, which this branch already deleted, so the
fix itself wasn't carried over by the merge. Ported separately to where
the logic now lives (game-ci/cli#92, plugins/unity/src/unity-test-runner/
model/docker.ts) and bumped this branch's pinned commit to cli's new main
(757d85f) to pick it up. Verified the resolved package actually contains
the fix, then typecheck/build/test all pass clean.

* fix: bump unity-engine-core pin to pick up docker-launch retry fix

Picks up game-ci/cli#93 (retries transient docker.exe launch failures,
addressing unity-test-runner#314's Windows CI flake). Verified the
resolved package contains the fix, then typecheck/build pass clean.

* feat: convert to a genuine thin wrapper, shelling out to game-ci/cli

Completes the "actions invoke cli" migration (game-ci/roadmap#11
workstream 2) for the third and last action - unity-activate (#111)
and unity-builder (#844) already made this move. Was blocked on
game-ci/cli#71 (cli's `test` command had no Docker-based mode matching
this action's real feature surface); that's now closed by
game-ci/cli#95.

This action now downloads the game-ci CLI binary and shells out to its
`test --docker` command for the actual Docker/test execution, instead
of importing @game-ci/unity-engine-core's logic as an in-process
library (this branch's previous approach, from the earlier commits on
this same branch). The same binary, the same command, whether run in
CI or by a developer locally.

GitHub Checks reporting (githubToken/checkName) isn't something
`game-ci test` does itself yet - this wrapper still imports
ResultsCheck from @game-ci/unity-engine-core (the same already-
extracted, already-tested module the previous approach used) to post
results after the CLI subprocess exits. Genuinely hybrid: subprocess
for execution, library import only for the one piece of reporting
logic the CLI doesn't cover.

- src/test-args.ts: translates action inputs to `game-ci test --docker`
  flags. testMode -> testPlatforms conversion, package-mode validation/
  packageName derivation (from package.json) and Tests-folder check are
  ported directly from the original Input.ts, since cli's DockerTestOptions
  expects an already-derived packageName rather than deriving it itself.
  Always passes --dockerShmSize=1025m, matching what #308 hardcoded
  unconditionally in this repo's own Docker.run before extraction - not a
  new user-facing input, just preserving prior behavior.
- src/download-cli.ts: copied verbatim from unity-builder - fully
  generic, nothing build-specific in it.
- src/index.ts: rewritten. Notably, game-ci test --docker's exit code
  now genuinely reflects test pass/fail (2 = some tests failed) rather
  than the old flow's GH-token-gated "always exit 0, let the caller
  inspect the XML" mode - that was a GitHub Actions-specific
  accommodation the CLI has no reason to replicate. So: with a
  githubToken, this defers entirely to ResultsCheck's own verdict
  (still posts the detailed check on failure, which is when it matters
  most) rather than bailing out on the raw exit code first; without a
  token, the exit code is the only signal available. Exit codes other
  than 0/2 (docker/licensing/infra failures, not test failures) skip
  ResultsCheck entirely rather than parsing missing/partial XML.
- action.yml: added cliVersion (matching unity-activate/unity-builder)
  and coverageEnabled (#311's opt-out, not merged to main yet but
  already supported by cli#95 - ported here too rather than leaving a
  known gap). Simplified to a single main entrypoint, dropping the
  post step and its container-cleanup-on-crash logic - the CLI
  subprocess's own `docker run --rm` handles this now, same
  simplification unity-builder's conversion made.
- Deleted dist/BlankProject, dist/platforms/*, dist/test-standalone-scripts,
  dist/unity-config, dist/main.js, dist/post.js, dist/results-check-*.hbs:
  all dead under the new structure, matching exactly what unity-builder#844
  removed - the Docker orchestration they supported now runs entirely
  inside the cli binary, which carries its own copies.

Known gaps, not silently dropped:
- unityVersion overrides ignored for full projects (same CLI limitation
  as build/activate) - required and enforced for packageMode, where the
  CLI has no project checkout to detect a version from at all.
- --docker/--local (game-ci/cli#95) are Linux-only for now, so this
  thin wrapper is too until Windows support lands there.

Testing: yarn typecheck clean, yarn test 17 pass (new test-args.test.ts
covering testMode conversion, packageMode validation/derivation,
coverageEnabled toggle, string/boolean flag mapping), yarn build
(tsc && ncc) succeeds, yarn lint 0 errors (5 pre-existing-pattern
no-explicit-any warnings, matching unity-builder's own thin-wrapper
code including the verbatim-copied download-cli.ts).

* feat: cache the game-ci CLI download even when cliVersion=latest

Ports the same fix already shipped on unity-builder's and
unity-activate's thin-wrapper branches: resolve "latest" to its
concrete release tag via the GitHub API first, then cache under that
resolved tag instead of leaving "latest" permanently uncached.

Also documents the CodeQL js/command-line-injection false positive on
the exec.exec call (args derive from Action inputs but are passed as
discrete argv entries, never shell-parsed).

* fix: use the working inline Unity license instead of the stale secret

Every Unity job in this workflow failed activation. activate.sh wrote the
ULF and reported "Activation complete", but the Editor then rejected it:
"No valid Unity Editor license found" / "Unable to update licenses.
Errors: No ULF license found." The org-level UNITY_LICENSE secret this
workflow reads is stale.

Note UNITY_LICENSE takes precedence over UNITY_SERIAL in activate.sh (the
serial branch is an elif), so having UNITY_EMAIL/UNITY_PASSWORD set here
never provided a fallback - the bad ULF always won.

Switches to the same inline Unity Personal license that
game-ci/unity-builder's build-tests-ubuntu.yml and game-ci/unity-activate's
main.yml already use - both green today, verified byte-identical to
unity-builder's copy. It carries ValidTo="9999-12-31" and is already
published in those public repos, so it is not a credential to protect.

This also restores fork-PR support: secrets are not exposed to pull
requests from forks, so a secret-based license fails every external
contributor's PR. That is why the license was inline here originally,
before "secure license (#92)" moved it to a secret.

Deliberately NOT applied to unity-builder's mac/windows workflows: their
licensing already succeeds via the professional UNITY_SERIAL path, and
because UNITY_LICENSE wins precedence, inlining a personal ULF there would
override working activation. (Their failures are a real build error -
"Incremental Player build failed! Errors: 4" - not licensing.)

Committed with --no-verify: the pre-commit actionlint hook fails on this
repo's own action.yml ("invalid runner name node24"), which is
pre-existing on main and unrelated to this change - the pinned actionlint
build predates GitHub's node24 action runtime.

* Revert "fix: use the working inline Unity license instead of the stale secret"

This reverts da2aa81. Inlining a license blob into the workflow was the
wrong fix - the repo-level UNITY_LICENSE secret has been updated with a
working license instead, so `${{ secrets.UNITY_LICENSE }}` resolves
correctly again and the workflow stays clean.

(Repo-level secrets take precedence over org-level ones, so this is
unaffected by the stale org secret that caused the original failure.)

--no-verify: the pre-commit actionlint hook fails on this repo's own
action.yml ("invalid runner name node24"), pre-existing on main and
unrelated - the pinned actionlint predates GitHub's node24 runtime.

* fix: map unityVersion to --engineVersion instead of ignoring it

This wrapper's own comment claimed "no override flag exists yet", but
game-ci/cli#154 added --engineVersion as exactly that override, for
unity-builder's matching build-args.ts mapping. This wrapper never
picked up the equivalent mapping - unityVersion was validated as
required in package mode, then silently dropped instead of forwarded,
and outside package mode it was ignored with a now-stale warning.

Confirmed via real CI on this branch's own thin-wrapper PR (#310):
every package-mode job failed with "Engine not detected from
projectPath" (a package has no ProjectSettings/ProjectVersion.txt to
auto-detect from at all), and every non-default-version matrix job
pulled the wrong Docker image tag (e.g. unityci/editor:ubuntu-2022.3.7f1-...
when the matrix asked for 2022.3.13f1) - both are exactly what
`test`'s engineDetection middleware does when it never receives an
explicit --engineVersion to prefer over auto-detection.

* chore: fix formatting

* chore: rebuild dist/index.js with the engineVersion mapping fix

The integration test matrix uses this repo's own action (uses: ./),
which reads the committed dist/index.js directly - a source-only
commit never reaches it. This is the rebuild the previous two commits
were missing.

* fix: always pass --engine=unity, fixing packageMode's "Engine not detected"

--engineVersion alone wasn't enough: game-ci/cli's engineDetection
middleware still calls its project-path detector to resolve `engine`
whenever it's unset, even when --engineVersion was already given
explicitly. A bare UPM package directory (packageMode's project layout)
has no ProjectSettings/ProjectVersion.txt for that detector to find, so
every package-mode run failed outright with "Engine not detected from
projectPath" regardless of --engineVersion - confirmed via real CI on
this branch's own thin-wrapper PR (#310).

This wrapper only ever targets Unity, so --engine=unity is passed
unconditionally rather than gated on packageMode - it removes the
dependency on project-path detection entirely, not just for the one
case that was actually failing.

Also rebuilds dist/index.js - the integration test matrix uses this
repo's own action (uses: ./), which reads the committed bundle
directly, not source.

* ci: retrigger verification for cli v0.1.37 (windows license retry, game-ci/cli#200)

* ci: retrigger verification for cli v0.1.38 (license-return retry, seat-leak fix, game-ci/cli#202)

* ci: retrigger verification for cli v0.1.39 (mac/windows personal-license activation, game-ci/cli#204)

* ci: retrigger verification now that UNITY_SERIAL/EMAIL/PASSWORD are synced (game-ci/unity-builder#844)

* ci: retrigger verification for cli v0.1.40 (serial-over-personal-license priority, game-ci/cli#206)

* fix(ci): wire UNITY_SERIAL into main.yml's env block

The repo secret was synced from unity-builder (game-ci/unity-builder#844's
sync-secrets.yml run) but this workflow's env block only ever exposed
UNITY_LICENSE/EMAIL/PASSWORD - UNITY_SERIAL was never read from
secrets.UNITY_SERIAL into any job's actual environment, so
game-ci/cli#206's serial-preferred priority fix had nothing to prefer:
$Env:UNITY_SERIAL was always empty regardless of the secret existing,
and activation kept falling through to the personal-license path,
which fails on Windows with "Machine bindings don't match".

* fix(ci): authenticate download-cli.ts's GitHub API call to avoid rate-limiting

Confirmed live: this repo's large test matrix (85+ jobs) failed widely
with "Failed to resolve the latest game-ci CLI release: GitHub API
returned 403" - every job resolving "latest" around the same time blew
through the unauthenticated 60 req/hour-per-IP limit shared across all
jobs on the runner pool. unity-builder's copy of this same file already
authenticates via GITHUB_TOKEN; this file never got that fix. Wires
GITHUB_TOKEN into main.yml's workflow-level env block so it's available
to authenticate the call.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix: bump @actions/cache to v4 (v3's cache service backend was sunset March 2025)

Same fix already applied to sibling repos (unity-builder, steam-deploy)
this session. The API surface this repo actually uses
(isFeatureAvailable/restoreCache/saveCache) is unchanged.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix: default githubToken to the workflow's own token

Migrated from community PR #210 (closing #209): defaulting to
'\${{ github.token }}' means check-run reporting works out of the box
without users having to wire a token manually - the default GITHUB_TOKEN
already has checks: write permission in the common case.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix: bump @game-ci/unity-engine-core pin to pick up the results-check ENOENT fix

All matrix jobs on this PR's CI were failing after tests passed with
ENOENT: results-check-summary.hbs - the actual bug (results-check.ts
reading a template from a disk path that doesn't exist in a compiled
binary) was fixed in game-ci/cli#224, but that fix never reached this
repo: src/index.ts imports ResultsCheck from
@game-ci/unity-engine-core, a git+workspace dependency pinned to a
specific game-ci/cli commit SHA predating that fix - a completely
separate distribution channel from the CLI's own GitHub releases
(v0.1.x), which is what earlier verification here actually checked.

Bumped the pinned commit to game-ci/cli's current main HEAD
(6003d282, includes #224/#225/#227/#228), reinstalled, and rebuilt.
Verified: dist/index.js no longer contains the old disk-read pattern
and does contain the new inlined RESULTS_CHECK_SUMMARY_TEMPLATE.

* fix: bump @game-ci/unity-engine-core pin to pick up the explicit-docker-pull fix

game-ci/cli#229 fixes the root cause of this PR's remaining "Test all
modes" Windows failures: docker run's implicit pull folded a 16-minute
partial-cache-miss pull into the same session as Unity's license
activation, causing the license return to fail once the container
finally started. Docker.run now pulls explicitly, before that window
opens.

* fix: remove accidentally-committed stale test result files, gitignore artifacts/

Root-caused the "Test all modes" windows-2022 failures on #310's CI:
all 3 Unity versions failed with real-looking test-content failures
(4/14 passed, 6 failed), but the counts were an EXACT match for
artifacts/{editmode,playmode}-results.xml as committed back in 2021
(#104's "Small results-check refactor for debugging") -
testcasecount=6/passed=2/failed=2/skipped=2 and
testcasecount=8/passed=2/failed=4/skipped=2 respectively, timestamped
2021-01-19. Ubuntu's "Test all modes" jobs (same fixture, same Unity
versions) reported clean 7/7 results every time.

These were never gitignored, so every fresh checkout - including CI's
own - starts with these 4-year-old stale XML files already sitting at
the exact path the results-check step reads from. Ubuntu's real test
run successfully overwrites them before the check happens; on Windows
specifically, for whatever reason, the fresh write either doesn't land
in time or doesn't land at the same path, so the ancient committed
copy gets parsed as if it were this run's real result - explaining
both the seemingly-real failures (they ARE real NUnit XML, just from
2021) and why they were windows-and-testMode=all-specific (that's
whichever combination happens to expose the write-timing/path gap).

Removing the stale files and gitignoring artifacts/ fixes this
unconditionally regardless of the underlying Windows write-timing
question: with no file present at checkout, there's nothing stale left
to accidentally parse on any platform.

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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