Skip to content

Feature/unity unit tests#239

Merged
norrietaylor merged 19 commits intomainfrom
feature/unity-unit-tests
Mar 29, 2026
Merged

Feature/unity unit tests#239
norrietaylor merged 19 commits intomainfrom
feature/unity-unit-tests

Conversation

@norrietaylor
Copy link
Copy Markdown
Owner

No description provided.

norrietaylor and others added 19 commits March 27, 2026 19:31
- Add Assembly-CSharp reference to TT2.Tests.EditMode.asmdef so tests can access production scripts
- Remove PlaceholderTest.cs scaffold (no longer needed)
- Add game-ci/unity-test-runner@v4 editmode job to unity-build.yml before build matrix, with artifact upload and same fork-skip/secrets/cache as build job

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Creates Enemies/ test folder with 9 NUnit tests covering all state
machine lifecycle scenarios: initial null state, ChangeState/Enter/Exit
ordering, Update/Execute delegation, null-state safety, and ChangeState(null).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
16 EditMode tests covering all 6 concrete enemy states (Idle, Patrol,
Chase, Attack, Stunned, Defeated) using a StubEnemyBase that creates
wired EnemyBase instances without scene dependencies. Uses reflection
for timer manipulation since Time.deltaTime is zero in EditMode.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove 200 agentic workflow files (.lock.yml, .md specs, shared/, aw/),
agentics-maintenance.yml, and the dead .gitattributes rule. Update
CLAUDE.md with architecture docs, build/test commands, and branch conventions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add needs: test to the build job so test failures block builds.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Unity 2022.3 crashes during domain reload after tests complete with a
Baselib semaphore assertion failure (exit code 133). Tests themselves
pass but the exit code fails the job.

Fix: add continue-on-error to the test runner step, then verify
pass/fail by parsing the XML results file. Add githubToken for
check annotations and checks:write permission.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The Baselib semaphore crash (exit 133) was caused by GPU/GLX
initialization in the headless Docker container. Add -nographics
custom parameter to skip GPU setup entirely. Remove the fragile
XML-parsing verify step — let the test runner action handle
pass/fail directly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Sharp

With overrideReferences: true, the test assembly could not see
Assembly-CSharp (production code) since it is not a precompiled DLL.
Set overrideReferences: false so Unity automatically resolves both
Assembly-CSharp and nunit.framework via the test runner references.
Add UNITY_INCLUDE_TESTS define constraint.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Unity assemblies defined via .asmdef cannot reference the default
Assembly-CSharp. Create TT2.Runtime.asmdef for production scripts
and update the test assembly to reference TT2.Runtime explicitly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HUDController.cs uses TMPro which requires Unity.TextMeshPro
assembly reference in TT2.Runtime.asmdef.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…mode

In Unity batch mode, AddComponent<EnemyBase>() fails because Unity
cannot auto-add the abstract Collider2D required by [RequireComponent].
Manually add Rigidbody2D and BoxCollider2D before EnemyBase.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… mode

In batch mode EditMode tests, Awake() may not fire or may run before
components are registered, leaving _rigidbody, _collider, and
_stateMachine null. Explicitly set these fields via reflection after
construction to ensure tests work regardless of Awake() timing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
DefeatedState.Enter() calls Object.Destroy() which is not supported
in EditMode and logs an error. Unity Test Framework treats unexpected
log errors as test failures. Use LogAssert.ignoreFailingMessages to
suppress during the Enter() call.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add scripts/preflight.sh: catches missing .meta files, GUID dupes,
  500-line cap, bad namespace imports, missing asmdef refs, and
  unprotected Object.Destroy in tests — runs in ~1s without Unity
- Add preflight job to CI that runs before test job for fast failure
- Cache Docker image between CI runs to avoid ~60s image pull
- Fix duplicate GUID in TT2.Runtime.asmdef.meta
- Drop fetch-depth: 0 (full history not needed)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ew failures

Instruct agents to run scripts/preflight.sh before every push and to
add new checks to the script when CI fails with a novel error class.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a fast “preflight” validation layer to catch common Unity/CI issues earlier, and wires it into the GitHub Actions pipeline (plus documents the workflow for contributors).

