[MBL-19264][Student][Teacher] Unstub SpeedGrader comments-related interaction tests #3434
Conversation
… SubmissionDetailsInteractionTest.kt refs: MBL-19264 affects: Student, Teacher release note:
…-interaction-tests
…larity and functionality. refs: MBL-19264 affects: Student, Teacher release note:
…-interaction-tests
![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 successfully implements previously stubbed UI tests for comment functionality in both the Student and Teacher apps. The changes demonstrate good test coverage for video/audio comment playback and comment library features.
Positive Aspects ✅
- Comprehensive test implementation: All 12 previously stubbed tests are now fully implemented with proper assertions
- Good test isolation: Tests properly set up mock data and use the FakeManagers pattern consistently
- Proper use of page objects: Follows the existing page object pattern from the espresso module
- UI modernization: Updated to use Compose testing APIs consistently (e.g.,
onNodeWithTag,performScrollTo) - Good helper functions: The
createVideoAttachmentandcreateAudioAttachmenthelpers reduce code duplication - Improved FakeSubmissionCommentsManager: Properly implements comment retrieval and creation with GraphQL mapping
Issues to Address 📋
- Import ordering: Multiple test files have imports that are not alphabetically sorted (CommentLibraryInteractionTest.kt:49, SpeedGraderCommentsInteractionTest.kt:47). According to CLAUDE.md, imports should follow Kotlin style guides and be sorted alphabetically.
- Hard-coded test strings: Consider extracting repeated comment text strings into constants (SpeedGraderCommentsInteractionTest.kt:270) for easier maintenance.
Code Quality Observations
Test Structure: The refactoring of goToSpeedGraderCommentsPage() to accept flexible parameters (commentText, isTeacherComment, commentCount) is well-designed and makes tests more readable.
Wait Strategy: The use of waitUntilExactlyOneExists with 5-second timeouts is appropriate for UI rendering scenarios and should help with test stability.
Thread Safety Note: The FakeSubmissionCommentsManager.createSubmissionComment() implementation mutates the submission's comments list, which is appropriate for test purposes. This should be safe as MockCanvas data is typically per-test instance, but worth noting for future maintainers if parallel test execution is added.
Overall Assessment
This is solid test implementation work that significantly improves test coverage for comment functionality. The minor issues noted above are primarily style concerns and don't affect functionality. Great job implementing these complex UI interaction tests!
...src/androidTest/java/com/instructure/teacher/ui/interaction/CommentLibraryInteractionTest.kt
Show resolved
Hide resolved
...ndroidTest/java/com/instructure/teacher/ui/interaction/SpeedGraderCommentsInteractionTest.kt
Show resolved
Hide resolved
...ndroidTest/java/com/instructure/teacher/ui/interaction/SpeedGraderCommentsInteractionTest.kt
Show resolved
Hide resolved
...ain/kotlin/com/instructure/canvas/espresso/mockcanvas/fakes/FakeSubmissionCommentsManager.kt
Show resolved
Hide resolved
📊 Code Coverage Report✅ Student
✅ Teacher
|
🧪 Unit Test Results✅ 📱 Student App
✅ 📱 Teacher App
✅ 🌅 Horizon
✅ 📦 Submodules
📊 Summary
Last updated: Tue, 09 Dec 2025 13:20:11 GMT |
|
Student Install Page |
|
Teacher Install Page |
refs: MBL-19264 affects: Student, Teacher release note:
Summary
Unstubbed and refactored SpeedGrader comment and comment library interaction tests to work with the new Compose-based UI implementation.
Test Coverage
Tests validate comment library filtering, selection, sending comments, empty states, and clearing filters.
refs: MBL-19264
affects: Student, Teacher
release note:
Checklist