Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Programming exercises: Fix display of long feedback details and submission count in exams #7348

Merged
merged 8 commits into from
Oct 11, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
import de.tum.in.www1.artemis.domain.participation.Participation;

@JsonInclude(JsonInclude.Include.NON_EMPTY)
public record ParticipationDTO(Long id, boolean testRun, String type) {
public record ParticipationDTO(Long id, boolean testRun, String type, Integer submissionCount) {

public static ParticipationDTO of(Participation participation) {
return new ParticipationDTO(participation.getId(), participation.isTestRun(), participation.getType());
return new ParticipationDTO(participation.getId(), participation.isTestRun(), participation.getType(), participation.getSubmissionCount());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ public record ResultDTO(Long id, ZonedDateTime completionDate, Boolean successfu
Integer codeIssueCount) {

@JsonInclude(JsonInclude.Include.NON_EMPTY)
public record FeedbackDTO(String text, String detailText, boolean hasLongFeedbackText, String reference, Double credits, Boolean positive, FeedbackType type,
public record FeedbackDTO(Long id, String text, String detailText, boolean hasLongFeedbackText, String reference, Double credits, Boolean positive, FeedbackType type,
Visibility visibility) {

public static FeedbackDTO of(Feedback feedback) {
return new FeedbackDTO(feedback.getText(), feedback.getDetailText(), feedback.getHasLongFeedbackText(), feedback.getReference(), feedback.getCredits(),
feedback.isPositive(), feedback.getType(), feedback.getVisibility());
return new FeedbackDTO(feedback.getId(), feedback.getText(), feedback.getDetailText(), feedback.getHasLongFeedbackText(), feedback.getReference(),
feedback.getCredits(), feedback.isPositive(), feedback.getType(), feedback.getVisibility());

}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,14 @@ void shouldGenerateTestwiseCoverageFileReport() throws Exception {
var resultNotification = TestwiseCoverageTestUtil.generateBambooBuildResultWithCoverage();
programmingExerciseResultTestService.shouldGenerateTestwiseCoverageFileReports(resultNotification);
}

@Test
@WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
void shouldRemoveTestCaseNamesFromWebsocketNotification() throws Exception {
var exercise = programmingExerciseResultTestService.getProgrammingExercise();
var planKey = (exercise.getProjectKey() + "-" + TEST_PREFIX + "student1").toUpperCase();
var notification = ProgrammingExerciseFactory.generateBambooBuildResult(Constants.ASSIGNMENT_REPO_NAME, planKey, null, null, List.of("test1", "test2"), List.of("test3"),
List.of());
programmingExerciseResultTestService.shouldCorrectlyNotifyStudentsAboutNewResults(notification, websocketMessagingService);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import de.tum.in.www1.artemis.AbstractSpringIntegrationJenkinsGitlabTest;
import de.tum.in.www1.artemis.config.Constants;
import de.tum.in.www1.artemis.domain.ProgrammingExercise;
import de.tum.in.www1.artemis.domain.enumeration.ProgrammingLanguage;
import de.tum.in.www1.artemis.service.connectors.ci.notification.dto.CommitDTO;
import de.tum.in.www1.artemis.util.TestConstants;
Expand All @@ -47,15 +48,23 @@ void tearDown() throws Exception {
gitlabRequestMockProvider.reset();
}

private String getRepoName(ProgrammingExercise exercise, String userLogin) {
return (exercise.getProjectKey() + "-" + userLogin).toUpperCase();
}

private String getFolderName(ProgrammingExercise exercise, String repoName) {
// The full name is specified as <FOLDER NAME> » <JOB NAME> <Build Number>
return exercise.getProjectKey() + " » " + repoName + " #3";
}

@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void shouldUpdateFeedbackInSemiAutomaticResult() throws Exception {
var loginName = TEST_PREFIX + "student1";
var exercise = programmingExerciseResultTestService.getProgrammingExercise();
var repoName = (exercise.getProjectKey() + "-" + loginName).toUpperCase();
// The full name is specified as <FOLDER NAME> » <JOB NAME> <Build Number>
var notification = ProgrammingExerciseFactory.generateTestResultDTO(exercise.getProjectKey() + " » " + repoName + " #3", repoName, null, exercise.getProgrammingLanguage(),
false, List.of("test1"), List.of(), new ArrayList<>(), new ArrayList<>(), null);
var repoName = getRepoName(exercise, loginName);
var notification = ProgrammingExerciseFactory.generateTestResultDTO(getFolderName(exercise, repoName), repoName, null, exercise.getProgrammingLanguage(), false,
List.of("test1"), List.of(), new ArrayList<>(), new ArrayList<>(), null);
programmingExerciseResultTestService.shouldUpdateFeedbackInSemiAutomaticResult(notification, loginName);
}

Expand Down Expand Up @@ -150,4 +159,16 @@ void shouldCreateResultOnCustomDefaultBranch() {
List.of(), List.of(commit), null);
programmingExerciseResultTestService.shouldCreateResultOnCustomDefaultBranch(customDefaultBranch, notification);
}

@Test
@WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
void shouldRemoveTestCaseNamesFromWebsocketNotification() throws Exception {
var exercise = programmingExerciseResultTestService.getProgrammingExercise();
var commit = new CommitDTO("abc123", "slug", DEFAULT_BRANCH);
String repoName = getRepoName(exercise, TEST_PREFIX + "student1");
String folderName = getFolderName(exercise, repoName);
var notification = ProgrammingExerciseFactory.generateTestResultDTO(folderName, repoName, null, ProgrammingLanguage.JAVA, false, List.of("test1", "test2"),
List.of("test3"), List.of(), List.of(commit), null);
programmingExerciseResultTestService.shouldCorrectlyNotifyStudentsAboutNewResults(notification, websocketMessagingService);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package de.tum.in.www1.artemis.exercise.programmingexercise;

import static de.tum.in.www1.artemis.config.Constants.NEW_RESULT_TOPIC;
import static java.util.Comparator.*;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -30,12 +31,14 @@
import de.tum.in.www1.artemis.participation.ParticipationUtilService;
import de.tum.in.www1.artemis.repository.*;
import de.tum.in.www1.artemis.service.StaticCodeAnalysisService;
import de.tum.in.www1.artemis.service.WebsocketMessagingService;
import de.tum.in.www1.artemis.service.connectors.GitService;
import de.tum.in.www1.artemis.service.connectors.bamboo.dto.BambooBuildResultNotificationDTO;
import de.tum.in.www1.artemis.service.dto.AbstractBuildResultNotificationDTO;
import de.tum.in.www1.artemis.service.programming.ProgrammingExerciseGradingService;
import de.tum.in.www1.artemis.user.UserUtilService;
import de.tum.in.www1.artemis.util.*;
import de.tum.in.www1.artemis.web.rest.dto.ResultDTO;

/**
* Note: this class should be independent of the actual VCS and CIS and contains common test logic for both scenarios:
Expand Down Expand Up @@ -371,6 +374,30 @@ public void shouldCreateResultOnCustomDefaultBranch(String defaultBranch, Object
assertThat(result).isNotNull();
}

// Test
public void shouldCorrectlyNotifyStudentsAboutNewResults(AbstractBuildResultNotificationDTO resultNotification, WebsocketMessagingService websocketMessagingService)
throws Exception {
programmingExerciseUtilService.addTestCasesToProgrammingExercise(programmingExercise);

var programmingSubmission = programmingExerciseUtilService.createProgrammingSubmission(programmingExerciseStudentParticipation, false);
programmingExerciseStudentParticipation.addSubmission(programmingSubmission);
programmingExerciseStudentParticipation = participationRepository.save(programmingExerciseStudentParticipation);

postResult(resultNotification);

// ensure that hidden feedback got filtered out (test2 is not active, test3 is hidden -> only 1 feedback visible)
verify(websocketMessagingService, timeout(2000)).sendMessageToUser(eq(userPrefix + "student1"), eq(NEW_RESULT_TOPIC), argThat(arg -> {
if (!(arg instanceof ResultDTO resultDTO)) {
return false;
}
if (resultDTO.feedbacks().size() != 1) {
return false;
}
var feedback = resultDTO.feedbacks().get(0);
return feedback.id() != null && feedback.positive();
}));
}

private int getNumberOfBuildLogs(Object resultNotification) {
if (resultNotification instanceof BambooBuildResultNotificationDTO) {
return ((BambooBuildResultNotificationDTO) resultNotification).getBuild().jobs().iterator().next().logs().size();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,37 @@ void testNotifyPush_isSetupCommit() throws Exception {

}

@Test
@WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
void testNotifyPush_studentCommitUpdatesSubmissionCount() throws Exception {
var participation = participationUtilService.addStudentParticipationForProgrammingExercise(exercise, TEST_PREFIX + "student1");

doNothing().when(continuousIntegrationTriggerService).triggerBuild(any());
Commit mockCommit = mock(Commit.class);
doReturn(mockCommit).when(versionControlService).getLastCommitDetails(any());
doReturn("default-branch").when(versionControlService).getDefaultBranchOfRepository(any());

doReturn("hash1").when(mockCommit).getCommitHash();
doReturn("default-branch").when(mockCommit).getBranch();
doReturn("Student 1").when(mockCommit).getAuthorName();
doReturn("[email protected]").when(mockCommit).getAuthorEmail();
doReturn("my nice little solution").when(mockCommit).getMessage();

String url = "/api/public/programming-submissions/" + participation.getId();
// no request body needed since the commit information are mocked above
request.postWithoutLocation(url, "test", HttpStatus.OK, null);

verify(websocketMessagingService, timeout(2000)).sendMessageToUser(eq(TEST_PREFIX + "student1"), eq(NEW_SUBMISSION_TOPIC),
argThat(arg -> arg instanceof SubmissionDTO submissionDTO && submissionDTO.participation().submissionCount() == 1));

// second push
doReturn("hash2").when(mockCommit).getCommitHash();
request.postWithoutLocation(url, "test", HttpStatus.OK, null);

verify(websocketMessagingService, timeout(2000)).sendMessageToUser(eq(TEST_PREFIX + "student1"), eq(NEW_SUBMISSION_TOPIC),
argThat(arg -> arg instanceof SubmissionDTO submissionDTO && submissionDTO.participation().submissionCount() == 2));
}

@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void getAllProgrammingSubmissionsAsInstructorAllSubmissionsReturned() throws Exception {
Expand Down