lvl0/incr

Fork 0

Test coverage for the counter #54

New issue

Closed

opened 2026-08-15 13:35:57 +02:00 by myrmidex · 2 comments

myrmidex commented

2026-08-15 13:35:57 +02:00

Owner

Restores the safety net. MilestoneTest.php is the repo's only test and #49 deletes it, so the suite is empty from that point until this lands.

Why this is its own ticket

Ideally this comes first, TDD-style. It can't: the thing under test (trackers.count, POST /increment) does not exist until #50, and the component under test changes shape again at #52.

The pragmatic sequencing is to accept a temporary gap and verify #47–#51 manually via the UI, then land this immediately after #50 so #52 has real regression cover for the stack swap. That is the point where tests earn the most — a Livewire rewrite with no tests is the risky move in this milestone.

Scope

Feature tests (post-#50, pre-#52):

POST /increment increments by exactly 1
Repeated increments accumulate correctly
PATCH /count sets an absolute value
PATCH /count rejects negatives, non-integers, missing values
A fresh install starts at 0 and stays on the counter screen

Livewire tests (post-#52):

Livewire::test(Counter::class)->call('increment') updates the rendered value
The dialog validates and persists

Also: delete tests/Unit/ExampleTest.php (framework placeholder).

Acceptance criteria

Increment and set-value paths covered, happy path and validation failures
Suite green in CI
No dependency on seeded dev data — fixtures/factories only

Restores the safety net. `MilestoneTest.php` is the repo's only test and #49 deletes it, so the suite is empty from that point until this lands. ## Why this is its own ticket Ideally this comes first, TDD-style. It can't: the thing under test (`trackers.count`, `POST /increment`) does not exist until #50, and the component under test changes shape again at #52. The pragmatic sequencing is to accept a temporary gap and verify #47–#51 manually via the UI, then land this immediately after #50 so #52 has real regression cover for the stack swap. That is the point where tests earn the most — a Livewire rewrite with no tests is the risky move in this milestone. ## Scope **Feature tests (post-#50, pre-#52):** - `POST /increment` increments by exactly 1 - Repeated increments accumulate correctly - `PATCH /count` sets an absolute value - `PATCH /count` rejects negatives, non-integers, missing values - A fresh install starts at 0 and stays on the counter screen **Livewire tests (post-#52):** - `Livewire::test(Counter::class)->call('increment')` updates the rendered value - The dialog validates and persists **Also:** delete `tests/Unit/ExampleTest.php` (framework placeholder). ## Acceptance criteria - [ ] Increment and set-value paths covered, happy path and validation failures - [ ] Suite green in CI - [ ] No dependency on seeded dev data — fixtures/factories only

myrmidex added this to the (deleted) milestone 2026-08-15 13:35:57 +02:00

myrmidex added the

enhancement

label 2026-08-15 13:35:57 +02:00

myrmidex self-assigned this 2026-08-15 13:35:57 +02:00

myrmidex modified the milestone from (deleted) to v0.4.0

2026-08-15 13:42:27 +02:00

myrmidex referenced this issue

2026-08-15 13:42:39 +02:00

Add PHPStan + larastan static analysis #55

myrmidex referenced this issue

2026-08-15 13:43:05 +02:00

Add prebuilt CI image and images.yml workflow #57

myrmidex referenced this issue

2026-08-15 14:43:39 +02:00

Remove milestones, progress bar and stats box #49

myrmidex commented

2026-08-15 14:53:20 +02:00

Author

Owner

Concrete test cases (from the #50 review)

The code-reviewer pass on #50 produced a specific list. Recording it here so this ticket has a checklist rather than a sketch.

Migration backfill (`2026_08_15_000002`)

Tracker with multiple entries summing to a fractional total → rounds correctly
Tracker with entries summing to a negative total → clamps to 0
Tracker with zero entries → count stays at the default 0
Fresh install where entries never existed (Schema::hasTable('entries') false) → migration does not error

Migration `down()`

up() then down() → entries exists again with correct columns and nullability
Tracker with count = 0 → produces no entry row
Tracker with count > 0 → produces exactly one row with quantity = count

`POST /increment`

No tracker → 404 with the expected JSON shape
Tracker exists → count increases by exactly 1, response carries the new value
Two sequential increments → reaches 2

`PATCH /count`

Valid non-negative integer → succeeds and persists
Negative → 422
Non-integer ("abc", 1.5) → 422
Missing count key → 422
No tracker → 404
At/above the unsignedInteger ceiling (4294967295) → 422, not a DB exception. (A max:4294967295 rule was added during #50; this test guards it.)

Route contract — high value

POST /increment and PATCH /count return Content-Type: application/json, not an Inertia redirect

This last one is the regression test that matters most. The bug it guards against — EntryController::store returning back(), so fetch() followed the redirect and reported success on validation failure — is exactly what #50 removed.

Model

Tracker::count casts to int, not string, after a fresh find()

## Concrete test cases (from the #50 review) The `code-reviewer` pass on #50 produced a specific list. Recording it here so this ticket has a checklist rather than a sketch. ### Migration backfill (`2026_08_15_000002`) - Tracker with multiple entries summing to a fractional total → rounds correctly - Tracker with entries summing to a negative total → clamps to 0 - Tracker with zero entries → `count` stays at the default 0 - Fresh install where `entries` never existed (`Schema::hasTable('entries')` false) → migration does not error ### Migration `down()` - `up()` then `down()` → `entries` exists again with correct columns and nullability - Tracker with `count = 0` → produces no entry row - Tracker with `count > 0` → produces exactly one row with `quantity = count` ### `POST /increment` - No tracker → 404 with the expected JSON shape - Tracker exists → count increases by exactly 1, response carries the new value - Two sequential increments → reaches 2 ### `PATCH /count` - Valid non-negative integer → succeeds and persists - Negative → 422 - Non-integer (`"abc"`, `1.5`) → 422 - Missing `count` key → 422 - No tracker → 404 - At/above the `unsignedInteger` ceiling (4294967295) → 422, not a DB exception. *(A `max:4294967295` rule was added during #50; this test guards it.)* ### Route contract — high value - `POST /increment` and `PATCH /count` return `Content-Type: application/json`, **not** an Inertia redirect This last one is the regression test that matters most. The bug it guards against — `EntryController::store` returning `back()`, so `fetch()` followed the redirect and reported success on validation failure — is exactly what #50 removed. ### Model - `Tracker::count` casts to `int`, not string, after a fresh `find()`

myrmidex referenced this issue

2026-08-15 15:20:32 +02:00

Collapse entry ledger to a scalar count column #50

myrmidex commented

2026-08-15 15:20:51 +02:00

Author

Owner

Done — `3a28988`

From one placeholder test to 25 tests, 62 assertions.

tests/Feature/CounterTest.php — 13 tests: increment adds exactly 1, increments accumulate, 404 without a tracker, absolute set, set to zero, #[DataProvider] over invalid values (negative, non-numeric, fractional, above the unsigned ceiling), missing value, JSON-not-redirect contract, integer cast
tests/Feature/TrackerTest.php — 5 tests: fresh install reports no tracker, tracker returned once created, creation without label/unit applies defaults, a new counter starts at 0, second creation 409s without writing a row
tests/Feature/CountBackfillMigrationTest.php — 7 tests against the #50 migration
database/factories/TrackerFactory.php — new; Tracker gained HasFactory
Deleted ExampleTest.php; added tests/Unit/.gitkeep (same phpunit.xml directory trap as #49)

These tests were verified to fail

A green suite proves nothing on its own, so both critical paths were mutation-checked:

Injected bug	Result
`increment('count', 2)`	2 failures, exact diagnosis (`count => 7` vs `6`)
`(int) floor($total)` for `max(0, (int) round($total))`	rounding failed `2` vs `3`; clamping surfaced `SQLSTATE[22003] Out of range`

The second is worth recording: without the clamp a negative ledger total crashes the migration on the unsigned column rather than storing a wrong number.

Test isolation — verified, not assumed

Review raised a critical concern: MySQL DDL implicitly commits, which can break RefreshDatabase's transaction and let migration tests pass on leaked state.

Checked with a temporary probe ordered to run last, after all DDL. It confirmed trackers.count still present, entries absent, zero leaked rows. Isolation holds. The mechanism is real, but Laravel re-migrates when the transaction is compromised — the cost is speed (~29s of the 35s runtime), not correctness. Probe removed afterwards.

From review

Applied: comment explaining the repeated require of an anonymous-class migration; note on why the two down() tests skip revertToLedger(); TrackerFactory defaults user_id to User::default()->id so an omitted override cannot silently create a second user with a second tracker.

Declined: a concurrent-increment race test (single-user app, and increment() is atomic SET count = count + 1), and reworking the cast test.

Confirmed deliberate: PATCH /tracker is uncovered because #48/#53 delete that code.

Known coupling

Every test resolves fixtures through User::default(), mirroring how the controllers work. #53 will require touching all of these — expected, not accidental.

Livewire tests remain outstanding and belong to #52.

## Done — `3a28988` From one placeholder test to **25 tests, 62 assertions**. - `tests/Feature/CounterTest.php` — 13 tests: increment adds exactly 1, increments accumulate, 404 without a tracker, absolute set, set to zero, `#[DataProvider]` over invalid values (negative, non-numeric, fractional, above the unsigned ceiling), missing value, JSON-not-redirect contract, integer cast - `tests/Feature/TrackerTest.php` — 5 tests: fresh install reports no tracker, tracker returned once created, creation without label/unit applies defaults, a new counter starts at 0, second creation 409s without writing a row - `tests/Feature/CountBackfillMigrationTest.php` — 7 tests against the #50 migration - `database/factories/TrackerFactory.php` — new; `Tracker` gained `HasFactory` - Deleted `ExampleTest.php`; added `tests/Unit/.gitkeep` (same `phpunit.xml` directory trap as #49) ### These tests were verified to fail A green suite proves nothing on its own, so both critical paths were mutation-checked: | Injected bug | Result | |---|---| | `increment('count', 2)` | 2 failures, exact diagnosis (`count => 7` vs `6`) | | `(int) floor($total)` for `max(0, (int) round($total))` | rounding failed `2` vs `3`; clamping surfaced `SQLSTATE[22003] Out of range` | The second is worth recording: **without the clamp a negative ledger total crashes the migration** on the unsigned column rather than storing a wrong number. ### Test isolation — verified, not assumed Review raised a critical concern: MySQL DDL implicitly commits, which can break `RefreshDatabase`'s transaction and let migration tests pass on leaked state. Checked with a temporary probe ordered to run last, after all DDL. It confirmed `trackers.count` still present, `entries` absent, zero leaked rows. **Isolation holds.** The mechanism is real, but Laravel re-migrates when the transaction is compromised — the cost is speed (~29s of the 35s runtime), not correctness. Probe removed afterwards. ### From review Applied: comment explaining the repeated `require` of an anonymous-class migration; note on why the two `down()` tests skip `revertToLedger()`; `TrackerFactory` defaults `user_id` to `User::default()->id` so an omitted override cannot silently create a second user with a second tracker. Declined: a concurrent-increment race test (single-user app, and `increment()` is atomic `SET count = count + 1`), and reworking the cast test. Confirmed deliberate: `PATCH /tracker` is uncovered because #48/#53 delete that code. ### Known coupling Every test resolves fixtures through `User::default()`, mirroring how the controllers work. **#53 will require touching all of these** — expected, not accidental. Livewire tests remain outstanding and belong to #52.

myrmidex closed this issue

2026-08-15 15:20:57 +02:00

myrmidex referenced this issue

2026-08-15 21:34:56 +02:00

Remove asset and price-tracking subsystem #48