[MBL-19651][All] - Fix E2E tests #3462
Conversation
refs: MBL-19651 affects: Student, Teacher, Parent release note: -
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR refactors help menu test files by extracting hardcoded strings into centralized constants (StringConstants.HelpMenu). This is a positive change that improves maintainability and reduces duplication across Parent, Student, and Teacher app tests.
Positive Observations
- Consistent approach across all three apps (Parent, Student, Teacher)
- Proper use of nested objects for app-specific constants (e.g.,
HelpMenu.Student,HelpMenu.Teacher) - Reduces magic strings in test code, making it easier to maintain when UI text changes
- Good test coverage for help menu functionality
Issues Found
-
Mock data subtitle change unrelated to refactoring (AccountEndpoints.kt:170, 221) - The "Ask the Community" subtitle was changed from "Get help from a Canvas expert" to a much longer text. This appears to be a content change mixed into a refactoring PR. Should verify this aligns with the constant definition and consider separating content changes from refactoring.
-
Unrelated test change (AssignmentDetailsPage.kt:250) - Changed
onView()towaitForView()for attempt spinner assertion. This timing fix is unrelated to string constant refactoring and should be in a separate PR or explained in the description. -
Unrelated test stub (AssignmentsE2ETest.kt:78) - Added
@Stubannotation to a failing test. While this may be necessary, it's unrelated to the help menu refactoring and should be documented or separated.
Verification Needed
To complete this review, I would need to verify:
- The actual definitions in
StringConstants.HelpMenuto ensure all constants are properly defined - Whether the mock data changes align with the constant definitions
- Whether the unrelated changes were intentional bundling or accidental commits
Code Quality
The refactoring work itself is clean and follows good practices. The main concern is the scope creep with unrelated changes mixed into what should be a focused refactoring PR.
| text="Ask the Community", | ||
| subtext="Get help from a Canvas expert" | ||
| subtext="Explore guides, updates, blogs, forums, and resources to help you find answers, collaborate with others, and keep learning." | ||
| ), |
There was a problem hiding this comment.
This subtitle change modifies the expected mock data but I don't see the corresponding constant definition in StringConstants.HelpMenu being updated in this PR.
The subtitle changed from:
- "Get help from a Canvas expert"
To:
- "Explore guides, updates, blogs, forums, and resources to help you find answers, collaborate with others, and keep learning."
This appears to be an intentional content change, but:
- Was this change intentional for this PR, or should it be in a separate PR?
- If the teacher tests are using
HelpMenu.Teacher.ASK_COMMUNITY_SUBTITLE, does that constant have this new longer text? - This change is duplicated at line 170 - both occurrences should be consistent with the constant definition.
|
|
||
| fun assertAttemptSpinnerDisplayed() { | ||
| onView(withId(R.id.attemptSpinner)).assertDisplayed() | ||
| waitForView(withId(R.id.attemptSpinner)).assertDisplayed() |
There was a problem hiding this comment.
This change from onView() to waitForView() seems unrelated to the string constant refactoring. While it may fix a timing issue, it should either:
- Be in a separate PR with its own context/explanation
- Be mentioned in the PR description if it's addressing a specific flakiness issue discovered during testing
Can you clarify if this is related to the help menu string constants work?
|
|
||
| @E2E | ||
| @Test | ||
| @Stub("Worker issue, failing on CI, needs to be fixed in ticket MBL-18749") |
There was a problem hiding this comment.
The addition of @Stub annotation for this test appears unrelated to the help menu string constants refactoring. This should be in a separate commit or PR focused on test stability issues.
If this test was already failing and you're just documenting it, that's fine, but it should be clearly mentioned in the PR description as a separate change.
📊 Code Coverage Report✅ Student
✅ Teacher
✅ Pandautils
📈 Overall Average
|
🧪 Unit Test Results✅ 📱 Parent App
✅ 📱 Student App
✅ 📱 Teacher App
✅ 🌅 Horizon
✅ 📦 Submodules
📊 Summary
Last updated: Tue, 06 Jan 2026 20:57:57 GMT |
|
Parent Install Page |
|
Teacher Install Page |
|
Student Install Page |
Fix E2E tests.
refs: MBL-19651
affects: Student, Teacher, Parent
release note: -