Development: Improve server test quality

laxerhd · Dec 7, 2024 · 0005603 · 0005603
1 parent 0f07355
commit 0005603
Show file tree

Hide file tree

Showing 12 changed files with 39 additions and 100 deletions.
diff --git a/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java
@@ -26,6 +26,7 @@
 import de.tum.cit.aet.artemis.communication.dto.ChannelIdAndNameDTO;
 import de.tum.cit.aet.artemis.communication.dto.FeedbackChannelRequestDTO;
 import de.tum.cit.aet.artemis.communication.dto.MetisCrudAction;
+import de.tum.cit.aet.artemis.communication.service.conversation.ConversationService;
 import de.tum.cit.aet.artemis.communication.util.ConversationUtilService;
 import de.tum.cit.aet.artemis.core.domain.Course;
 import de.tum.cit.aet.artemis.core.domain.CourseInformationSharingConfiguration;
@@ -47,6 +48,9 @@ class ChannelIntegrationTest extends AbstractConversationTest {
 
     private static final String TEST_PREFIX = "chtest";
 
+    @Autowired
+    private ConversationService conversationService;
+
     @Autowired
     private TutorialGroupTestRepository tutorialGroupRepository;
 
@@ -92,9 +96,8 @@ void setupTestScenario() throws Exception {
 
     @AfterEach
     void tearDown() {
-        tutorialGroupRepository.deleteAll();
-        conversationMessageRepository.deleteAll();
-        conversationRepository.deleteAllByCourseId(exampleCourseId);
+        var conversations = conversationRepository.findAllByCourseId(exampleCourseId);
+        conversations.forEach(conversation -> conversationService.deleteConversation(conversation));
     }
 
     @Override
@@ -944,8 +947,8 @@ void createFeedbackChannel_asStudent_shouldReturnForbidden() {
     @Test
     @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
     void createFeedbackChannel_asInstructor_shouldCreateChannel() {
-        Long courseId = 1L;
-        Long exerciseId = 1L;
+        long courseId = 1L;
+        long exerciseId = 1L;
         ChannelDTO channelDTO = new ChannelDTO();
         channelDTO.setName("feedback-channel");
         channelDTO.setDescription("Discussion channel for feedback");
@@ -958,8 +961,8 @@ void createFeedbackChannel_asInstructor_shouldCreateChannel() {
 
         ChannelDTO response = null;
         try {
-            response = request.postWithResponseBody(BASE_ENDPOINT.replace("{courseId}", courseId.toString()).replace("{exerciseId}", exerciseId.toString()), feedbackChannelRequest,
-                    ChannelDTO.class, HttpStatus.CREATED);
+            response = request.postWithResponseBody(BASE_ENDPOINT.replace("{courseId}", Long.toString(courseId)).replace("{exerciseId}", Long.toString(exerciseId)),
+                    feedbackChannelRequest, ChannelDTO.class, HttpStatus.CREATED);
         }
         catch (Exception e) {
             fail("Failed to create feedback channel", e);

diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/GitServiceTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/GitServiceTest.java
@@ -86,6 +86,8 @@ void testCheckoutRepositoryNotOnServer() throws GitAPIException, IOException {
         gitUtilService.deleteRepo(GitUtilService.REPOS.LOCAL);
         gitUtilService.reinitializeLocalRepository();
         try (var repo = gitService.getOrCheckoutRepository(repoUri, true)) {
+            assertThat(repo).isNotNull();
+            assertThat(repo.getRemoteRepositoryUri()).isEqualTo(repoUri);
             assertThat(gitUtilService.isLocalEqualToRemote()).isTrue();
         }
     }
@@ -125,6 +127,7 @@ void testCheckoutRepositoryAtCommit(boolean withUri) throws GitAPIException {
         if (withUri) {
             var uri = gitUtilService.getRepoUriByType(GitUtilService.REPOS.LOCAL);
             try (var repo = gitService.checkoutRepositoryAtCommit(uri, commitHash, true)) {
+                assertThat(repo).isNotNull();
             }
         }
         else {

diff --git a/...st/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java b/...st/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java
@@ -34,10 +34,8 @@
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.BiFunction;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import java.util.zip.ZipFile;
@@ -95,14 +93,12 @@
 import de.tum.cit.aet.artemis.plagiarism.domain.PlagiarismComparison;
 import de.tum.cit.aet.artemis.plagiarism.domain.PlagiarismStatus;
 import de.tum.cit.aet.artemis.plagiarism.domain.text.TextPlagiarismResult;
-import de.tum.cit.aet.artemis.plagiarism.domain.text.TextSubmissionElement;
 import de.tum.cit.aet.artemis.plagiarism.dto.PlagiarismResultDTO;
 import de.tum.cit.aet.artemis.programming.domain.AuxiliaryRepository;
 import de.tum.cit.aet.artemis.programming.domain.ProgrammingExercise;
 import de.tum.cit.aet.artemis.programming.domain.ProgrammingExerciseStudentParticipation;
 import de.tum.cit.aet.artemis.programming.domain.ProgrammingExerciseTestCase;
 import de.tum.cit.aet.artemis.programming.domain.ProgrammingLanguage;
-import de.tum.cit.aet.artemis.programming.domain.ProgrammingSubmission;
 import de.tum.cit.aet.artemis.programming.domain.ProjectType;
 import de.tum.cit.aet.artemis.programming.domain.RepositoryType;
 import de.tum.cit.aet.artemis.programming.domain.VcsRepositoryUri;
@@ -580,10 +576,6 @@ void testExportSubmissionsByStudentLogins() throws Exception {
         // TODO: unzip the files and add some checks
     }
 
-    void testExportSubmissionsByStudentLogins_failToCreateZip() throws Exception {
-        exportSubmissionsByStudentLogins(HttpStatus.BAD_REQUEST);
-    }
-
     private File exportSubmissionsByStudentLogins(HttpStatus expectedStatus) throws Exception {
         var repository1 = gitService.getExistingCheckedOutRepositoryByLocalPath(localRepoFile.toPath(), null);
         var repository2 = gitService.getExistingCheckedOutRepositoryByLocalPath(localRepoFile2.toPath(), null);
@@ -1797,12 +1789,12 @@ void testCheckPlagiarismJplagReport() throws Exception {
         }
     }
 
-    private void assertPlagiarismResult(ProgrammingExercise programmingExercise, PlagiarismResultDTO<TextPlagiarismResult> result, double expectedSimilarity) {
+    private void assertPlagiarismResult(ProgrammingExercise programmingExercise, PlagiarismResultDTO<?> result, double expectedSimilarity) {
         // verify plagiarism result
         assertThat(result.plagiarismResult().getComparisons()).hasSize(1);
         assertThat(result.plagiarismResult().getExercise().getId()).isEqualTo(programmingExercise.getId());
 
-        PlagiarismComparison<TextSubmissionElement> comparison = result.plagiarismResult().getComparisons().iterator().next();
+        PlagiarismComparison<?> comparison = result.plagiarismResult().getComparisons().iterator().next();
         assertThat(comparison.getSimilarity()).isEqualTo(expectedSimilarity, Offset.offset(0.0001));
         assertThat(comparison.getStatus()).isEqualTo(PlagiarismStatus.NONE);
         assertThat(comparison.getMatches()).hasSize(1);
@@ -2286,40 +2278,6 @@ void testReEvaluateAndUpdateProgrammingExercise_isNotSameGivenExerciseIdInReques
         request.put("/api/programming-exercises/" + programmingExercise.getId() + "/re-evaluate", programmingExerciseToBeConflicted, HttpStatus.CONFLICT);
     }
 
-    void test_redirectGetSolutionRepositoryFilesWithoutContent(BiFunction<ProgrammingExercise, Map<String, String>, LocalRepository> setupRepositoryMock) throws Exception {
-        setupRepositoryMock.apply(programmingExercise, Map.ofEntries(Map.entry("A.java", "abc"), Map.entry("B.java", "cde"), Map.entry("C.java", "efg")));
-
-        var savedExercise = programmingExerciseRepository.findByIdWithTemplateAndSolutionParticipationElseThrow(programmingExercise.getId());
-
-        request.getWithForwardedUrl("/api/programming-exercises/" + programmingExercise.getId() + "/file-names", HttpStatus.OK,
-                "/api/repository/" + savedExercise.getSolutionParticipation().getId() + "/file-names");
-    }
-
-    void test_redirectGetTemplateRepositoryFilesWithContent(BiFunction<ProgrammingExercise, Map<String, String>, LocalRepository> setupRepositoryMock) throws Exception {
-        setupRepositoryMock.apply(programmingExercise, Map.ofEntries(Map.entry("A.java", "abc"), Map.entry("B.java", "cde"), Map.entry("C.java", "efg")));
-
-        var savedExercise = programmingExerciseRepository.findByIdWithTemplateAndSolutionParticipationElseThrow(programmingExercise.getId());
-
-        request.getWithForwardedUrl("/api/programming-exercises/" + programmingExercise.getId() + "/template-files-content", HttpStatus.OK,
-                "/api/repository/" + savedExercise.getTemplateParticipation().getId() + "/files-content");
-    }
-
-    void testRedirectGetParticipationRepositoryFilesWithContentAtCommit(BiFunction<ProgrammingExercise, Map<String, String>, ProgrammingSubmission> setupRepositoryMock)
-            throws Exception {
-        var submission = setupRepositoryMock.apply(programmingExercise, Map.ofEntries(Map.entry("A.java", "abc"), Map.entry("B.java", "cde"), Map.entry("C.java", "efg")));
-
-        request.getWithForwardedUrl("/api/programming-exercise-participations/" + participation1.getId() + "/files-content/" + submission.getCommitHash(), HttpStatus.OK,
-                "/api/repository/" + participation1.getId() + "/files-content/" + submission.getCommitHash());
-    }
-
-    void testRedirectGetParticipationRepositoryFilesWithContentAtCommitForbidden(BiFunction<ProgrammingExercise, Map<String, String>, ProgrammingSubmission> setupRepositoryMock)
-            throws Exception {
-        var submission = setupRepositoryMock.apply(programmingExercise, Map.ofEntries(Map.entry("A.java", "abc"), Map.entry("B.java", "cde"), Map.entry("C.java", "efg")));
-        // without forwarding the redirectUrl is null
-        request.getWithForwardedUrl("/api/programming-exercise-participations/" + participation1.getId() + "/files-content/" + submission.getCommitHash(), HttpStatus.FORBIDDEN,
-                null);
-    }
-
     private long getMaxProgrammingExerciseId() {
         return programmingExerciseRepository.findAll(PageRequest.of(0, 1, Sort.by(Sort.Direction.DESC, "id"))).stream().mapToLong(ProgrammingExercise::getId).max().orElse(1L);
     }

diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/StaticCodeAnalysisParserUnitTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/StaticCodeAnalysisParserUnitTest.java
@@ -75,12 +75,12 @@ void testSpotbugsParser() throws IOException {
     }
 
     @Test
-    void testParseInvalidXML() throws IOException {
+    void testParseInvalidXML() {
         assertThatCode(() -> testParserWithFileNamed("invalid_xml.xml", "pmd.xml", "invalid_xml.txt")).isInstanceOf(RuntimeException.class);
     }
 
     @Test
-    void testInvalidName() throws IOException {
+    void testInvalidName() {
         assertThatCode(() -> testParserWithFile("invalid_name.xml", "invalid_name.txt")).isInstanceOf(UnsupportedToolException.class);
     }
 }
diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCInfoContributorTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCInfoContributorTest.java
@@ -1,27 +1,18 @@
 package de.tum.cit.aet.artemis.programming.icl;
 
-import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_ARTEMIS;
-import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_BUILDAGENT;
-import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_LOCALVC;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import org.junit.jupiter.api.Test;
 import org.springframework.boot.actuate.info.Info;
-import org.springframework.test.context.ActiveProfiles;
-import org.springframework.test.context.bean.override.mockito.MockitoSpyBean;
 
 import de.tum.cit.aet.artemis.programming.service.localvc.LocalVCInfoContributor;
 
-@ActiveProfiles({ PROFILE_ARTEMIS, PROFILE_LOCALVC, PROFILE_BUILDAGENT })
 class LocalVCInfoContributorTest {
 
-    @MockitoSpyBean
-    private LocalVCInfoContributor localVCInfoContributor;
-
     @Test
     void testContribute() {
         Info.Builder builder = new Info.Builder();
-        localVCInfoContributor = new LocalVCInfoContributor();
+        LocalVCInfoContributor localVCInfoContributor = new LocalVCInfoContributor();
         try {
             localVCInfoContributor.contribute(builder);
         }

diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java
@@ -1023,9 +1023,9 @@ private void testPriority(String login, int expectedPriority) throws Exception {
 
             await().until(() -> {
                 BuildJobQueueItem buildJobQueueItem = queuedJobs.peek();
-                log.info("Poll queue jobs: is null %s".formatted(buildJobQueueItem == null ? "true" : "false"));
+                log.info("Poll queue jobs: is null {}", buildJobQueueItem == null ? "true" : "false");
                 if (buildJobQueueItem == null) {
-                    queuedJobs.forEach(item -> log.info("Item in Queue: %s".formatted(item.toString())));
+                    queuedJobs.forEach(item -> log.info("Item in Queue: {}", item));
                 }
                 return buildJobQueueItem != null && buildJobQueueItem.participationId() == studentParticipation.getId();
             });

diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/MultipleHostKeyProviderTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/MultipleHostKeyProviderTest.java
@@ -1,17 +1,14 @@
 package de.tum.cit.aet.artemis.programming.icl;
 
-import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_LOCALVC;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.nio.file.Path;
 
 import org.junit.jupiter.api.Test;
-import org.springframework.context.annotation.Profile;
 
 import de.tum.cit.aet.artemis.programming.AbstractProgrammingIntegrationLocalCILocalVCTestBase;
 import de.tum.cit.aet.artemis.programming.service.localvc.ssh.MultipleHostKeyProvider;
 
-@Profile(PROFILE_LOCALVC)
 class MultipleHostKeyProviderTest extends AbstractProgrammingIntegrationLocalCILocalVCTestBase {
 
     private static final String TEST_PREFIX = "multiplehostkeyprovider";

diff --git a/...test/java/de/tum/cit/aet/artemis/programming/service/JenkinsJobPermissionServiceTest.java b/...test/java/de/tum/cit/aet/artemis/programming/service/JenkinsJobPermissionServiceTest.java
@@ -27,7 +27,7 @@ class JenkinsJobPermissionServiceTest extends AbstractProgrammingIntegrationJenk
     private static MockedStatic<JenkinsJobPermissionsUtils> mockedJenkinsJobPermissionsUtils;
 
     @BeforeEach
-    void initTestCase() throws Exception {
+    void initTestCase() {
         jenkinsRequestMockProvider.enableMockingOfRequests(jenkinsServer, jenkinsJobPermissionsService);
         gitlabRequestMockProvider.enableMockingOfRequests();
         mockedJenkinsJobPermissionsUtils = mockStatic(JenkinsJobPermissionsUtils.class);

diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsServiceTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsServiceTest.java
@@ -249,27 +249,28 @@ private void testFailToUpdatePlanRepositoryRestClientException(HttpStatus expect
     @Test
     @WithMockUser(roles = "INSTRUCTOR", username = TEST_PREFIX + "instructor1")
     void testUpdateBuildPlanRepoUrisForStudent() throws Exception {
-        MockedStatic<JenkinsBuildPlanUtils> mockedUtils = mockStatic(JenkinsBuildPlanUtils.class);
-        ArgumentCaptor<String> toBeReplacedCaptor = ArgumentCaptor.forClass(String.class);
-        ArgumentCaptor<String> replacementCaptor = ArgumentCaptor.forClass(String.class);
-        mockedUtils.when(() -> JenkinsBuildPlanUtils.replaceScriptParameters(any(), toBeReplacedCaptor.capture(), replacementCaptor.capture())).thenCallRealMethod();
+        try (MockedStatic<JenkinsBuildPlanUtils> mockedUtils = mockStatic(JenkinsBuildPlanUtils.class)) {
+            ArgumentCaptor<String> toBeReplacedCaptor = ArgumentCaptor.forClass(String.class);
+            ArgumentCaptor<String> replacementCaptor = ArgumentCaptor.forClass(String.class);
+            mockedUtils.when(() -> JenkinsBuildPlanUtils.replaceScriptParameters(any(), toBeReplacedCaptor.capture(), replacementCaptor.capture())).thenCallRealMethod();
 
-        var programmingExercise = continuousIntegrationTestService.programmingExercise;
-        programmingExerciseUtilService.addTemplateParticipationForProgrammingExercise(programmingExercise);
-        programmingExerciseUtilService.addSolutionParticipationForProgrammingExercise(programmingExercise);
-        programmingExerciseUtilService.addTestCasesToProgrammingExercise(programmingExercise);
-        var participation = participationUtilService.addStudentParticipationForProgrammingExercise(programmingExercise, TEST_PREFIX + "student1");
+            var programmingExercise = continuousIntegrationTestService.programmingExercise;
+            programmingExerciseUtilService.addTemplateParticipationForProgrammingExercise(programmingExercise);
+            programmingExerciseUtilService.addSolutionParticipationForProgrammingExercise(programmingExercise);
+            programmingExerciseUtilService.addTestCasesToProgrammingExercise(programmingExercise);
+            var participation = participationUtilService.addStudentParticipationForProgrammingExercise(programmingExercise, TEST_PREFIX + "student1");
 
-        String projectKey = programmingExercise.getProjectKey();
-        String planName = programmingExercise.getProjectKey();
+            String projectKey = programmingExercise.getProjectKey();
+            String planName = programmingExercise.getProjectKey();
 
-        String templateRepoUri = programmingExercise.getTemplateRepositoryUri();
-        jenkinsRequestMockProvider.mockUpdatePlanRepository(projectKey, planName, HttpStatus.OK);
+            String templateRepoUri = programmingExercise.getTemplateRepositoryUri();
+            jenkinsRequestMockProvider.mockUpdatePlanRepository(projectKey, planName, HttpStatus.OK);
 
-        continuousIntegrationService.updatePlanRepository(projectKey, planName, ASSIGNMENT_REPO_NAME, null, participation.getRepositoryUri(), templateRepoUri, "main");
+            continuousIntegrationService.updatePlanRepository(projectKey, planName, ASSIGNMENT_REPO_NAME, null, participation.getRepositoryUri(), templateRepoUri, "main");
 
-        assertThat(toBeReplacedCaptor.getValue()).contains("-exercise.git");
-        assertThat(replacementCaptor.getValue()).contains(TEST_PREFIX + "student1.git");
+            assertThat(toBeReplacedCaptor.getValue()).contains("-exercise.git");
+            assertThat(replacementCaptor.getValue()).contains(TEST_PREFIX + "student1.git");
+        }
     }
 
     @Test

diff --git a/...m/cit/aet/artemis/programming/service/ProgrammingExerciseFeedbackCreationServiceTest.java b/...m/cit/aet/artemis/programming/service/ProgrammingExerciseFeedbackCreationServiceTest.java
@@ -200,7 +200,7 @@ void createTimeoutFeedback() {
                 \tat de.tum.in.test.api.internal.TimeoutUtils.generateTimeoutFailure(TimeoutUtils.java:79)
                 \tat de.tum.in.test.api.internal.TimeoutUtils.executeWithTimeout(TimeoutUtils.java:72)
                 \tat de.tum.in.test.api.internal.TimeoutUtils.performTimeoutExecution(TimeoutUtils.java:42)
-                    """;
+                """;
         String actualFeedback = createFeedbackFromTestCase("test1", List.of(msgWithStackTrace), false);
         assertThat(actualFeedback)
                 .isEqualTo("The test case execution timed out. This indicates issues in your code such as endless loops, issues with recursion or really slow performance. "

diff --git a/.../tum/cit/aet/artemis/programming/test_repository/ProgrammingSubmissionTestRepository.java b/.../tum/cit/aet/artemis/programming/test_repository/ProgrammingSubmissionTestRepository.java
@@ -58,17 +58,6 @@ default Optional<ProgrammingSubmission> findFirstWithResultsByTypeNotAndTypeNotN
         return findProgrammingSubmissionById(id);
     }
 
-    Optional<ProgrammingSubmission> findFirstByParticipationIdOrderBySubmissionDateDesc(Long participationId);
-
-    default Optional<ProgrammingSubmission> findFirstWithResultsByParticipationIdOrderBySubmissionDateDesc(Long participationId) {
-        var programmingSubmissionOptional = findFirstByParticipationIdOrderBySubmissionDateDesc(participationId);
-        if (programmingSubmissionOptional.isEmpty()) {
-            return Optional.empty();
-        }
-        var id = programmingSubmissionOptional.get().getId();
-        return findProgrammingSubmissionById(id);
-    }
-
     @EntityGraph(type = LOAD, attributePaths = { "buildLogEntries" })
     Optional<ProgrammingSubmission> findWithEagerBuildLogEntriesById(Long submissionId);
 

diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/theia/TheiaInfoContributorTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/theia/TheiaInfoContributorTest.java
@@ -1,17 +1,14 @@
 package de.tum.cit.aet.artemis.programming.theia;
 
-import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_THEIA;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.actuate.info.Info;
-import org.springframework.context.annotation.Profile;
 
 import de.tum.cit.aet.artemis.core.config.Constants;
 import de.tum.cit.aet.artemis.core.config.TheiaInfoContributor;
 
-@Profile(PROFILE_THEIA)
 class TheiaInfoContributorTest {
 
     @Value("${theia.portal-url}")