test(store): assert the whole reminder, not a bare minute count
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<Long, TaskReminder>` 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.
This commit is contained in:
+6
-2
@@ -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)
|
||||
|
||||
+10
-4
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user