[MBL-19469][Student] - Discussion Checkpoints Notifications E2E Test#3622
Conversation
Refactor notification assertion and click logic to handle contains. refs: MBL-19469 affects: Student release note: -
There was a problem hiding this comment.
Review: Discussion Checkpoints Notifications E2E Test
This PR adds an E2E test for Discussion Checkpoints notifications and refactors NotificationPage to support both exact and partial text matching. The test structure is solid and follows the existing patterns well.
Issues Found
-
Breaking behavior change in
NotificationPage(potential regression) —NotificationPage.ktlines 42–47 and 77–84
The originalassertNotificationDisplayed(title)andclickNotification(title)always usedcontainsTextCaseInsensitive, which does substring + case-insensitive matching. With the new default (contains = false), all existing callers silently switch towithText(title)— exact, case-sensitive matching. Any existing test relying on the old partial/case-insensitive behavior will now fail without any compile-time warning. Consider inverting the default to preserve backward compatibility, or renaming the parameter to make the intent explicit:// Option A: preserve old default behavior fun assertNotificationDisplayed(title: String, exactMatch: Boolean = false) { val matcher = if (exactMatch) { allOf(withText(title), withId(R.id.title)) } else { allOf(containsTextCaseInsensitive(title), withId(R.id.title)) }
-
Potential timezone-related test flakiness —
NotificationsE2ETest.kt,assignmentDetailsDisplayFormatline
getCustomDateCalendar(n)creates a Calendar in UTC (sets hour=10, minute=1, second=1 in UTC). However,assignmentDetailsDisplayFormatdoes not set a timezone, soSimpleDateFormatdefaults to the device's local timezone when formatting. On a CI runner in a non-UTC timezone, the formatted time will differ from what the app displays (which uses UTC). Add an explicit timezone:val assignmentDetailsDisplayFormat = SimpleDateFormat("MMM d, yyyy h:mm a", Locale.US).apply { timeZone = TimeZone.getTimeZone("UTC") }
-
Unused
syllabusBodyinseedData—NotificationsE2ETest.kt,testDiscussionCheckpointsNotificationsE2E
seedData(..., syllabusBody = "this is the syllabus body")seeds data that is never referenced by this test. Removing it reduces noise and avoids unnecessary server calls. -
Inconsistent retry block formatting —
NotificationsE2ETest.kt, retry block
The closing brace ofcatchBlockis placed on the same line as the call (refresh() }), while the main lambda opens on a new line. This looks inconsistent with the Kotlin style used elsewhere in the file:retryWithIncreasingDelay(times = 10, maxDelay = 3000, catchBlock = { refresh() }) { notificationPage.assertNotificationDisplayed(discussionWithCheckpointsTitle) notificationPage.assertNotificationDisplayed("Assignment Created - $discussionWithCheckpointsTitle", contains = true) }
Positive Notes
- Migrating from
StudentTesttoStudentComposeTestis the right call for a test that interacts with Compose-based pages likediscussionDetailsPage. - Using
retryWithIncreasingDelayfor polling notification appearance is a good pattern for flake-resistant E2E tests. - The date formatting split (seed format vs. display format) is thorough and covers the format mismatch between the API and the UI.
- The
contains: Boolean = falseextension to the page object methods is a clean, backward-compatible API addition in concept — just needs the default value adjusted as noted above.
🧪 Unit Test Results✅ 📱 Student App
✅ 🌅 Horizon
✅ 📦 Submodules
📊 Summary
Last updated: Mon, 06 Apr 2026 22:19:09 GMT |
📊 Code Coverage Report✅ Student
✅ Teacher
✅ Pandautils
📈 Overall Average
|
Implement Discussion checkpoint notifications E2E test.
Refactor notification assertion and click logic to handle contains.
refs: MBL-19469
affects: Student
release note: -