Skip to content

[#13304] Refactoring for Unit Test Access Controls - #13404

Merged
samuelfangjw merged 32 commits into
TEAMMATES:masterfrom
Rockernun:master
Mar 13, 2026
Merged

[#13304] Refactoring for Unit Test Access Controls#13404
samuelfangjw merged 32 commits into
TEAMMATES:masterfrom
Rockernun:master

Conversation

@Rockernun

@Rockernun Rockernun commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

Part of #13304

Outline of Solution
This PR refactors access-control unit tests to rely on the high-level and mid-level helper methods from BaseActionTest, reducing duplication and clarifying intent. I also split scenarios into narrowly scoped tests so each file exercises one behavior per test.

The refactored files are:

  • GetSessionResultsActionTest.java
  • GetStudentActionTest.java
  • GetStudentsActionTest.java

 

Notes for reviewers
Introduced focused stubbing utilities in GetStudentActionTest to ensure consistent entity ↔ course binding during self-lookup flows:

  • stubSelfLookupAsSameCourse(...)
  • stubSelfLookupAsOtherCourse(...)
  • regKeyParams(...)

Open to moving/renaming the new stubbing helpers to a shared location if preferred. If there are more appropriate BaseActionTest helpers I missed, I’m happy to switch over. Feedback on test naming and assertion granularity is welcome😄

Rockernun and others added 30 commits November 10, 2025 12:12
…ub.com/Rockernun/teammates into test/refactor/sqlui-get-student-action-get-students-action
…ss tests

- Add stubSelfLookupAsSameCourse() and
stubSelfLookupAsOtherCourse()
- Each helper binds the login gid to student.account.googleId and sets
student.course appropriately
…ub.com/Rockernun/teammates into test/refactor/sqlui-get-student-action-get-students-action
- Rename test method names to satisfy Method name rule (remove extra underscores / confirm to pattern)
- Add trailing commas in array initializers to satisfy ArrayTrailingComma
- To satisfy PMD
- Keep its creation scoped to setUp as a local
@mingyuancode
mingyuancode requested a review from Copilot February 2, 2026 17:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@samuelfangjw samuelfangjw self-assigned this Mar 13, 2026
@samuelfangjw samuelfangjw added the c.Task Other non-user-facing works, e.g. refactoring, adding tests label Mar 13, 2026
@samuelfangjw samuelfangjw added this to the V9.0.0-beta.7 milestone Mar 13, 2026
@samuelfangjw
samuelfangjw merged commit 1b122d2 into TEAMMATES:master Mar 13, 2026
9 checks passed
WeeJean pushed a commit to WeeJean/teammates that referenced this pull request Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c.Task Other non-user-facing works, e.g. refactoring, adding tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants