Skip to content

Commit

Permalink
improvde code after review
Browse files Browse the repository at this point in the history
  • Loading branch information
krusche committed Jan 4, 2025
1 parent e395120 commit 9aedbd5
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ public void notifyUserAboutNewResult(Result result, ProgrammingExerciseParticipa

/**
* Notify Iris about the submission status for the given result and student participation.
* Only if the user has accepted Iris, the exercise is not an exam exercise, and the exercise chat is enabled in the exercise settings
* Only notifies if the user has accepted Iris, the exercise is not an exam exercise, and the exercise chat is enabled in the exercise settings
* NOTE: we check those settings early to prevent unnecessary database queries and exceptions later on in most cases. More sophisticated checks are done in the Iris service.
* <p>
* If the submission was successful, Iris will be informed about the successful submission.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ void testDeleteOldDockerImages_NoOutdatedImages() {
buildAgentDockerService.deleteOldDockerImages();

// Verify that removeImageCmd() was not called.
verify(dockerClient, times(0)).removeImageCmd(anyString());
verify(dockerClient, never()).removeImageCmd(anyString());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.RETURNS_MOCKS;
import static org.mockito.Mockito.after;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.timeout;
Expand Down Expand Up @@ -139,7 +140,7 @@ void testPatchModelingSubmissionWithWrongPrincipal() {
// when we submit a patch, but with the wrong user ...
participationTeamWebsocketService.patchModelingSubmission(participation.getId(), patch, getPrincipalMock("student2"));
// the patch should not be broadcast.
verify(websocketMessagingService, timeout(2000).times(0)).sendMessage(websocketTopic(participation), List.of());
verify(websocketMessagingService, after(1000).never()).sendMessage(websocketTopic(participation), List.of());
}

@Test
Expand All @@ -152,7 +153,7 @@ void testUpdateModelingSubmission() {
// the submission should be handled by the service (i.e. saved), ...
verify(modelingSubmissionService, timeout(2000).times(1)).handleModelingSubmission(any(), any(), any());
// but it should NOT be broadcast (sync is handled with patches only).
verify(websocketMessagingService, timeout(2000).times(0)).sendMessage(websocketTopic(participation), List.of());
verify(websocketMessagingService, after(1000).never()).sendMessage(websocketTopic(participation), List.of());
}

@Test
Expand All @@ -163,9 +164,9 @@ void testUpdateModelingSubmissionWithWrongPrincipal() {
// when we submit a new modeling submission with the wrong user ...
participationTeamWebsocketService.updateModelingSubmission(participation.getId(), submission, getPrincipalMock("student2"));
// the submission is NOT saved ...
verify(modelingSubmissionService, timeout(2000).times(0)).handleModelingSubmission(any(), any(), any());
verify(modelingSubmissionService, after(1000).never()).handleModelingSubmission(any(), any(), any());
// it is also not broadcast.
verify(websocketMessagingService, timeout(2000).times(0)).sendMessage(websocketTopic(participation), List.of());
verify(websocketMessagingService, after(1000).never()).sendMessage(websocketTopic(participation), List.of());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static org.awaitility.Awaitility.await;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

Expand Down Expand Up @@ -244,7 +245,7 @@ void testShouldNotFireProgressStalledEventWithEventDisabled() {
createSubmissionWithScore(studentParticipation, 40);
createSubmissionWithScore(studentParticipation, 40);
var result = createSubmissionWithScore(studentParticipation, 40);
verify(pyrisEventService, times(0)).trigger(new NewResultEvent(result));
verify(pyrisEventService, never()).trigger(new NewResultEvent(result));
}

@Test
Expand All @@ -259,7 +260,7 @@ void testShouldNotFireBuildFailedEventWhenEventSettingDisabled() {
// Create a failing submission for the student.
var result = createFailingSubmission(studentParticipation);
// very that the event is not fired
verify(pyrisEventService, times(0)).trigger(new NewResultEvent(result));
verify(pyrisEventService, never()).trigger(new NewResultEvent(result));
}

@Test
Expand All @@ -284,7 +285,7 @@ void testShouldShouldNotFireProgressStalledEventWithExistingSuccessfulSubmission

await().atMost(2, TimeUnit.SECONDS).untilAsserted(() -> {
verify(irisExerciseChatSessionService, times(2)).onNewResult(any(Result.class));
verify(pyrisPipelineService, times(0)).executeExerciseChatPipeline(any(), any(), any(), any(), any());
verify(pyrisPipelineService, never()).executeExerciseChatPipeline(any(), any(), any(), any(), any());
});
}

Expand All @@ -298,8 +299,8 @@ void testShouldNotFireProgressStalledEventWithLessThanThreeSubmissions() {

pyrisEventService.trigger(new NewResultEvent(result));

verify(irisExerciseChatSessionService, times(0)).onNewResult(any(Result.class));
verify(pyrisPipelineService, times(0)).executeExerciseChatPipeline(any(), any(), any(), any(), any());
verify(irisExerciseChatSessionService, never()).onNewResult(any(Result.class));
verify(pyrisPipelineService, never()).executeExerciseChatPipeline(any(), any(), any(), any(), any());
}

@Test
Expand All @@ -313,8 +314,8 @@ void testShouldNotFireProgressStalledEventWithIncreasingScores() {

pyrisEventService.trigger(new NewResultEvent(result));

verify(irisExerciseChatSessionService, times(0)).onNewResult(any(Result.class));
verify(pyrisPipelineService, times(0)).executeExerciseChatPipeline(any(), any(), any(), any(), any());
verify(irisExerciseChatSessionService, never()).onNewResult(any(Result.class));
verify(pyrisPipelineService, never()).executeExerciseChatPipeline(any(), any(), any(), any(), any());
}

@Test
Expand Down

0 comments on commit 9aedbd5

Please sign in to comment.