[MBL-19569][Student] - Implement New To Do E2E Tests #3451
Conversation
Fix favorite courses dataseeding process. refs: MBL-19569 affects: Student release note:
…nd todos, past item filters). Fix dataseeding issue with favorite course filter logic. Implement PlannerAPI for seeding calendar todos. Add and modify some apimodels. Add many page objects methods. refs: MBL-19569 affects: Student release note:
…udentTest. refs: MBL-19569 affects: Student release note:
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR significantly enhances the To-Do feature with comprehensive E2E testing and improved filter functionality. The implementation looks solid overall with good test coverage and proper use of Compose testing patterns.
✅ Strengths
- Comprehensive test coverage: Three new E2E tests thoroughly cover the To-Do list functionality including filters, calendar events, personal to-dos, and swipe gestures
- Well-structured tests: Clear test organization with descriptive logging at each step following the PREPARATION/STEP/ASSERTION pattern
- Good use of Compose testing: Proper use of test tags, semantic matching, and the page object pattern
- API integration: New
PlannerAPIproperly integrated with consistent patterns - Accessibility improvements: Better semantic descriptions and test tag organization for UI components
⚠️ Issues Found
- Critical Logic Error (SeedApi.kt:145) - Unused lambda in favorite courses seeding. The lambda wraps the logic but is never executed, causing the favorite courses feature to silently fail. This needs immediate attention.
- Code cleanup needed - Several commented-out code sections that should be removed (CalendarScreenPage.kt:84)
- Formatting inconsistencies - Extra whitespace and blank lines that don't match project style
📋 Code Quality Observations
Positive:
- Consistent use of data seeding patterns for test setup
- Proper handling of calendar-specific edge cases (Sunday checks)
- Good separation of concerns with dedicated filter page objects
- Appropriate use of Kotlin idioms and coroutines
Minor Improvements:
- Consider more specific test tags for better test reliability
- Some test assertions could be more descriptive about expected vs actual states
🔒 Security & Performance
No security concerns identified. The changes are test-focused with proper data isolation. Performance should be unaffected as changes are primarily in test code.
🧪 Test Coverage
Excellent test coverage with three comprehensive E2E tests covering:
- Main To-Do functionality with various date range filters
- Past date filters with edge case handling
- Calendar events, personal to-dos, and swipe gestures
Recommendation
Fix the critical lambda issue in SeedApi.kt before merging, as it will cause the favorite courses feature to not work correctly in tests. The other issues are minor cleanup items that should also be addressed for code quality.
Great work on the comprehensive testing! The filter functionality looks well-designed and thoroughly tested.
| @OptIn(ExperimentalTestApi::class) | ||
| fun assertItemDisplayed(itemTitle: String) { | ||
| composeTestRule.waitUntilExactlyOneExists(hasText(itemTitle), 5000) | ||
| // composeTestRule.waitUntilExactlyOneExists(hasTestTag("eventTitle") and hasText(itemTitle), 5000) |
There was a problem hiding this comment.
The commented code should be removed. The new implementation using waitUntil with a custom condition is better than the commented waitUntilExactlyOneExists approach. Clean up the commented line.
| CoursesApi.addCourseToFavorites(coursesList[it].id,teachersList[0].token) | ||
| // Seed favorite courses for all students | ||
| (0 until minOf(request.favoriteCourses, coursesList.size)).forEach { courseIndex -> | ||
| studentsList.forEach { student -> |
There was a problem hiding this comment.
This lambda appears to be unused. The lambda creates a scope but its result isn't assigned or used. This looks like a logic error - the favorite course logic should be executing immediately, not wrapped in an unused lambda.
Suggested fix:
(0 until minOf(request.favoriteCourses, coursesList.size)).forEach { courseIndex ->
studentsList.forEach { student ->
coursesList[courseIndex].isFavorite = true
addFavoriteCourses(
CoursesApi.addCourseToFavorites(
coursesList[courseIndex].id,
student.token
)
)
}
}| return CanvasNetworkAdapter.retrofitWithToken(token).create(CalendarEventService::class.java) | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Extra blank line added here. While not critical, it's inconsistent with the previous code style. Consider removing it to maintain consistency.
| } | ||
| ) { | ||
| Checkbox( | ||
| Checkbox( |
There was a problem hiding this comment.
Extra space before Checkbox component. Remove for consistent formatting.
📊 Code Coverage Report✅ Student
✅ Teacher
|
🧪 Unit Test Results✅ 📱 Student App
✅ 🌅 Horizon
✅ 📦 Submodules
📊 Summary
Last updated: Wed, 17 Dec 2025 21:21:02 GMT |
|
Student Install Page |
Test Plan
Changes
refs: MBL-19569
affects: Student
release note: none