[MBL-17351][Student] Extend AssignmentDetails interaction test with more submission types #3365
Conversation
…types. refs: MBL-17351 affects: Student release note:
…ionTest-with-more-submission-types
…ecording submission scenarios. refs: MBL-17351 affects: Student release note:
…ionTest-with-more-submission-types
…n tests for better clarity. refs: MBL-17351 affects: Student release note:
…ionTest-with-more-submission-types
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Parent Install Page |
📊 Code Coverage Report✅ Student
✅ Teacher
|
|
Teacher Install Page |
|
Student Install Page |
Adds test.txt, test_audio.mp3, and test_video.mp4 to androidTest assets directory. These files are required by AssignmentDetailsInteractionTest for testing file upload, audio, and video submission functionality. Without these files in the repository, the tests fail in CI with FileNotFoundException. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…ionTest-with-more-submission-types
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Parent Install Page |
|
Teacher Install Page |
|
Student Install Page |
refs: MBL-17351 affects: Student release note:
There was a problem hiding this comment.
Review Summary
Great work adding comprehensive test coverage for various assignment submission types! The tests cover important functionality including text entry, file upload, and media recording submissions.
Strengths
- ✅ Good test coverage across multiple submission types
- ✅ Proper use of try-finally blocks for cleanup in most tests
- ✅ Clear test naming following the existing pattern
- ✅ Appropriate use of TestMetaData annotations
Key Improvement Areas
1. Code Duplication: There's significant duplication in test setup code. Consider extracting common patterns into helper methods.
2. Resource Management: One test (testSubmission_submitMediaRecordingRecordVideo) is missing the try-finally pattern for Intents cleanup, which could cause test pollution.
3. Null Safety: Multiple uses of the !! operator without proper null checks could lead to unclear test failures. Consider adding assertions before non-null assertions.
4. Test Stability:
- The audio recording test manipulates files during UI automation which could be timing-dependent
- UiSelector regex patterns like
resourceIdMatches(".*recordAudioButton")are fragile - Hardcoded filenames (e.g., "audio.amr") could break if implementation changes
5. Missing Dependencies: Several helper methods are called but not visible in the diff (stubFilePickerIntent, setupFileOnDevice, copyAssetFileToExternalCache). Ensure these are properly implemented.
Recommendations
- Extract common test setup into a shared helper method
- Create a helper or test rule for managing Espresso Intents lifecycle
- Add explicit null checks with meaningful assertions
- Document or share constants for hardcoded values with production code
- Consider more reliable synchronization for file manipulation tests
The tests are functional and provide valuable coverage. The suggestions above focus on improving maintainability, reliability, and clarity for future developers.
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Show resolved
Hide resolved
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Outdated
Show resolved
Hide resolved
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Outdated
Show resolved
Hide resolved
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Show resolved
Hide resolved
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Show resolved
Hide resolved
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Show resolved
Hide resolved
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Show resolved
Hide resolved
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Show resolved
Hide resolved
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Show resolved
Hide resolved
|
Parent Install Page |
|
Teacher Install Page |
|
Student Install Page |
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Outdated
Show resolved
Hide resolved
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Outdated
Show resolved
Hide resolved
.../androidTest/java/com/instructure/student/ui/interaction/AssignmentDetailsInteractionTest.kt
Outdated
Show resolved
Hide resolved
|
There are many places where you use fully classified names instead of imports, I would fix those (haven't commented on all of those though). |
refs: MBL-17351 affects: Student release note:
…ionTest-with-more-submission-types
|
Parent Install Page |
|
Teacher Install Page |
|
Student Install Page |
|
Student Install Page |
|
Student Install Page |
adamNagy56
deleted the
MBL-17351-Extend-AssignmentDetailsInteractionTest-with-more-submission-types
branch
Summary
Extend AssignmentDetails interaction test with more submission types. (TextEntry, FileUpload, different MediaRecording types)
refs: MBl-17351
affects: Student
release note: