From ee29c9bca110d26a1db042ac3240332d2980e441 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Tue, 22 Sep 2026 08:53:16 +0200 Subject: [PATCH] test(store): assert the whole reminder, not a bare minute count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First on-device run of the instrumented suite against the branch tip: 66 tests, one failure, and it was the test that was wrong. `alarmsRoundTripAndReplaceRatherThanAccumulate` asserted `source.alarms()[id] == 30`. The seam stopped returning a bare `Int` in fcee1d1, when collapsing an alarm to a minute count turned out to be what fired an imported START-referenced reminder off DUE — `alarms()` has returned `Map` ever since. The production value was correct (`TaskReminder(minutesBefore=30, fromStart=false)`); only the expectation was left behind. It compiled the whole time because Truth's `isEqualTo` takes `Any?`, so an `Int` compared against a `TaskReminder?` is a perfectly legal call that can only ever be false. Nothing short of running it would have found this, which is the argument for the ROADMAP item that asked for the run. Now asserts the whole value, so the reference is part of the contract rather than something the test is free to ignore. 66/66 green after the fix. --- .../data/tasks/room/RoomTasksDataSourceTest.kt | 8 ++++++-- docs/ROADMAP.md | 14 ++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt index 69f4e33..c7f58f3 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt @@ -5,6 +5,7 @@ import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.common.truth.Truth.assertThat import de.jeanlucmakiola.agendula.data.tasks.TaskQuery +import de.jeanlucmakiola.agendula.data.tasks.TaskReminder import de.jeanlucmakiola.agendula.domain.TaskForm import de.jeanlucmakiola.agendula.domain.TaskStatus import org.junit.After @@ -309,12 +310,15 @@ class RoomTasksDataSourceTest { fun alarmsRoundTripAndReplaceRatherThanAccumulate() { val id = source.insertTask(form(due = now + 1.days)) + // The whole reminder, not just the minute count: collapsing it to a bare + // Int is what fired an imported START-referenced alarm off DUE, and an + // alarm this seam sets from the UI is always due-referenced. source.setAlarm(id, 30) - assertThat(source.alarms()[id]).isEqualTo(30) + assertThat(source.alarms()[id]).isEqualTo(TaskReminder(minutesBefore = 30)) source.setAlarm(id, 60) assertThat(db.alarms().forTask(id)).hasSize(1) - assertThat(source.alarms()[id]).isEqualTo(60) + assertThat(source.alarms()[id]).isEqualTo(TaskReminder(minutesBefore = 60)) source.setAlarm(id, null) assertThat(source.alarms()).doesNotContainKey(id) diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 2d45d84..d67cec8 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -242,10 +242,16 @@ most of it broken, absent or unusable. Reasoning in `aapt` trap: the task can exit non-zero on a fully green run, so read `app/build/outputs/androidTest-results/connected/debug/*.xml` before believing the exit code. -- ⬜ **Re-run it against the branch tip.** That run predates the 4 Sep commits, - which reworked `RoomTasksDataSource`, `ModeRoutingTasksDataSource` and - `RecurrenceExpander` and added instrumented cases of their own — so the tests - covering the store code as it stands have still never executed. +- ✅ **Re-ran it against the branch tip** — 66 tests, 0 failures, Pixel 10 / + API 37, 22 Sep 2026, now including `ExternalImportTest`. Worth having done: + the first run failed one test. + `RoomTasksDataSourceTest.alarmsRoundTripAndReplaceRatherThanAccumulate` + asserted `alarms()[id] == 30`, from before the seam returned a `TaskReminder` + rather than a bare minute count — the production value was right + (`TaskReminder(minutesBefore=30, fromStart=false)`) and the assertion was + stale. It compiled because Truth's `isEqualTo` takes `Any?`, so nothing but + executing it could have caught it. Exactly the defect class this item + existed to find. - ⬜ Verify on a device — and note that the upgrade path this phase was designed around is **not** the one real users are on. `OneShotImport` reads a bundled dmfs provider's `databases/tasks.db`, and no release ever bundled one, so every