Skip to content

Conversation

@kdeakinstructure
Copy link
Contributor

@kdeakinstructure kdeakinstructure commented Nov 27, 2025

Fix custom statuses e2e tests in student and parent (use global variable).

refs: MBL-19566
affects: Student, Teacher, Parent
release note:

  • Run E2E test suite

Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

This PR fixes a test cleanup issue by changing customStatusId from a local variable to a class property assignment, ensuring the @After tearDown method can access it for cleanup. The change is applied consistently to both the Parent and Student E2E tests.

Issues Found

  • Null Safety Concern (apps/parent/src/androidTest/.../CustomStatusesE2ETest.kt:48 & apps/student/src/androidTest/.../CustomStatusesE2ETest.kt:46): If upsertCustomGradeStatus() returns null, the test will continue with a null customStatusId, which could cause unexpected behavior when passed to gradeSubmission(). Consider adding requireNotNull(customStatusId) { "Failed to create custom status" } after the assignment to fail fast with a clear error message.

Positive Feedback

Consistent implementation - The fix is applied identically to both Parent and Student tests, maintaining consistency across the codebase.

Proper cleanup pattern - Using the class property with the @After tearDown ensures custom statuses are properly cleaned up, respecting the 3-status limit mentioned in the comments.

Clear intent - The change correctly addresses the scope issue that was preventing cleanup from working.

Test Coverage

The changes are in E2E test files themselves, so no additional test coverage is needed. The modification allows existing tearDown logic to function properly.

Recommendation

The core fix is correct and necessary. The null safety enhancement suggested in the inline comments would make the tests more robust by providing clearer failure messages if custom status creation fails during test setup.

@github-actions
Copy link

github-actions bot commented Nov 27, 2025

📊 Code Coverage Report

✅ Student

  • PR Coverage: 42.80%
  • Master Coverage: 42.80%
  • Delta: +0.00%

✅ Teacher

  • PR Coverage: 25.45%
  • Master Coverage: 25.45%
  • Delta: +0.00%

⚠️ Pandautils

  • PR Coverage: 22.63%
  • Master Coverage: 22.63%
  • Delta: -0.00%

📈 Overall Average

  • PR Coverage: 30.29%
  • Master Coverage: 30.29%
  • Delta: -0.00%

@github-actions
Copy link

github-actions bot commented Nov 27, 2025

Parent Install Page

@github-actions
Copy link

github-actions bot commented Nov 27, 2025

Student Install Page

@github-actions
Copy link

github-actions bot commented Nov 27, 2025

Teacher Install Page

@kdeakinstructure kdeakinstructure merged commit 4b3f1a4 into master Nov 27, 2025
42 of 46 checks passed
@kdeakinstructure kdeakinstructure deleted the MBL-19566-fix-custom-statuses-e2e-tests branch November 27, 2025 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants