lvl0/dishplanner

Fork 0

Test suite assertion count is nondeterministic across runs #60

New issue

Closed

opened 2026-08-19 13:45:03 +02:00 by myrmidex · 1 comment

myrmidex commented

2026-08-19 13:45:03 +02:00

Owner

Summary

The test suite passes consistently (152 tests, 1 skipped) but the assertion count varies between runs on an unchanged working tree. Observed on release/v0.9.0 at cb2dbd9.

Evidence

Three consecutive full-suite runs, no code changes between them:

Tests: 152, Assertions: 1393, Skipped: 1
Tests: 152, Assertions: 1393, Skipped: 1
Tests: 152, Assertions: 1391, Skipped: 1

Narrowed to the Schedule tests via --filter=Schedule, which reproduces the same ±2 swing:

OK (73 tests, 668 assertions)
OK (73 tests, 666 assertions)

Confirmed contributing cause: date-dependent assertion count

tests/Unit/Schedule/Services/ScheduleCalendarServiceTest.php::test_marks_today_correctly builds its calendar from the real current date:

$today = now();
$calendarDays = $this->service->getCalendarDays($planner, $today->month, $today->year);

$todayIndex = $today->day - 1;
$this->assertTrue($calendarDays[$todayIndex]['isToday']);

foreach ($calendarDays as $index => $day) {
    if ($index !== $todayIndex && $day['day'] !== null) {
        $this->assertFalse($day['isToday']);   // runs (daysInMonth - 1) times
    }
}

The loop asserts once per real day in the current month, minus today, so the count is a function of the calendar:

Run date	Loop assertions
2026-08-19	30
2026-09-19	29
2026-04-19	29
2026-02-19	27

A suite whose assertion count depends on today's date is fragile: it makes assertion totals useless as a regression signal and means February silently exercises 3 fewer assertions than August.

Sibling tests in the same class avoid this by pinning the date (getCalendarDays($planner, 3, 2026), ..., 4, 2026), which is the pattern to follow — freeze time with Carbon::setTestNow() / travelTo(), or pin the month/year and assert a fixed count.

Open question — a second source remains

Month length cannot change between two runs seconds apart, so date-dependence explains fragility across days, not the ±2 observed within one session. A filtered run of ScheduleCalendarServiceTest alone gave a stable 162 assertions across the runs checked, so the remaining run-to-run variance was not isolated to that class.

There is a second, genuinely nondeterministic source somewhere in the Schedule tests that has not been identified. Candidates not yet ruled out: faker-driven fixture counts feeding assertion loops, or ordering/collection-size variation in the schedule generation tests.

Suggested work

Freeze time in test_marks_today_correctly (and audit the class for other uses of now()).
Track down the remaining run-to-run variance — bisect the Schedule tests by file, running each repeatedly until the count moves.
Consider a fixed clock for the whole suite so this class of drift cannot reappear.

Notes

Not introduced by the Dusk→Pest migration; the tests involved are pre-existing Unit/Feature tests, and the drift reproduces on unmodified code.
Lint (PASS 227 files) and PHPStan (clean) both green at the time of filing.

## Summary The test suite passes consistently (152 tests, 1 skipped) but the **assertion count varies between runs** on an unchanged working tree. Observed on `release/v0.9.0` at `cb2dbd9`. ## Evidence Three consecutive full-suite runs, no code changes between them: ``` Tests: 152, Assertions: 1393, Skipped: 1 Tests: 152, Assertions: 1393, Skipped: 1 Tests: 152, Assertions: 1391, Skipped: 1 ``` Narrowed to the Schedule tests via `--filter=Schedule`, which reproduces the same ±2 swing: ``` OK (73 tests, 668 assertions) OK (73 tests, 666 assertions) ``` ## Confirmed contributing cause: date-dependent assertion count `tests/Unit/Schedule/Services/ScheduleCalendarServiceTest.php::test_marks_today_correctly` builds its calendar from the **real current date**: ```php $today = now(); $calendarDays = $this->service->getCalendarDays($planner, $today->month, $today->year); $todayIndex = $today->day - 1; $this->assertTrue($calendarDays[$todayIndex]['isToday']); foreach ($calendarDays as $index => $day) { if ($index !== $todayIndex && $day['day'] !== null) { $this->assertFalse($day['isToday']); // runs (daysInMonth - 1) times } } ``` The loop asserts once per real day in the current month, minus today, so the count is a function of the calendar: | Run date | Loop assertions | |---|---| | 2026-08-19 | 30 | | 2026-09-19 | 29 | | 2026-04-19 | 29 | | 2026-02-19 | 27 | A suite whose assertion count depends on today's date is fragile: it makes assertion totals useless as a regression signal and means February silently exercises 3 fewer assertions than August. Sibling tests in the same class avoid this by pinning the date (`getCalendarDays($planner, 3, 2026)`, `..., 4, 2026`), which is the pattern to follow — freeze time with `Carbon::setTestNow()` / `travelTo()`, or pin the month/year and assert a fixed count. ## Open question — a second source remains Month length **cannot change between two runs seconds apart**, so date-dependence explains fragility *across days*, not the ±2 observed *within one session*. A filtered run of `ScheduleCalendarServiceTest` alone gave a stable 162 assertions across the runs checked, so the remaining run-to-run variance was not isolated to that class. There is a second, genuinely nondeterministic source somewhere in the Schedule tests that has **not** been identified. Candidates not yet ruled out: faker-driven fixture counts feeding assertion loops, or ordering/collection-size variation in the schedule generation tests. ## Suggested work 1. Freeze time in `test_marks_today_correctly` (and audit the class for other uses of `now()`). 2. Track down the remaining run-to-run variance — bisect the Schedule tests by file, running each repeatedly until the count moves. 3. Consider a fixed clock for the whole suite so this class of drift cannot reappear. ## Notes - Not introduced by the Dusk→Pest migration; the tests involved are pre-existing Unit/Feature tests, and the drift reproduces on unmodified code. - Lint (`PASS 227 files`) and PHPStan (clean) both green at the time of filing.

myrmidex added this to the v0.9.0 milestone 2026-08-19 13:45:03 +02:00

myrmidex added the

bug

testing

labels 2026-08-19 13:45:03 +02:00

myrmidex self-assigned this 2026-08-19 13:45:03 +02:00

myrmidex removed their assignment 2026-08-19 13:46:54 +02:00

myrmidex commented

2026-08-20 20:24:33 +02:00

Author

Owner

Resolved

Fixed in c3a5532. Both sources identified — including the "second source" the description left open.

1. Date-dependent loop (drift across days)

ScheduleCalendarServiceTest::test_marks_today_correctly built its calendar from the real now() and asserted once per day in the current month. Fixed by freezing the clock to 2026-03-15 via travelTo() and pinning the month/year, matching the sibling tests that already pass (planner, 3, 2026). The loop is now a single assertCount(0, ...).

travelBack() is deliberately not called: InteractsWithTestCaseLifecycle::tearDownTheTestEnvironment() (framework lines 155–161) unconditionally resets Carbon::setTestNow() after every test, pass or fail, so the frozen clock cannot leak into siblings.

2. Variable-length assertion loop (the ±2 within one session) — found

The open question is answered: it was ScheduleGeneratorTest::test_it_takes_minimum_recurrences_into_account.

It used ->reduce() to assert once per gap between placements of the recurring dish. How many times the generator places that dish depends on random fixture state, so the assertion count tracked a random outcome. Reproduced directly — the file alone gave 17 then 19 assertions on consecutive runs.

This is only visible in aggregate. Run in isolation it sits stable at 9, because RefreshDatabase reseeds identically when it runs first — which is why the original --filter=ScheduleCalendarServiceTest probe in the description came back clean and pointed away from the real culprit.

