Initial commit
This commit is contained in:
@@ -0,0 +1,312 @@
|
||||
# Phase 3.K.1 — Test harness foundation (DI refactor + pure-logic tests)
|
||||
|
||||
**Status:** Design, awaiting review
|
||||
**Date:** 2026-04-17
|
||||
**Parent:** Phase 3, Area K (Test harness). Originally a single P4 task in `2026-04-17-phase3-content-meta-retention.md`; expanded into the K.x series after brainstorming.
|
||||
|
||||
---
|
||||
|
||||
## Goal
|
||||
|
||||
Establish a test harness in the TowerAn project that serves as a **safety net for future refactoring**. Introduce unit tests for pure-logic singletons (`PlayerStats`, `PlayerAbilities`, `GameState`, `TalentTree`, `TalentData`) so that subsequent extraction work in the `FirstScreen` god-class (planned as spec series K.2–K.7) can proceed without fear of silent regressions.
|
||||
|
||||
The spec also delivers the architectural prerequisite for testability: **removing the `getInstance()` singleton pattern** from the persistent-state classes, replacing it with constructor injection via a lightweight `GameContext` holder.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Any extraction of logic out of `FirstScreen`. Those are separate specs (K.2 `BulletSystem`, K.3 `SpawnController`, K.4 `WaveController`, K.5 `EnemyBehaviour`, K.6 `AbilitySystem`, K.7 `UpgradeSystem`). K.1 only prepares the ground.
|
||||
- Tests for `Achievements` and `DailySeed`. These classes do not exist yet (Phase 3 Tasks G1 / H1); their tests are authored alongside the classes, not in K.1.
|
||||
- Tests for enemy behaviour (`BaseCircleData` and subclasses). Deferred to K.5 because the new Phase 3 archetypes (Healer, Dasher, Sniper, Gnat) are not yet implemented.
|
||||
- Code coverage gating (JaCoCo). Introducing coverage thresholds without baseline history is premature.
|
||||
- CI integration (GitHub Actions or similar). Scope is local `./gradlew test`. CI wiring can be a tiny follow-up once K.1 is merged.
|
||||
- Headless libGDX (`HeadlessApplication`). Not needed because the DI refactor lets tests supply an in-memory `Preferences` directly.
|
||||
|
||||
## Background
|
||||
|
||||
TowerAn is a libGDX 1.13.1 game on Java 8, gdx-liftoff template, flat `ru.project.tower` package. There are currently **no tests of any kind** — no JUnit, no Spock, no manual harness. Verification has been purely visual: launch `Lwjgl3Launcher`, play, watch.
|
||||
|
||||
Five classes hold persistent or shared state through a static-singleton pattern:
|
||||
|
||||
- `PlayerStats` — currency, max wave, talent node levels. Backed by `Preferences` (`tower_attack_player_data`). `getInstance()` eagerly calls `Gdx.app.getPreferences(...)`.
|
||||
- `PlayerAbilities` — unlocked abilities, levels. Backed by `Preferences` (`tower_attack_player_abilities`). Same pattern.
|
||||
- `GameState` — in-memory run state (current wave, square health, money, run bonuses). No `Preferences`.
|
||||
- `TalentTree` — tree structure + level queries that call `PlayerStats.getInstance()` internally (lines 133, 138).
|
||||
- `TalentData` — plain data class used by `TalentTree`. No singleton; already testable but currently untested.
|
||||
|
||||
`getInstance()` usage is concentrated in five files (15 calls): `AbilitySelectionScreen` (2), `AbilityShopScreen` (2), `FirstScreen` (8), `TalentTreeScreen` (1), `TalentTree` (2).
|
||||
|
||||
Because singletons eagerly touch `Gdx.app` and `Preferences` on first `getInstance()`, any test that instantiates them fails outside a running libGDX context. The options are (a) reflection-nuking the `instance` field between tests, (b) `@VisibleForTesting resetForTests()` helpers, or (c) breaking the pattern. We chose (c) — it's a one-time architectural change that makes tests clean and unlocks downstream `FirstScreen` extraction work in K.2+.
|
||||
|
||||
## Architecture overview
|
||||
|
||||
Three layers, implemented in this order (later layers depend on earlier ones):
|
||||
|
||||
1. **DI refactor.** Delete `private static instance` and `public static getInstance()` from `PlayerStats`, `PlayerAbilities`, `GameState`, `TalentTree`. Make constructors public and accept their dependencies explicitly.
|
||||
2. **Composition root.** Introduce `GameContext` as a holder for the refactored singletons. `Main.create()` constructs two `Preferences` instances, wraps them in a `GameContext`, and passes the `GameContext` to each screen via constructor injection.
|
||||
3. **Test harness.** Add JUnit 5 + AssertJ to `core/build.gradle`. Create an in-memory `FakePreferences` helper in `core/src/test/java`. Write unit tests for the five target classes.
|
||||
|
||||
Nothing in the rendering layer, the screen hierarchy, the enemy hierarchy, or the gameplay loop changes beyond mechanical `X.getInstance()` → `ctx.x` substitutions.
|
||||
|
||||
## Components
|
||||
|
||||
### `GameContext` (new, `core/src/main/java/ru/project/tower/GameContext.java`)
|
||||
|
||||
A tiny holder with `public final` fields, idiomatic Java for an immutable data carrier (Google Java Style §5.3 allows `public final` for this role).
|
||||
|
||||
```java
|
||||
public class GameContext {
|
||||
public final Preferences playerDataPrefs;
|
||||
public final Preferences abilitiesPrefs;
|
||||
public final PlayerStats playerStats;
|
||||
public final PlayerAbilities playerAbilities;
|
||||
public final GameState gameState;
|
||||
public final TalentTree talentTree;
|
||||
|
||||
public GameContext(Preferences playerDataPrefs, Preferences abilitiesPrefs) {
|
||||
this.playerDataPrefs = playerDataPrefs;
|
||||
this.abilitiesPrefs = abilitiesPrefs;
|
||||
this.playerStats = new PlayerStats(playerDataPrefs);
|
||||
this.playerAbilities = new PlayerAbilities(abilitiesPrefs);
|
||||
this.gameState = new GameState();
|
||||
this.talentTree = new TalentTree(this.playerStats);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Construction order matters: `TalentTree` depends on `PlayerStats`, so `playerStats` is created first. `Preferences` references are retained on the holder so future services (e.g., `Achievements`, `DailySeed` in Phase 3 G/H) can reuse them without re-plumbing `Main`.
|
||||
|
||||
### Singleton-to-DI conversions
|
||||
|
||||
For each target, the change is the same mechanical shape. Example for `PlayerStats`:
|
||||
|
||||
```java
|
||||
// Before
|
||||
private static PlayerStats instance;
|
||||
private Preferences preferences;
|
||||
|
||||
private PlayerStats() {
|
||||
preferences = Gdx.app.getPreferences(PREFS_NAME);
|
||||
loadData();
|
||||
}
|
||||
|
||||
public static PlayerStats getInstance() {
|
||||
if (instance == null) instance = new PlayerStats();
|
||||
return instance;
|
||||
}
|
||||
|
||||
// After
|
||||
private final Preferences preferences;
|
||||
|
||||
public PlayerStats(Preferences preferences) {
|
||||
this.preferences = preferences;
|
||||
loadData();
|
||||
}
|
||||
```
|
||||
|
||||
The private `PREFS_NAME = "tower_attack_player_data"` constant is deleted; the same literal moves to `Main` as `private static final String PLAYER_DATA_PREFS_NAME`. `PlayerAbilities` gets analogous treatment with `ABILITIES_PREFS_NAME`.
|
||||
|
||||
`GameState` has no `Preferences`, so its constructor is simply made public and argumentless. `TalentTree(PlayerStats stats)` stores the reference and replaces the two `PlayerStats.getInstance()` calls at lines 133 / 138 with `this.stats.getTalentNodeLevel(...)` and `this.stats.incrementTalentNode(...)`.
|
||||
|
||||
### `Main` as composition root
|
||||
|
||||
```java
|
||||
public class Main extends Game {
|
||||
private static final String PLAYER_DATA_PREFS_NAME = "tower_attack_player_data";
|
||||
private static final String ABILITIES_PREFS_NAME = "tower_attack_player_abilities";
|
||||
|
||||
public GameContext ctx;
|
||||
|
||||
@Override
|
||||
public void create() {
|
||||
ctx = new GameContext(
|
||||
Gdx.app.getPreferences(PLAYER_DATA_PREFS_NAME),
|
||||
Gdx.app.getPreferences(ABILITIES_PREFS_NAME)
|
||||
);
|
||||
setScreen(new MainScreen(this, ctx));
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`ctx` is a public field on `Main` so screens that already hold a `Main` reference can read it if needed, but the preferred path is receiving `GameContext` in the constructor.
|
||||
|
||||
### Screen constructor changes
|
||||
|
||||
Every screen class currently instantiated with `new Xxx(game)` becomes `new Xxx(game, ctx)`:
|
||||
|
||||
- `MainScreen(Main game, GameContext ctx)`
|
||||
- `FirstScreen(Main game, GameContext ctx)`
|
||||
- `AbilityShopScreen(Main game, GameContext ctx)`
|
||||
- `AbilitySelectionScreen(Main game, GameContext ctx)`
|
||||
- `TalentTreeScreen(Main game, GameContext ctx)`
|
||||
|
||||
Every `X.getInstance()` call becomes `ctx.x`. In `FirstScreen`, where there are 8 calls, cache into fields (`private final PlayerStats playerStats;` etc.) during the constructor to avoid `ctx.playerStats` repetition.
|
||||
|
||||
### `FakePreferences` (test helper, `core/src/test/java/ru/project/tower/FakePreferences.java`)
|
||||
|
||||
An in-memory implementation of `com.badlogic.gdx.Preferences` backed by `HashMap<String, Object>`. Approximately 30 trivial methods — every `putX(k, v)` stores in the map, every `getX(k)` / `getX(k, default)` reads. `flush()` is a no-op. `clear()` clears the map. `contains(k)` delegates to `map.containsKey`.
|
||||
|
||||
One shared helper serves every test class. Chosen over Mockito because Mockito for a single deterministic interface adds a dependency and weaker error messages for a trivial case.
|
||||
|
||||
## Data flow
|
||||
|
||||
**Production:**
|
||||
```
|
||||
Main.create()
|
||||
└─ creates two Preferences via Gdx.app.getPreferences(...)
|
||||
└─ new GameContext(playerDataPrefs, abilitiesPrefs)
|
||||
└─ setScreen(new MainScreen(this, ctx))
|
||||
│
|
||||
└─ ctx propagates to every downstream screen constructor
|
||||
└─ screens access ctx.playerStats, ctx.talentTree, etc.
|
||||
```
|
||||
|
||||
**Tests:**
|
||||
```
|
||||
@BeforeEach
|
||||
└─ new FakePreferences()
|
||||
└─ new PlayerStats(fakePrefs) // or the class under test, injected with fakes
|
||||
@Test
|
||||
└─ AAA: arrange state, invoke method, assert outcome
|
||||
(both on the object's API and on the fake Preferences where persistence matters)
|
||||
```
|
||||
|
||||
## Test conventions
|
||||
|
||||
- **Framework:** JUnit 5 (`org.junit.jupiter:junit-jupiter:5.10.2`).
|
||||
- **Assertions:** AssertJ (`org.assertj:assertj-core:3.26.3`). All assertions use `assertThat(...)` — no `assertEquals`. Rationale: fluent API, clearer messages, industry standard in modern Java.
|
||||
- **Test-method naming:** `methodName_whenCondition_thenExpected`. Sorts naturally by method in IDE, reads like a sentence.
|
||||
- `incrementTalentNode_whenNewNode_thenStoredInPreferences`
|
||||
- `getCurrency_whenNotYetSet_thenReturnsZero`
|
||||
- **Test-class naming:** `<ClassUnderTest>Test` (e.g., `PlayerStatsTest.java`).
|
||||
- **Body structure:** Arrange / Act / Assert separated by blank lines. No `// Arrange` comments.
|
||||
- **Isolation:** JUnit 5's default new-instance-per-test + `@BeforeEach` for setup. No static state between tests.
|
||||
- **One behaviour per test.** If a test has three `assertThat(...)` lines, they should all be aspects of the same behaviour (e.g., state returned _and_ persisted). Don't bundle unrelated scenarios.
|
||||
|
||||
## Test coverage targets
|
||||
|
||||
Approximately 30–35 test methods across 6 test classes, as sketched below. This is a floor, not a ceiling — prefer covering each public-API method with at least one happy-path test and one edge case.
|
||||
|
||||
### `PlayerStatsTest`
|
||||
- `getCurrency_whenNotYetSet_thenReturnsZero`
|
||||
- `addCurrency_whenCalled_thenIncrementsAndPersists`
|
||||
- `spendCurrency_whenInsufficient_thenReturnsFalseAndDoesNotDeduct`
|
||||
- `spendCurrency_whenSufficient_thenReturnsTrueAndDeducts`
|
||||
- `getMaxWaveReached_thenReadsFromPreferences`
|
||||
- `updateMaxWaveReached_whenNewHigher_thenUpdates`
|
||||
- `updateMaxWaveReached_whenNewLower_thenIgnored`
|
||||
- `getTalentNodeLevel_whenUnseen_thenZero`
|
||||
- `incrementTalentNode_whenCalled_thenLevelOneAndPersisted`
|
||||
- `incrementTalentNode_whenCalledTwice_thenLevelTwo`
|
||||
- Legacy aggregate getter(s): one test covering the wrapper still reports the right total.
|
||||
- A test that constructs `PlayerStats` from pre-populated `FakePreferences` and reads back expected state (persistence round-trip).
|
||||
|
||||
### `PlayerAbilitiesTest`
|
||||
- Every ability-unlock flag, get/set round-trip through `FakePreferences`.
|
||||
- Unlock toggles don't affect unrelated flags.
|
||||
- Default-state test (fresh `FakePreferences` gives the expected initial roster).
|
||||
|
||||
### `GameStateTest`
|
||||
- Initial values (wave, square health, money).
|
||||
- Wave increment.
|
||||
- Phase 2 `run*` bonus fields (damage, pierce, etc.) accumulate correctly.
|
||||
- Reset-for-new-run behaviour. The implementation plan must first confirm whether `GameState` has a reset API; if not, the plan adds a small one as part of this spec (this is a test-enabling concession, not feature work).
|
||||
|
||||
### `TalentTreeTest`
|
||||
- `initializeTree` creates expected roots and node IDs.
|
||||
- `getTalentLevel(id)` delegates to injected `PlayerStats`.
|
||||
- `incrementTalent(id)` delegates to injected `PlayerStats`.
|
||||
- Keystone node IDs (`keystone_pierce`, `keystone_crit`, `keystone_aoe`) are all registered.
|
||||
|
||||
### `TalentDataTest`
|
||||
- Construction: id, name, description, type, position, max level, base cost, cost multiplier all round-trip via getters.
|
||||
- Cost-at-level formula round-trip. The exact formula is read from the current `TalentData` source during implementation; the test locks in whatever that formula is, not a newly-invited one.
|
||||
|
||||
### `GameContextTest`
|
||||
- Construction with two `FakePreferences` yields non-null services.
|
||||
- `ctx.talentTree.getTalentLevel(someId)` reads from the same `PlayerStats` that `ctx.playerStats` exposes (i.e., wiring is correct).
|
||||
- Construction order: `TalentTree` sees the injected `PlayerStats`, not a new one.
|
||||
|
||||
## Build configuration
|
||||
|
||||
`core/build.gradle`:
|
||||
|
||||
```gradle
|
||||
dependencies {
|
||||
api "com.badlogicgames.gdx:gdx-freetype:$gdxVersion"
|
||||
api "com.badlogicgames.gdx:gdx:$gdxVersion"
|
||||
|
||||
testImplementation 'org.junit.jupiter:junit-jupiter:5.10.2'
|
||||
testImplementation 'org.assertj:assertj-core:3.26.3'
|
||||
|
||||
if (enableGraalNative == 'true') {
|
||||
implementation "io.github.berstanio:gdx-svmhelper-annotations:$graalHelperVersion"
|
||||
}
|
||||
}
|
||||
|
||||
test {
|
||||
useJUnitPlatform()
|
||||
}
|
||||
```
|
||||
|
||||
Java 8 compatibility is preserved: JUnit 5.10 and AssertJ 3.26 both still ship Java-8-compatible bytecode.
|
||||
|
||||
## File layout
|
||||
|
||||
```
|
||||
core/src/
|
||||
main/java/ru/project/tower/
|
||||
GameContext.java [NEW]
|
||||
PlayerStats.java [MODIFIED — constructor + remove getInstance]
|
||||
PlayerAbilities.java [MODIFIED — constructor + remove getInstance]
|
||||
GameState.java [MODIFIED — constructor public + remove getInstance]
|
||||
TalentTree.java [MODIFIED — constructor + remove getInstance]
|
||||
Main.java [MODIFIED — composition root]
|
||||
MainScreen.java [MODIFIED — constructor takes ctx]
|
||||
FirstScreen.java [MODIFIED — constructor takes ctx, replace 8 getInstance]
|
||||
AbilityShopScreen.java [MODIFIED — constructor takes ctx]
|
||||
AbilitySelectionScreen.java [MODIFIED — constructor takes ctx]
|
||||
TalentTreeScreen.java [MODIFIED — constructor takes ctx]
|
||||
test/java/ru/project/tower/ [NEW TREE]
|
||||
FakePreferences.java
|
||||
PlayerStatsTest.java
|
||||
PlayerAbilitiesTest.java
|
||||
GameStateTest.java
|
||||
TalentTreeTest.java
|
||||
TalentDataTest.java
|
||||
GameContextTest.java
|
||||
```
|
||||
|
||||
## Error handling
|
||||
|
||||
There is one edge where the refactor can silently change behaviour: `getInstance()` lazily creates the singleton on first call. A class that was instantiated *before* `Gdx.app` was ready (unlikely in this codebase but worth naming) would now fail eagerly at `Main.create()` instead of at first use. Since `Main.create()` is the libGDX entry point and `Gdx.app` is guaranteed set by the backend before `create()` runs, this is safe.
|
||||
|
||||
All other error paths are unchanged — `Preferences.putX`/`getX` throw the same exceptions; `loadData()` runs the same logic.
|
||||
|
||||
## Verification
|
||||
|
||||
Because K.1 introduces the test harness itself, verification is both **automated** (the tests) and **manual** (the DI refactor must not break the running game):
|
||||
|
||||
- `./gradlew test` runs cleanly, all tests pass.
|
||||
- Launch `Lwjgl3Launcher`. Confirm:
|
||||
- Main menu renders; currency and max-wave values read from existing preferences.
|
||||
- Entering the talent tree shows purchased levels unchanged.
|
||||
- Starting a run, buying an upgrade, dying, and returning to menu preserves currency.
|
||||
- Ability shop displays owned abilities correctly.
|
||||
|
||||
Any of these failing indicates the `getInstance()` → `ctx.x` substitution dropped a wire.
|
||||
|
||||
## Risks and mitigations
|
||||
|
||||
- **Missed call-site.** 15 `getInstance()` calls plus possibly some we didn't count (constants, static initializers). Mitigation: after the refactor, `grep -r "getInstance()" core/src/main` must return zero hits. Automated in implementation plan.
|
||||
- **Screen-constructor signature changes break something external.** `lwjgl3/` and `android/` only touch `Main`, not individual screens. Mitigation: grep across all modules for `new MainScreen(`, `new FirstScreen(`, etc. after the refactor.
|
||||
- **Hidden `Preferences`-name constants.** We found two (`tower_attack_player_data`, `tower_attack_player_abilities`). If Phase 3 work introduces a third, the spec still works — just add another `Preferences` parameter to `GameContext`. Not a risk for K.1 itself.
|
||||
- **Test flakiness from order dependence.** Prevented by JUnit 5 per-test-instance default + fresh `FakePreferences` in `@BeforeEach`.
|
||||
|
||||
## Out of scope (reminder)
|
||||
|
||||
K.1 delivers the harness and proves the pattern on five pure-logic classes. Every subsequent K.x spec (K.2–K.7) builds on this to extract and test real gameplay subsystems. K.1 is merged when:
|
||||
|
||||
1. All 15+ `getInstance()` calls are gone.
|
||||
2. `GameContext` is the sole composition point for persistent services.
|
||||
3. `./gradlew test` runs and passes all 30+ tests authored in this spec.
|
||||
4. The game still boots and plays correctly on the desktop launcher.
|
||||
Reference in New Issue
Block a user