Skip to content

Commit 4a97277

Browse files
refactor: Current time millis cleanup (#222)
* feat: add TestTimeConstants for deterministic test times Creates TestTimeConstants object with STANDARD_TEST_TIME (June 15, 2026 noon UTC) to replace System.currentTimeMillis() in tests. This prevents flaky tests caused by midnight/boundary crossing issues. Part of System.currentTimeMillis() removal effort - see docs/dev_todo/system_current_time_millis_removal.md Co-authored-by: wharris <wharris@upscalews.com> * refactor: update test fixtures to use TestTimeConstants - MockTimeProvider: use STANDARD_TEST_TIME as default - UITestFixture/UITestFixtureRobolectric: use constant in event creation - BaseCalendarTestFixture/CalendarProviderTestFixture: use constant for clock reset - MockContextProvider: use timeProvider instead of System.currentTimeMillis() - MockDismissedEventsStorage: use constant for timestamp defaults Co-authored-by: wharris <wharris@upscalews.com> * refactor: remove System.currentTimeMillis() from Robolectric tests Updated test files to use TestTimeConstants.STANDARD_TEST_TIME: - DateTimeUtilsTest.kt - FilterStateTest.kt - MonitorEventAlertEntryPreMutedTest.kt - CalendarIntentsRobolectricTest.kt - ViewEventActivityStateTest.kt - UpcomingEventsLookaheadTest.kt - TagsManagerRobolectricTest.kt Co-authored-by: wharris <wharris@upscalews.com> * refactor: remove System.currentTimeMillis() from instrumentation tests Updated test files to use TestTimeConstants.STANDARD_TEST_TIME: - EventDismissTest.kt - PreDismissIntegrationTest.kt - PreSnoozeIntegrationTest.kt - PreMuteIntegrationTest.kt - ApplicationControllerCoreTest.kt - TagsManagerTest.kt - CalendarMonitorServiceTest.kt - CalendarMonitorServiceTestFirstScanEver.kt - CalendarMonitorServiceEventReminderTest.kt - RoomCrSqlitePocTest.kt Co-authored-by: wharris <wharris@upscalews.com> * refactor: remove System.currentTimeMillis() from production code - SettingsBackupManager: inject CNPlusClockInterface with CNPlusSystemClock default - TestActivity: use existing clock property for unique title generation Co-authored-by: wharris <wharris@upscalews.com> * docs: mark System.currentTimeMillis() removal as complete - Update plan doc status to COMPLETED - Add repo rule: Never use System.currentTimeMillis() directly - Document verification results Co-authored-by: wharris <wharris@upscalews.com> * fix: use clock provider pattern in UpcomingEventsFragment - Change hardcoded CNPlusSystemClock to use getClock() like ActiveEventsFragment - Add testClock to UITestFixtureRobolectric - Inject testClock into both ActiveEventsFragment and UpcomingEventsFragment This ensures tests using TestTimeConstants.STANDARD_TEST_TIME have consistent time across fixture event creation and fragment filtering logic. Co-authored-by: wharris <wharris@upscalews.com> * fix: inject testClock into ApplicationController for instrumented tests Integration tests were failing because: 1. ApplicationController.clock was hardcoded to use CNPlusSystemClock() (real time) 2. Tests created events with times based on TestTimeConstants.STANDARD_TEST_TIME (June 2026) 3. Time-dependent operations like dismissAllButRecentAndSnoozed compared event times against real system time Changes: - Add clockProvider pattern to ApplicationController (similar to UI fragments) - Add resetClockProvider() method for test cleanup - Inject testClock into ApplicationController in: - ApplicationControllerCoreTest - CalendarMonitorServiceTest - CalendarMonitorServiceTestFirstScanEver - CalendarMonitorServiceEventReminderTest - UITestFixture (for UpcomingEventsFragmentTest) - Update UITestFixture.mockCalendarProvider to use testClock Co-authored-by: wharris <wharris@upscalews.com> * fix: pass testClock to CalendarMonitorManual in testFirstScanEver CalendarMonitorManual was created without testClock, causing it to use real system time for scan range calculations while test events used TestTimeConstants.STANDARD_TEST_TIME. This caused alerts not to be found in the second scan because the scan range was based on real time. Co-authored-by: wharris <wharris@upscalews.com> * feat: change STANDARD_TEST_TIME to December 23, 2023 and fix DismissedEventsFragment clock injection Address PR review comments: - Change test time from June 15, 2026 to December 23, 2023 (day before Christmas Eve) - Add missing DismissedEventsFragment.clockProvider injection in UITestFixture - Add missing DismissedEventsFragment.clockProvider injection in UITestFixtureRobolectric - Update docs/dev_todo/system_current_time_millis_removal.md to reflect new date Co-authored-by: wharris <wharris@upscalews.com> * fix: correct date to December 25, 2023 (Christmas Day) Co-authored-by: wharris <wharris@upscalews.com> * fix: add missing ApplicationController.clockProvider injections - Add ApplicationController.clockProvider to UITestFixtureRobolectric (bugbot finding) - Add ApplicationController.resetClockProvider() to UITestFixtureRobolectric cleanup - Add clock injection to PreMuteIntegrationTest which uses ApplicationController.registerNewEvents() Thorough audit confirms all test fixtures now have proper clock injection: - UITestFixture (androidTest): fragments + ApplicationController - UITestFixtureRobolectric (test): fragments + ApplicationController - MockApplicationComponents: mocks ApplicationController.clock - Individual integration tests: inject via clockProvider or MockK Co-authored-by: wharris <wharris@upscalews.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent e4561b6 commit 4a97277

33 files changed

Lines changed: 436 additions & 93 deletions

.cursor/rules/main-rules.mdc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,13 @@ Copyright (C) 2025 William Harris (wharris+cnplus@upscalews.com)
6060

6161
# Never Catch the broad Exception class
6262

63-
no catching Exception e. it can lead to hiding important bugs to catch. always do something more specific
63+
no catching Exception e. it can lead to hiding important bugs to catch. always do something more specific
64+
65+
# Never use System.currentTimeMillis() directly
66+
67+
Use `CNPlusClockInterface` instead - it enables testable time-dependent code.
68+
69+
**Production code:** Inject `CNPlusSystemClock()` or access via interface property
70+
**Test code:** Use `TestTimeConstants.STANDARD_TEST_TIME` or `CNPlusTestClock`
71+
72+
See `docs/architecture/clock_implementation.md` and `docs/dev_todo/system_current_time_millis_removal.md` for details.

android/app/src/androidTest/java/com/github/quarck/calnotify/app/ApplicationControllerCoreTest.kt

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import com.github.quarck.calnotify.calendar.EventStatus
1212
import com.github.quarck.calnotify.dismissedeventsstorage.EventDismissType
1313
import com.github.quarck.calnotify.eventsstorage.EventsStorage
1414
import com.github.quarck.calnotify.logs.DevLog
15+
import com.github.quarck.calnotify.testutils.TestTimeConstants
16+
import com.github.quarck.calnotify.utils.CNPlusTestClock
1517
import org.junit.After
1618
import org.junit.Assert.*
1719
import org.junit.Before
@@ -30,13 +32,18 @@ class ApplicationControllerCoreTest {
3032

3133
private val LOG_TAG = "ApplicationControllerCoreTest"
3234
private lateinit var context: Context
33-
private val baseTime = System.currentTimeMillis()
35+
private val baseTime = TestTimeConstants.STANDARD_TEST_TIME
36+
private lateinit var testClock: CNPlusTestClock
3437

3538
@Before
3639
fun setup() {
3740
DevLog.info(LOG_TAG, "Setting up test")
3841
context = InstrumentationRegistry.getInstrumentation().targetContext
3942

43+
// Set up test clock to use the same time as our test events
44+
testClock = CNPlusTestClock(baseTime)
45+
ApplicationController.clockProvider = { testClock }
46+
4047
// Clear any existing test events
4148
EventsStorage(context).use { db ->
4249
db.events.filter { it.title.startsWith("Test Event") }.forEach {
@@ -48,6 +55,9 @@ class ApplicationControllerCoreTest {
4855
@After
4956
fun cleanup() {
5057
DevLog.info(LOG_TAG, "Cleaning up test")
58+
// Reset the clock provider to avoid test pollution
59+
ApplicationController.resetClockProvider()
60+
5161
// Clean up test events
5262
EventsStorage(context).use { db ->
5363
db.events.filter { it.title.startsWith("Test Event") }.forEach {
@@ -235,7 +245,7 @@ class ApplicationControllerCoreTest {
235245
// Recent event
236246
val recentEvent = createTestEvent(
237247
eventId = eventId,
238-
lastStatusChangeTime = System.currentTimeMillis() // Just now
248+
lastStatusChangeTime = baseTime // Just now (relative to baseTime)
239249
)
240250
EventsStorage(context).use { db ->
241251
db.addEvent(recentEvent)
@@ -258,7 +268,7 @@ class ApplicationControllerCoreTest {
258268
val snoozedEvent = createTestEvent(
259269
eventId = eventId,
260270
lastStatusChangeTime = baseTime - Consts.DISMISS_ALL_THRESHOLD - 60000L,
261-
snoozedUntil = System.currentTimeMillis() + 3600000L
271+
snoozedUntil = baseTime + 3600000L
262272
)
263273
EventsStorage(context).use { db ->
264274
db.addEvent(snoozedEvent)
@@ -299,7 +309,7 @@ class ApplicationControllerCoreTest {
299309
val eventId = 100031L
300310
val event = createTestEvent(
301311
eventId = eventId,
302-
snoozedUntil = System.currentTimeMillis() + 3600000L,
312+
snoozedUntil = baseTime + 3600000L,
303313
isMuted = false
304314
)
305315
EventsStorage(context).use { db ->

android/app/src/androidTest/java/com/github/quarck/calnotify/app/PreDismissIntegrationTest.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import com.github.quarck.calnotify.dismissedeventsstorage.DismissedEventsStorage
3333
import com.github.quarck.calnotify.eventsstorage.EventsStorage
3434
import com.github.quarck.calnotify.logs.DevLog
3535
import com.github.quarck.calnotify.monitorstorage.MonitorStorage
36+
import com.github.quarck.calnotify.testutils.TestTimeConstants
3637
import com.github.quarck.calnotify.utils.CNPlusTestClock
3738
import io.mockk.every
3839
import io.mockk.mockkObject
@@ -58,7 +59,7 @@ class PreDismissIntegrationTest {
5859

5960
private val LOG_TAG = "PreDismissIntegrationTest"
6061
private lateinit var context: Context
61-
private val baseTime = System.currentTimeMillis()
62+
private val baseTime = TestTimeConstants.STANDARD_TEST_TIME
6263
private var testEventId = 920000L // Use high IDs to avoid conflicts
6364
private lateinit var testClock: CNPlusTestClock
6465

android/app/src/androidTest/java/com/github/quarck/calnotify/app/PreMuteIntegrationTest.kt

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ import com.github.quarck.calnotify.calendar.MonitorEventAlertEntry
3232
import com.github.quarck.calnotify.eventsstorage.EventsStorage
3333
import com.github.quarck.calnotify.logs.DevLog
3434
import com.github.quarck.calnotify.monitorstorage.MonitorStorage
35+
import com.github.quarck.calnotify.testutils.TestTimeConstants
36+
import com.github.quarck.calnotify.utils.CNPlusTestClock
37+
import com.github.quarck.calnotify.app.ApplicationController
3538
import org.junit.After
3639
import org.junit.Assert.*
3740
import org.junit.Before
@@ -51,21 +54,30 @@ class PreMuteIntegrationTest {
5154

5255
private val LOG_TAG = "PreMuteIntegrationTest"
5356
private lateinit var context: Context
54-
private val baseTime = System.currentTimeMillis()
57+
private val baseTime = TestTimeConstants.STANDARD_TEST_TIME
5558
private var testEventId = 900000L // Use high IDs to avoid conflicts
59+
private lateinit var testClock: CNPlusTestClock
5660

5761
@Before
5862
fun setup() {
5963
DevLog.info(LOG_TAG, "Setting up test")
6064
context = InstrumentationRegistry.getInstrumentation().targetContext
6165

66+
// Set up test clock for deterministic time
67+
testClock = CNPlusTestClock(baseTime)
68+
ApplicationController.clockProvider = { testClock }
69+
6270
// Clean up any leftover test data
6371
cleanupTestData()
6472
}
6573

6674
@After
6775
fun cleanup() {
6876
DevLog.info(LOG_TAG, "Cleaning up test")
77+
78+
// Reset clock provider
79+
ApplicationController.resetClockProvider()
80+
6981
cleanupTestData()
7082
}
7183

android/app/src/androidTest/java/com/github/quarck/calnotify/app/PreSnoozeIntegrationTest.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import com.github.quarck.calnotify.calendar.MonitorEventAlertEntry
3232
import com.github.quarck.calnotify.eventsstorage.EventsStorage
3333
import com.github.quarck.calnotify.logs.DevLog
3434
import com.github.quarck.calnotify.monitorstorage.MonitorStorage
35+
import com.github.quarck.calnotify.testutils.TestTimeConstants
3536
import org.junit.After
3637
import org.junit.Assert.*
3738
import org.junit.Before
@@ -53,7 +54,7 @@ class PreSnoozeIntegrationTest {
5354

5455
private val LOG_TAG = "PreSnoozeIntegrationTest"
5556
private lateinit var context: Context
56-
private val baseTime = System.currentTimeMillis()
57+
private val baseTime = TestTimeConstants.STANDARD_TEST_TIME
5758
private var testEventId = 910000L // Use high IDs to avoid conflicts
5859

5960
@Before

android/app/src/androidTest/java/com/github/quarck/calnotify/app/TagsManagerTest.kt

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import com.github.quarck.calnotify.calendar.EventDisplayStatus
99
import com.github.quarck.calnotify.calendar.EventOrigin
1010
import com.github.quarck.calnotify.calendar.EventStatus
1111
import com.github.quarck.calnotify.logs.DevLog
12+
import com.github.quarck.calnotify.testutils.TestTimeConstants
1213
import org.junit.Assert.*
1314
import org.junit.Before
1415
import org.junit.Test
@@ -36,26 +37,27 @@ class TagsManagerTest {
3637
title: String = "Test Event",
3738
desc: String = "Test Description"
3839
): EventAlertRecord {
40+
val now = TestTimeConstants.STANDARD_TEST_TIME
3941
return EventAlertRecord(
4042
calendarId = 1L,
4143
eventId = 1L,
4244
isAllDay = false,
4345
isRepeating = false,
44-
alertTime = System.currentTimeMillis(),
46+
alertTime = now,
4547
notificationId = 0,
4648
title = title,
4749
desc = desc,
48-
startTime = System.currentTimeMillis(),
49-
endTime = System.currentTimeMillis() + 3600000,
50-
instanceStartTime = System.currentTimeMillis(),
51-
instanceEndTime = System.currentTimeMillis() + 3600000,
50+
startTime = now,
51+
endTime = now + 3600000,
52+
instanceStartTime = now,
53+
instanceEndTime = now + 3600000,
5254
location = "",
53-
lastStatusChangeTime = System.currentTimeMillis(),
55+
lastStatusChangeTime = now,
5456
snoozedUntil = 0L,
5557
displayStatus = EventDisplayStatus.Hidden,
5658
color = 0,
5759
origin = EventOrigin.ProviderBroadcast,
58-
timeFirstSeen = System.currentTimeMillis(),
60+
timeFirstSeen = now,
5961
eventStatus = EventStatus.Confirmed,
6062
attendanceStatus = AttendanceStatus.None,
6163
flags = 0

android/app/src/androidTest/java/com/github/quarck/calnotify/database/poc/RoomCrSqlitePocTest.kt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
2424
import androidx.test.platform.app.InstrumentationRegistry
2525
import com.github.quarck.calnotify.eventsstorage.LegacyEventsStorage
2626
import com.github.quarck.calnotify.logs.DevLog
27+
import com.github.quarck.calnotify.testutils.TestTimeConstants
2728
import org.junit.After
2829
import org.junit.Assert.*
2930
import org.junit.Before
@@ -141,7 +142,7 @@ class RoomCrSqlitePocTest {
141142
val entity1 = RoomPocEntity(
142143
id = 1,
143144
name = "Test Entity 1",
144-
timestamp = System.currentTimeMillis(),
145+
timestamp = TestTimeConstants.STANDARD_TEST_TIME,
145146
description = "First test entity"
146147
)
147148
val insertResult = dao.insert(entity1)
@@ -231,7 +232,7 @@ class RoomCrSqlitePocTest {
231232
RoomPocEntity(
232233
id = i.toLong(),
233234
name = "Entity $i",
234-
timestamp = System.currentTimeMillis() + i,
235+
timestamp = TestTimeConstants.STANDARD_TEST_TIME + i,
235236
description = "Description for entity $i"
236237
)
237238
}
@@ -278,7 +279,7 @@ class RoomCrSqlitePocTest {
278279
val dao = database!!.pocDao()
279280

280281
// Do some operations
281-
dao.insert(RoomPocEntity(1, "Test", System.currentTimeMillis()))
282+
dao.insert(RoomPocEntity(1, "Test", TestTimeConstants.STANDARD_TEST_TIME))
282283
val retrieved = dao.getById(1)
283284
assertNotNull(retrieved)
284285

android/app/src/androidTest/java/com/github/quarck/calnotify/deprecated_raw_calendarmonitor/CalendarMonitorServiceEventReminderTest.kt

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import com.github.quarck.calnotify.textutils.EventFormatterInterface
4141
import java.util.Locale
4242
import com.github.quarck.calnotify.app.AlarmSchedulerInterface
4343
import com.github.quarck.calnotify.ui.UINotifier
44+
import com.github.quarck.calnotify.testutils.TestTimeConstants
4445
import com.github.quarck.calnotify.utils.cancelExactAndAlarm
4546
import com.github.quarck.calnotify.utils.CNPlusTestClock
4647
import org.junit.Ignore
@@ -134,6 +135,10 @@ class CalendarMonitorServiceEventReminderTest {
134135
@After
135136
fun cleanup() {
136137
DevLog.info(LOG_TAG, "Cleaning up test environment")
138+
139+
// Reset the clock provider before unmocking to avoid test pollution
140+
ApplicationController.resetClockProvider()
141+
137142
unmockkAll()
138143

139144
// Delete test events and calendar
@@ -622,9 +627,12 @@ class CalendarMonitorServiceEventReminderTest {
622627

623628
private fun setupMockTimer() {
624629
// Create CNPlusTestClock with mockTimer - it will automatically set up the mock
625-
testClock = CNPlusTestClock(System.currentTimeMillis(), mockTimer)
630+
testClock = CNPlusTestClock(TestTimeConstants.STANDARD_TEST_TIME, mockTimer)
626631
currentTime.set(testClock.currentTimeMillis())
627632

633+
// Inject testClock into ApplicationController so time-dependent operations use the same clock
634+
ApplicationController.clockProvider = { testClock }
635+
628636
// No need to manually configure mockTimer's schedule behavior anymore
629637
// as this is now handled by CNPlusTestClock's init block
630638
}

android/app/src/androidTest/java/com/github/quarck/calnotify/deprecated_raw_calendarmonitor/CalendarMonitorServiceTest.kt

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import com.github.quarck.calnotify.NotificationSettings
4545
import com.github.quarck.calnotify.notification.EventNotificationManager
4646
import com.github.quarck.calnotify.globalState
4747
import org.junit.Ignore
48+
import com.github.quarck.calnotify.testutils.TestTimeConstants
4849
import com.github.quarck.calnotify.utils.CNPlusClockInterface
4950
import com.github.quarck.calnotify.utils.CNPlusTestClock
5051

@@ -339,7 +340,10 @@ class CalendarMonitorServiceTest {
339340

340341
private fun setupMockTimer() {
341342
// Create CNPlusTestClock with mockTimer - it will automatically set up the mock
342-
testClock = CNPlusTestClock(System.currentTimeMillis(), mockTimer)
343+
testClock = CNPlusTestClock(TestTimeConstants.STANDARD_TEST_TIME, mockTimer)
344+
345+
// Inject testClock into ApplicationController so time-dependent operations use the same clock
346+
ApplicationController.clockProvider = { testClock }
343347

344348
// No need to manually configure mockTimer's schedule behavior anymore
345349
// as this is now handled by CNPlusTestClock's init block
@@ -775,6 +779,10 @@ class CalendarMonitorServiceTest {
775779
@After
776780
fun cleanup() {
777781
DevLog.info(LOG_TAG, "Cleaning up test environment")
782+
783+
// Reset the clock provider before unmocking to avoid test pollution
784+
ApplicationController.resetClockProvider()
785+
778786
unmockkAll()
779787

780788
// Delete test events and calendar
@@ -819,7 +827,7 @@ class CalendarMonitorServiceTest {
819827
monitorState.firstScanEver = false
820828

821829
// Use consistent time method
822-
testClock.setCurrentTime(System.currentTimeMillis())
830+
testClock.setCurrentTime(TestTimeConstants.STANDARD_TEST_TIME)
823831
val startTime = testClock.currentTimeMillis() // Capture initial time
824832

825833
// Calculate event times first

android/app/src/androidTest/java/com/github/quarck/calnotify/deprecated_raw_calendarmonitor/CalendarMonitorServiceTestFirstScanEver.kt

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import com.github.quarck.calnotify.NotificationSettings
4545
import com.github.quarck.calnotify.notification.EventNotificationManager
4646
import com.github.quarck.calnotify.globalState
4747
import org.junit.Ignore
48+
import com.github.quarck.calnotify.testutils.TestTimeConstants
4849
import com.github.quarck.calnotify.utils.CNPlusClockInterface
4950
import com.github.quarck.calnotify.utils.CNPlusTestClock
5051

@@ -341,7 +342,10 @@ class CalendarMonitorServiceTestFirstScanEver {
341342

342343
private fun setupMockTimer() {
343344
// Create CNPlusTestClock with mockTimer - it will automatically set up the mock
344-
testClock = CNPlusTestClock(System.currentTimeMillis(), mockTimer)
345+
testClock = CNPlusTestClock(TestTimeConstants.STANDARD_TEST_TIME, mockTimer)
346+
347+
// Inject testClock into ApplicationController so time-dependent operations use the same clock
348+
ApplicationController.clockProvider = { testClock }
345349

346350
// No need to manually configure mockTimer's schedule behavior anymore
347351
// as this is now handled by CNPlusTestClock's init block
@@ -350,7 +354,8 @@ class CalendarMonitorServiceTestFirstScanEver {
350354
private fun setupMockCalendarMonitor() {
351355

352356
// Mock the key method in CalendarMonitorManual that handles due alerts during firstScanEver
353-
val realManualScanner = CalendarMonitorManual(CalendarProvider)
357+
// NOTE: Must pass testClock so scan ranges are calculated using test time, not real time
358+
val realManualScanner = CalendarMonitorManual(CalendarProvider, testClock)
354359

355360
spyManualScanner = spyk(realManualScanner)
356361

@@ -969,6 +974,10 @@ class CalendarMonitorServiceTestFirstScanEver {
969974
@After
970975
fun cleanup() {
971976
DevLog.info(LOG_TAG, "Cleaning up test environment")
977+
978+
// Reset the clock provider before unmocking to avoid test pollution
979+
ApplicationController.resetClockProvider()
980+
972981
unmockkAll()
973982

974983
// Delete test events and calendar

0 commit comments

Comments
 (0)