-
Notifications
You must be signed in to change notification settings - Fork 108
[MBL-19509][Teacher] - Implement E2E test for disabled front page course home selection flow #3433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[MBL-19509][Teacher] - Implement E2E test for disabled front page course home selection flow #3433
Conversation
…election Add E2E test to verify that the "Pages Front Page" option in course settings is disabled when no front page exists, and enabled when a front page is present. Changes: - Added assertRadioButtonClickable() and assertRadioButtonNotClickable() methods to CourseSettingsPage for checking radio button enabled state - Added selectNewHomePageOption() method to select specific home page option by text - Added cancelNewHomePageSelectionDialog() method to cancel the dialog - Renamed assertHomePageChanged() to assertHomePageText() for clarity - Added comprehensive E2E test testCannotSelectCourseSettingsFrontPageIfNoFrontPageE2E() that: * Verifies "Pages Front Page" is disabled when no front page exists * Creates a front page and verifies the option becomes enabled * Removes the front page and verifies the option is disabled again * Checks that course home falls back to "Course Activity Stream" when front page is removed Technical details: - Radio buttons are found by their text content using onViewWithText() - Uses assertEnabled() and assertDisabled() extension functions for clickability checks - Test covers the full lifecycle of front page creation and removal refs: MBL-19509 affects: Teacher release note: none 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🧪 Unit Test Results📊 Summary
Last updated: Thu, 11 Dec 2025 08:46:13 GMT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary - MBL-19486: Course Settings Front Page Selection
This PR adds comprehensive E2E test coverage for handling the "Pages Front Page" radio button in Course Settings when no front page exists. The implementation is solid and follows the project's testing patterns well.
✅ Strengths
- Well-structured test: The new E2E test thoroughly covers the bug scenario with proper setup, execution, and verification
- Good refactoring: Renamed
assertHomePageChanged()toassertHomePageText()which better describes what the method does - Helpful utilities: Added
selectNewHomePageOption(),cancelNewHomePageSelectionDialog(),assertRadioButtonClickable(), andassertRadioButtonNotClickable()methods that improve test expressiveness - Follows patterns: Consistent with existing test structure and page object pattern from the
:espressomodule - Proper test metadata: Tagged with
@E2E,Priority.BUG_CASE, and the ticket number
📋 Minor Issues Found
- Code duplication: The test has repetitive setup/verification logic (lines 145-174 and 193-224) that could be extracted into helper methods
- Typos in comments: Several assertion log messages have grammatical issues with "yet" vs "now" usage
- Consistency: Mixed phrasing across assertion messages could be standardized
Security & Performance
✅ No security concerns identified
✅ No performance issues - test uses standard Espresso patterns
✅ Proper use of data seeding API for test isolation
Test Coverage
✅ Excellent coverage of the bug scenario:
- Tests radio button disabled state when no front page exists
- Verifies radio button becomes enabled after creating a front page
- Confirms fallback behavior when front page is removed
- Validates all other radio buttons remain clickable throughout
Code Quality
The code follows Canvas Android conventions well:
- Proper use of logging tags (STEP_TAG, ASSERTION_TAG, PREPARATION_TAG)
- Consistent with MVVM and page object patterns
- Good separation of concerns in page object methods
- Appropriate use of Espresso navigation patterns
Overall, this is a quality addition that provides valuable regression protection for MBL-19486. The inline comments address minor improvements that would enhance maintainability.
...teacher/src/androidTest/java/com/instructure/teacher/ui/e2e/classic/CourseSettingsE2ETest.kt
Show resolved
Hide resolved
...teacher/src/androidTest/java/com/instructure/teacher/ui/e2e/classic/CourseSettingsE2ETest.kt
Show resolved
Hide resolved
...teacher/src/androidTest/java/com/instructure/teacher/ui/e2e/classic/CourseSettingsE2ETest.kt
Show resolved
Hide resolved
...teacher/src/androidTest/java/com/instructure/teacher/ui/e2e/classic/CourseSettingsE2ETest.kt
Show resolved
Hide resolved
📊 Code Coverage Report✅ Student
✅ Teacher
|
adamNagy56
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I think the teacher e2e test suit shouldn't be skipped because there is an extra test added.
...teacher/src/androidTest/java/com/instructure/teacher/ui/e2e/classic/CourseSettingsE2ETest.kt
Outdated
Show resolved
Hide resolved
...teacher/src/androidTest/java/com/instructure/teacher/ui/e2e/classic/CourseSettingsE2ETest.kt
Outdated
Show resolved
Hide resolved
Yeah, I've generated the PR with Claude and it skipped the E2E, I started it manually via Bitrise, here's the successful run: |
Summary
assertHomePageChanged→assertHomePageText)Test plan
testCannotSelectCourseSettingsFrontPageIfNoFrontPageE2Eand verify it passestestCourseSettingsE2Estill passes after method refactoringCourseSettingsInteractionTeststill passes🤖 Generated with Claude Code