Changes:

  • Added scripts/preflight.sh to perform Unity repo sanity checks without requiring Unity installed.
  • Updated CI workflow to run preflight before EditMode tests and added an attempt at Docker image caching for the test job.
  • Expanded CLAUDE.md with local testing + CI pipeline + preflight guidance and a pre-commit checklist.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
scripts/preflight.sh New local/CI smoke-check script for Unity meta/GUID/size/import/asmdef/test-safety rules.
CLAUDE.md Documentation updates describing test running, CI stages, preflight expectations, and contributor rules.
.github/workflows/unity-build.yml Adds a preflight job before tests; introduces Docker image caching steps for the test job.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 176 to 214
---

## Branch & PR Conventions

- Branch naming: `feature/<desc>`, `fix/<desc>`, `chore/<desc>`
- PR titles: imperative mood — "Add X" not "Added X"
- One approving review required before merge

---

## Pre-Commit Checklist

1. **Run `./scripts/preflight.sh`** — must pass before pushing
2. All new `.cs` files are in `Unity/Assets/Scripts/<domain>/`
3. Every new `.cs` file has a `.cs.meta` with unique GUID
4. Every new folder has a `.meta` file in its parent directory
5. All scripts use correct `TaekwondoTech.*` namespace
6. No file exceeds 500 lines
7. 4-space indent, LF line endings
8. Any `IDamageable` implementation includes all required members
9. New `using` directives for external packages (e.g., `TMPro`) must have a matching reference in `Unity/Assets/Scripts/TT2.Runtime.asmdef`
10. EditMode tests calling production code that uses `Object.Destroy()` must wrap the call with `LogAssert.ignoreFailingMessages = true/false`

**If CI fails with a new class of error**, add a corresponding check to `scripts/preflight.sh` before fixing the code, so the same error is caught locally in future.

---

## Branch & PR Conventions

- Branch naming: `feature/<desc>`, `fix/<desc>`, `chore/<desc>`
- PR titles: imperative mood — "Add X" not "Added X"
- One approving review required before merge

---

## Pre-Commit Checklist

- [ ] All new `.cs` files are in `Unity/Assets/Scripts/<domain>/`
- [ ] Every new `.cs` file has a `.cs.meta` with unique GUID
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

The “Branch & PR Conventions” and “Pre-Commit Checklist” sections are duplicated later in the file (the headings and checklists appear twice). Please remove the duplicate copy and keep a single checklist/conventions section to avoid conflicting updates over time.

Copilot uses AI. Check for mistakes.
fail "Missing folder .meta: $dir_meta"
MISSING_FOLDER_META=$((MISSING_FOLDER_META + 1))
fi
done < <(git diff --name-only --diff-filter=A HEAD 2>/dev/null | xargs -I{} dirname {} | sort -u | grep "^Unity/Assets")
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

This check claims to validate “New folders (staged or untracked)”, but git diff --name-only --diff-filter=A HEAD will not include untracked files/folders. If you want to cover untracked additions too, incorporate git status --porcelain (or similar) and handle both staged and untracked paths; otherwise, adjust the comment/output to match the actual behavior.

Suggested change
done < <(git diff --name-only --diff-filter=A HEAD 2>/dev/null | xargs -I{} dirname {} | sort -u | grep "^Unity/Assets")
done < <({ git diff --name-only --diff-filter=A HEAD 2>/dev/null; git ls-files --others --exclude-standard 2>/dev/null; } | xargs -I{} dirname {} | sort -u | grep "^Unity/Assets")

Copilot uses AI. Check for mistakes.
Comment on lines +62 to +71
# 3. No duplicate GUIDs among new/changed .meta files
echo ""
echo "--- GUID uniqueness (new/changed files) ---"
GUID_ERRORS=0
for meta_file in $(git diff --name-only HEAD 2>/dev/null | grep '\.meta$' || true); do
[ ! -f "$meta_file" ] && continue
guid=$(grep "^guid:" "$meta_file" 2>/dev/null | head -1 | awk '{print $2}')
[ -z "$guid" ] && continue
matches=$(find Unity/Assets -name "*.meta" -exec grep -l "^guid: $guid" {} + 2>/dev/null | wc -l)
if [ "$matches" -gt 1 ]; then
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

These checks rely on git diff ... HEAD to detect “new/changed” files. In CI the working tree is typically clean, so git diff HEAD is empty and the folder-meta/GUID-collision checks won’t run against PR changes. Consider diffing against the PR base (e.g., using GITHUB_BASE_REF / merge-base) or accepting a base ref/commit as an argument so the script can validate PR deltas in CI as well as local runs.

Copilot uses AI. Check for mistakes.
contents: read
steps:
- name: Checkout repository
uses: actions/checkout@v4
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

The preflight job checks out with default depth and then runs scripts/preflight.sh, but the script’s “new/changed file” checks are driven by git diff against a base. To make CI preflight actually evaluate PR deltas, update this checkout to fetch enough history/refs (e.g., fetch-depth: 0 and/or fetch the PR base ref) and/or pass the base ref/commit to the script so it can diff against the PR base instead of relying on a clean working tree.

Suggested change
uses: actions/checkout@v4
uses: actions/checkout@v4
with:
fetch-depth: 0

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +34
There is no CLI build system. All building and testing happens through Unity Editor or CI.

**Running tests locally:** Unity Editor → Window → General → Test Runner → EditMode tab

Tests use Unity Test Framework (NUnit) at `Unity/Assets/Tests/EditMode/`. The test assembly (`TT2.Tests.EditMode`) uses namespace `TaekwondoTech.Tests.EditMode` and references `TT2.Runtime` (production code).

**Preflight checks (run before every push):**
```
./scripts/preflight.sh
```
This catches most CI failures locally in ~1 second: missing `.meta` files, duplicate GUIDs, 500-line violations, bad namespace imports, missing asmdef package references, and unprotected `Object.Destroy()` in EditMode tests. **Always run this before pushing.** If you encounter a new class of CI failure not caught by preflight, add a check for it to the script so future runs catch it early.

**CI pipeline** runs automatically on pushes to `main` and PRs (not forks). Three jobs in order:
1. **Preflight** (~10s) — runs `scripts/preflight.sh`
2. **EditMode Tests** (~3-4min) — `game-ci/unity-test-runner@v4` with `-nographics`
3. **Builds** (~4min each) — WebGL, iOS, Android in parallel via `game-ci/unity-builder`

Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

This document now contains duplicated sections (e.g., “Build & Test Commands” and the CI pipeline description appear here and then repeat again later in the file). This makes the guidance ambiguous and harder to maintain—please consolidate into a single canonical section and remove the duplicated block(s).

Suggested change
There is no CLI build system. All building and testing happens through Unity Editor or CI.
**Running tests locally:** Unity Editor → Window → General → Test Runner → EditMode tab
Tests use Unity Test Framework (NUnit) at `Unity/Assets/Tests/EditMode/`. The test assembly (`TT2.Tests.EditMode`) uses namespace `TaekwondoTech.Tests.EditMode` and references `TT2.Runtime` (production code).
**Preflight checks (run before every push):**
```
./scripts/preflight.sh
```
This catches most CI failures locally in ~1 second: missing `.meta` files, duplicate GUIDs, 500-line violations, bad namespace imports, missing asmdef package references, and unprotected `Object.Destroy()` in EditMode tests. **Always run this before pushing.** If you encounter a new class of CI failure not caught by preflight, add a check for it to the script so future runs catch it early.
**CI pipeline** runs automatically on pushes to `main` and PRs (not forks). Three jobs in order:
1. **Preflight** (~10s) — runs `scripts/preflight.sh`
2. **EditMode Tests** (~3-4min) — `game-ci/unity-test-runner@v4` with `-nographics`
3. **Builds** (~4min each) — WebGL, iOS, Android in parallel via `game-ci/unity-builder`
For the authoritative details on local build/test workflow and the CI pipeline, see the **“Build, Test & CI pipeline”** section later in this document. That section is the single canonical source of truth and should be updated if the process changes.
**Quick local workflow (summary only):**
- Run EditMode tests via Unity Editor → Window → General → Test Runner → **EditMode** tab.
- Always run the preflight script before pushing:
```bash
./scripts/preflight.sh

This catches most CI failures locally in about a second.

Refer to the later Build, Test & CI pipeline section for the full explanation of test locations, assemblies, and CI job configuration.

Copilot uses AI. Check for mistakes.
@norrietaylor norrietaylor merged commit 1650ec6 into main Mar 29, 2026
16 checks passed
@norrietaylor norrietaylor deleted the feature/unity-unit-tests branch March 29, 2026 17:04
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.

2 participants