From c264a219b36b5035d9dd6051cc216183315e48c8 Mon Sep 17 00:00:00 2001 From: Isaries Date: Mon, 31 Aug 2026 10:34:20 +0800 Subject: [PATCH] test: say what the run membership case actually asserts The test was named removeStudent_StudentNotInRun_RemoveStudentFromRun, but APIControllerTest.createWorkgroups builds workgroup2 under run1 with student2 as its member, so student2 is in the run. What the fixture does model is the state the doc comment names as the reason not to reject: in a workgroup, no longer in the period, since run1Period1 holds only student1. The comment also claimed the test showed both steps of removeStudentFromRun are scoped to the run. studentService is mocked here, so nothing about that is exercised and the test would pass identically for a fully enrolled student. It asserts the controller's delegation and the comment now says so. --- .../RemoveStudentRunControllerTest.java | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/wise/portal/presentation/web/controllers/teacher/management/RemoveStudentRunControllerTest.java b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/management/RemoveStudentRunControllerTest.java index f6e87b2ce..95b9b7c43 100644 --- a/src/test/java/org/wise/portal/presentation/web/controllers/teacher/management/RemoveStudentRunControllerTest.java +++ b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/management/RemoveStudentRunControllerTest.java @@ -70,13 +70,21 @@ public void removeStudent_TargetUserIsStudent_RemoveStudentFromRun() throws Exce } /** - * Both steps of removeStudentFromRun are scoped to the run, so a student who is not in the run - * is left untouched. Rejecting them here instead would turn a repeated removal into an error, - * and would leave a student who is still in a workgroup but no longer in a period with no way - * to be cleaned up. + * The student role check is the only thing this controller asks about the target user: it + * deliberately does not ask whether they belong to the run. Write permission on the run is + * checked before it, and removeStudent_NoWritePermission_ThrowAccessDenied covers that half. + * Rejecting a non-member here would turn a repeated removal into an error, and would leave a + * student who is still in a workgroup but no longer in a period with no way to be cleaned up. + * student2 is exactly that case in the fixture, being a member of workgroup2 under run1 while + * run1Period1's own member list holds only student1. + * + *

What this asserts is the delegation and nothing beyond it. studentService is mocked, so the + * claim that both steps of removeStudentFromRun are themselves scoped to the run is not tested + * here and belongs to that service's own tests. */ @Test - public void removeStudent_StudentNotInRun_RemoveStudentFromRun() throws Exception { + public void removeStudent_StudentInWorkgroupButNotInPeriod_RemoveStudentFromRun() + throws Exception { expect(runService.retrieveById(runId1)).andReturn(run1); expect(runService.hasWritePermission(teacherAuth, run1)).andReturn(true); expect(userService.retrieveById(student2Id)).andReturn(student2);