Fixed by collecting the gaps and making one fixed assertion over them, which also yields a better failure message than N independent assertions.

Coverage fix carried along

The filter changed from scheduledUserDishes()->first()->userDish->dish->id === $dishRecurring->id to a ->contains() over the whole relation. UserDishRepository::findInterferingUserDishes() controls which dishes enter the candidate pool but not the insert order of ScheduledUserDish rows within a schedule — that follows User::all() iteration order. The old check silently skipped any schedule where the recurring dish landed on a user who wasn't first, so the test was under-counting. Confirmed in review against the repository source.

Verification

Check	Before	After
Full suite	1393 / 1393 / 1391	1379, identical ×3
`--filter=Schedule`	673 / 671	638, identical ×3
Pint	—	PASS, 229 files
PHPStan	—	clean

Note on `phpstan-baseline.neon`

Removing the reduce() left an unmatched ignore pattern, which fails the analyse gate. The stale entry is removed in the same commit — the baseline change is required, not cosmetic.

Suggestion 3 from the description (a fixed clock for the whole suite) was not implemented. The two specific sources are fixed and the count is stable; a suite-wide clock is a larger change worth its own ticket if the class of drift reappears.

## Resolved Fixed in `c3a5532`. Both sources identified — including the "second source" the description left open. ### 1. Date-dependent loop (drift across days) `ScheduleCalendarServiceTest::test_marks_today_correctly` built its calendar from the real `now()` and asserted once per day in the current month. Fixed by freezing the clock to 2026-03-15 via `travelTo()` and pinning the month/year, matching the sibling tests that already pass `(planner, 3, 2026)`. The loop is now a single `assertCount(0, ...)`. `travelBack()` is deliberately not called: `InteractsWithTestCaseLifecycle::tearDownTheTestEnvironment()` (framework lines 155–161) unconditionally resets `Carbon::setTestNow()` after every test, pass or fail, so the frozen clock cannot leak into siblings. ### 2. Variable-length assertion loop (the ±2 within one session) — **found** The open question is answered: it was `ScheduleGeneratorTest::test_it_takes_minimum_recurrences_into_account`. It used `->reduce()` to assert once per *gap between* placements of the recurring dish. How many times the generator places that dish depends on random fixture state, so the assertion count tracked a random outcome. Reproduced directly — the file alone gave **17 then 19** assertions on consecutive runs. This is only visible in aggregate. Run in isolation it sits stable at 9, because `RefreshDatabase` reseeds identically when it runs first — which is why the original `--filter=ScheduleCalendarServiceTest` probe in the description came back clean and pointed away from the real culprit. Fixed by collecting the gaps and making one fixed assertion over them, which also yields a better failure message than N independent assertions. ### Coverage fix carried along The filter changed from `scheduledUserDishes()->first()->userDish->dish->id === $dishRecurring->id` to a `->contains()` over the whole relation. `UserDishRepository::findInterferingUserDishes()` controls which dishes enter the candidate pool but not the insert order of `ScheduledUserDish` rows within a schedule — that follows `User::all()` iteration order. The old check silently skipped any schedule where the recurring dish landed on a user who wasn't first, so the test was under-counting. Confirmed in review against the repository source. ### Verification | Check | Before | After | |---|---|---| | Full suite | 1393 / 1393 / 1391 | **1379**, identical ×3 | | `--filter=Schedule` | 673 / 671 | **638**, identical ×3 | | Pint | — | PASS, 229 files | | PHPStan | — | clean | ### Note on `phpstan-baseline.neon` Removing the `reduce()` left an unmatched ignore pattern, which **fails** the analyse gate. The stale entry is removed in the same commit — the baseline change is required, not cosmetic. Suggestion 3 from the description (a fixed clock for the whole suite) was not implemented. The two specific sources are fixed and the count is stable; a suite-wide clock is a larger change worth its own ticket if the class of drift reappears.

myrmidex closed this issue

2026-08-20 20:24:38 +02:00