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

Development: Update Spring Boot to 3.4.0 #9852

Merged
merged 10 commits into from
Dec 2, 2024
Merged

Conversation

krusche
Copy link
Member

@krusche krusche commented Nov 24, 2024

Checklist

General

Server

Motivation and Context

We want to use the latest versions to keep the code modern, stable and secure

Description

Update Spring Boot and related dependencies to the latest version

Steps for Testing

Go through the whole app and test all kinds of functionality. Everything should be exactly the same as before

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.







Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Summary by CodeRabbit

  • New Features

    • Enhanced mocking capabilities for Jenkins job permissions in various test classes.
    • New methods added to the ProgrammingExerciseTestService for importing programming exercises and managing repository access.
  • Bug Fixes

    • Improved error handling in tests related to programming submissions and exercise creation.
  • Refactor

    • Updated annotations from @SpyBean to @MockitoSpyBean across multiple classes to align with updated mocking practices.
  • Documentation

    • Clarified guidelines and comments regarding the use of Mockito in tests.

@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) documentation buildagent Pull requests that affect the corresponding module communication Pull requests that affect the corresponding module core Pull requests that affect the corresponding module exam Pull requests that affect the corresponding module exercise Pull requests that affect the corresponding module programming Pull requests that affect the corresponding module labels Nov 24, 2024
@krusche krusche changed the title Development: Update Spring to 3.4.0 Development: Update Spring Boot to 3.4.0 Nov 24, 2024
@krusche krusche added this to the 7.7.3 milestone Nov 24, 2024
@krusche krusche modified the milestones: 7.7.3, 7.7.4 Nov 28, 2024
@PaRangger PaRangger removed the communication Pull requests that affect the corresponding module label Nov 29, 2024
@github-actions github-actions bot added the communication Pull requests that affect the corresponding module label Nov 29, 2024
@krusche krusche marked this pull request as ready for review November 29, 2024 20:06
@krusche krusche requested a review from a team as a code owner November 29, 2024 20:06
@krusche krusche removed ready for review buildagent Pull requests that affect the corresponding module labels Nov 29, 2024
@krusche krusche added ready for review and removed exam Pull requests that affect the corresponding module exercise Pull requests that affect the corresponding module programming Pull requests that affect the corresponding module labels Nov 29, 2024
Copy link

coderabbitai bot commented Nov 29, 2024

Walkthrough

The pull request includes extensive changes across multiple files, primarily focusing on updates to dependency management in the build.gradle file, modifications to test configurations, and enhancements in test case setups. Key changes involve updating various library versions, altering mocking strategies from @SpyBean to @MockitoSpyBean, and refining test methods to include new parameters for Jenkins job permissions. Additionally, guidelines for server tests have been clarified, and several new methods have been introduced to improve functionality and error handling in the ProgrammingExerciseTestService.

Changes

File Path Change Summary
build.gradle Updated multiple dependencies to newer versions, changed repository URLs, and removed test container dependencies.
docs/dev/guidelines/server-tests.rst Renamed annotations from @MockBean to @MockitoBean and @SpyBean to @MockitoSpyBean. Added guidance on static mocks and parallel test execution.
gradle.properties Updated dependency versions for Spring Boot, Spring Framework, Spring Security, and others.
src/main/java/de/tum/cit/aet/artemis/buildagent/BuildAgentConfiguration.java Replaced ApacheDockerHttpClient with ZerodepDockerHttpClient in the dockerClient() method.
src/main/java/de/tum/cit/aet/artemis/core/config/EurekaClientConfiguration.java Renamed class and updated methods to use RestClient instead of RestTemplate.
src/main/java/de/tum/cit/aet/artemis/core/config/LiquibaseConfiguration.java Enhanced null handling for contexts and label filters in the liquibase method.
src/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.java Removed @Autowired annotations from several RestTemplate bean methods.
src/test/java/de/tum/cit/aet/artemis/communication/PostingServiceUnitTest.java Simplified method calls and expanded test cases for user mentions.
src/test/java/de/tum/cit/aet/artemis/core/authentication/InternalAuthenticationIntegrationTest.java Removed programmingExercise variable and updated setup method.
src/test/java/de/tum/cit/aet/artemis/core/authentication/UserJenkinsGitlabIntegrationTest.java Updated mocking setup to include jenkinsJobPermissionsService.
src/test/java/de/tum/cit/aet/artemis/core/connector/GitlabRequestMockProvider.java Marked class as deprecated and added new @SpyBean annotations.
src/test/java/de/tum/cit/aet/artemis/core/connector/JenkinsRequestMockProvider.java Updated method signature to include jenkinsJobPermissionsService.
src/test/java/de/tum/cit/aet/artemis/exam/ExamIntegrationTest.java Enhanced Jenkins request mocking setup.
src/test/java/de/tum/cit/aet/artemis/exam/ExamParticipationIntegrationTest.java Updated method calls to include jenkinsJobPermissionsService.
src/test/java/de/tum/cit/aet/artemis/exam/ExamUserIntegrationTest.java Updated Jenkins request mocking setup.
src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java Updated mocking setup for Jenkins job permissions.
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTest.java Updated methods to include jenkinsJobPermissionsService in mock requests.
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisIntegrationTest.java Replaced @SpyBean with @MockitoSpyBean for multiple service fields.
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationJenkinsGitlabTest.java Updated annotations to @MockitoSpyBean for various fields.

Possibly related PRs

Suggested labels

dependencies, refactoring, bugfix

Suggested reviewers

  • JohannesStoehr
  • az108
  • SimonEntholzer
  • BBesrour

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 pmd (7.7.0)
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisIntegrationTest.java

The following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration.


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between ca9cce8 and 5cf39e8.

📒 Files selected for processing (1)
  • src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisIntegrationTest.java (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisIntegrationTest.java

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Outside diff range and nitpick comments (32)
src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCInfoContributorTest.java (3)

Line range hint 26-28: Improve error handling in test

Empty catch blocks are dangerous, especially in tests, as they can hide important failures. The NPE might indicate a deeper issue that should be addressed.

Consider one of these approaches:

-       try {
-           localVCInfoContributor.contribute(builder);
-       }
-       catch (NullPointerException e) {
-       }
+       // Option 1: If NPE is expected, assert it
+       assertThatThrownBy(() -> localVCInfoContributor.contribute(builder))
+           .isInstanceOf(NullPointerException.class);
+
+       // Option 2: If NPE should not occur, remove try-catch
+       localVCInfoContributor.contribute(builder);

Line range hint 31-32: Enhance assertion specificity

The assertion could be more specific about what it's testing and provide a better failure message.

-       assertThat((Boolean) info.getDetails().get("useVersionControlAccessToken")).isFalse();
+       assertThat(info.getDetails())
+           .as("Version control access token setting should be disabled")
+           .containsEntry("useVersionControlAccessToken", false);

Line range hint 21-33: Utilize more JUnit 5 features

The test could benefit from JUnit 5's advanced features to improve readability and maintainability.

Consider this enhanced version:

+    @DisplayName("Local VC Info Contributor should not use version control access token")
     @Test
-    void testContribute() {
-        Info.Builder builder = new Info.Builder();
-        localVCInfoContributor = new LocalVCInfoContributor();
-        try {
-            localVCInfoContributor.contribute(builder);
-        }
-        catch (NullPointerException e) {
-        }
-
-        Info info = builder.build();
-        assertThat((Boolean) info.getDetails().get("useVersionControlAccessToken")).isFalse();
+    void shouldNotUseVersionControlAccessToken_whenContributing() {
+        // given
+        var builder = new Info.Builder();
+
+        // when
+        localVCInfoContributor.contribute(builder);
+        var info = builder.build();
+
+        // then
+        assertThat(info.getDetails())
+            .as("Version control access token setting should be disabled")
+            .containsEntry("useVersionControlAccessToken", false);
     }
src/test/java/de/tum/cit/aet/artemis/programming/ConsistencyCheckGitlabJenkinsIntegrationTest.java (1)

13-13: Add a comment explaining the jenkinsJobPermissionsService parameter

Consider adding a brief comment explaining why the jenkinsJobPermissionsService parameter is needed in the mock setup.

+    // Enable request mocking with Jenkins job permissions to properly simulate permission-based behavior in tests
     jenkinsRequestMockProvider.enableMockingOfRequests(jenkinsServer, jenkinsJobPermissionsService);
gradle.properties (1)

Line range hint 8-8: Remove duplicate spring_boot_version declaration

There are two declarations of spring_boot_version in the file. This can lead to unpredictable behavior depending on which one is read last.

Remove one of the duplicate entries:

spring_boot_version=3.3.6
-spring_boot_version=3.4.0

Also applies to: 12-12

src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsAuthorizationInterceptorTest.java (1)

Line range hint 1-150: Consider adding DB query tracking

While the test structure follows the guidelines well, consider adding explicit DB query count tracking as per the db_query_count_tests: track_performance guideline.

Consider adding the following at the class level:

@TestExecutionListeners(value = { QueryCountListener.class }, mergeMode = MergeMode.MERGE_WITH_DEFAULTS)
src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsJobPermissionServiceTest.java (1)

Line range hint 42-120: Consider enhancing error message assertions

While the tests effectively verify exception throwing, consider making the error message assertions more specific by checking the complete error message rather than just the prefix.

Example improvement for one test:

-assertThatIOException().isThrownBy(() -> jenkinsJobPermissionsService.addTeachingAssistantPermissionsToUserForFolder(taLogin, folderName))
-        .withMessageStartingWith("Cannot add ta permissions to user for folder");
+assertThatIOException().isThrownBy(() -> jenkinsJobPermissionsService.addTeachingAssistantPermissionsToUserForFolder(taLogin, folderName))
+        .withMessage("Cannot add ta permissions to user for folder %s: %s", folderName, "DOM Exception occurred");
src/main/java/de/tum/cit/aet/artemis/core/config/LiquibaseConfiguration.java (1)

85-85: LGTM! Consider extracting the null-safe first element pattern

The null handling is correct and consistent with the contexts configuration. However, since this pattern is used twice, consider extracting it into a utility method for better maintainability.

+ private <T> T getFirstOrNull(List<T> list) {
+     return list != null ? list.getFirst() : null;
+ }

- liquibase.setContexts(liquibaseProperties.getContexts() != null ? liquibaseProperties.getContexts().getFirst() : null);
+ liquibase.setContexts(getFirstOrNull(liquibaseProperties.getContexts()));

- liquibase.setLabelFilter(liquibaseProperties.getLabelFilter() != null ? liquibaseProperties.getLabelFilter().getFirst() : null);
+ liquibase.setLabelFilter(getFirstOrNull(liquibaseProperties.getLabelFilter()));
src/main/java/de/tum/cit/aet/artemis/buildagent/BuildAgentConfiguration.java (1)

152-153: Consider implementing the TODO suggestion for startup optimization

The TODO comment suggests moving bean creation to post-startup. This can be implemented using @EventListener(ApplicationReadyEvent.class) with a custom initialization method:

-@Bean
-public DockerClient dockerClient() {
+@Bean(destroyMethod = "close")
+public DockerClient dockerClient() {
+    return null;  // Initial placeholder
+}
+
+@EventListener(ApplicationReadyEvent.class)
+public void initializeDockerClient() {
     log.debug("Create bean dockerClient");
     DockerClientConfig config = DefaultDockerClientConfig.createDefaultConfigBuilder()
         .withDockerHost(dockerConnectionUri)
         .build();
     DockerHttpClient httpClient = new ZerodepDockerHttpClient.Builder()
         .dockerHost(config.getDockerHost())
         .sslConfig(config.getSSLConfig())
         .build();
-    DockerClient dockerClient = DockerClientImpl.getInstance(config, httpClient);
+    DockerClient client = DockerClientImpl.getInstance(config, httpClient);
     log.debug("Docker client created with connection URI: {}", dockerConnectionUri);
-    return dockerClient;
+    // Use ApplicationContext to replace the placeholder bean
+    ((ConfigurableApplicationContext) applicationContext)
+        .getBeanFactory()
+        .registerSingleton("dockerClient", client);
 }
src/test/java/de/tum/cit/aet/artemis/core/authentication/InternalAuthenticationIntegrationTest.java (1)

45-46: Track technical debt: Migration to LocalVC

The TODO comment indicates a planned migration from GitLab to LocalVC. This should be tracked as technical debt to ensure it's not forgotten during the Spring Boot upgrade.

Would you like me to create a GitHub issue to track this technical debt item?

src/test/java/de/tum/cit/aet/artemis/core/user/AccountResourceWithGitLabIntegrationTest.java (1)

27-27: Track the LocalVC migration task

The TODO comment indicates a need to migrate from GitLab to LocalVC for testing. This would improve test isolation and performance.

Would you like me to create a GitHub issue to track this technical debt? I can help draft the issue with:

  • Description of the migration need
  • Benefits of using LocalVC
  • Suggested implementation approach
  • Testing considerations
docs/dev/guidelines/server-tests.rst (3)

154-158: Enhance section title and content clarity

While the content is technically accurate, consider these improvements for better clarity:

  1. Make the section title more specific to include both annotations: "Avoid using @MockitoBean and @MockitoSpyBean"
  2. Add a brief mention that these are Spring Boot 3.x annotations to provide context for the version update
-6. Avoid using @MockitoBean
+6. Avoid using @MockitoBean and @MockitoSpyBean
 ===========================
 
-Do not use the ``@MockitoSpyBean`` or ``@MockitoBean`` annotation unless absolutely necessary or possibly in an abstract Superclass. `Here <https://www.baeldung.com/spring-tests>`__ you can see why in more detail.
+Do not use the Spring Boot 3.x annotations ``@MockitoSpyBean`` or ``@MockitoBean`` unless absolutely necessary or possibly in an abstract Superclass. `Here <https://www.baeldung.com/spring-tests>`__ you can see why in more detail.

Line range hint 161-177: Enhance the static mocks example and documentation

The example effectively demonstrates the transition from @MockitoSpyBean to static mocks, but consider these improvements:

  1. Add a warning box about the importance of closing static mocks
  2. Include a note about potential threading issues with static mocks in parallel tests
  3. Add a brief explanation of why static mocks are preferred over service-level mocks

Add this warning box after the example:

.. warning::
   Always close static mocks in a finally block or try-with-resources to prevent memory leaks and test pollution. Static mocks can affect other tests running in parallel if not properly closed.

Also, add this explanation before the example:

Static mocks are preferred because they:
* Target the exact point of failure
* Reduce test setup complexity
* Avoid application context pollution
* Provide better test isolation

Line range hint 154-177: Add version-specific information to support Spring Boot update

Since this PR updates Spring Boot to 3.4.0, consider adding version-specific information:

  1. Document any breaking changes in testing annotations between Spring Boot versions
  2. Mention if certain mocking approaches are preferred in Spring Boot 3.x

Add a version compatibility note:

.. note::
   Spring Boot 3.x introduces changes to testing annotations and mocking approaches. The guidelines in this document reflect best practices for Spring Boot 3.4.0 and later versions.
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTest.java (1)

Line range hint 174-179: LGTM! Consider documenting test scenarios

The mock configuration is well-structured and appropriately handles different assessment types. However, consider adding a brief comment explaining the test scenarios for automatic assessment.

Add a descriptive comment before the conditional block:

        programmingExercise.setAssessmentType(assessmentType);

+       // For automatic assessment, configure Jenkins and GitLab mocks to validate repository and build plan setup
        if (assessmentType == AssessmentType.AUTOMATIC) {
            jenkinsRequestMockProvider.enableMockingOfRequests(jenkinsServer, jenkinsJobPermissionsService);
            gitlabRequestMockProvider.enableMockingOfRequests();
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTemplateIntegrationTest.java (1)

207-207: LGTM! Consider extracting mock setup/teardown.

The change maintains symmetry with the setup method. Consider extracting these mock setup/teardown calls into a helper method to ensure consistency and reduce duplication.

+ private void configureMockProviders() {
+     jenkinsRequestMockProvider.enableMockingOfRequests(jenkinsServer, jenkinsJobPermissionsService);
+     gitlabRequestMockProvider.enableMockingOfRequests();
+ }

  @BeforeEach
  void setup() throws Exception {
      // ... other setup code ...
-     jenkinsRequestMockProvider.enableMockingOfRequests(jenkinsServer, jenkinsJobPermissionsService);
-     gitlabRequestMockProvider.enableMockingOfRequests();
+     configureMockProviders();
      // ... rest of setup
  }

  @AfterEach
  void tearDown() throws Exception {
-     jenkinsRequestMockProvider.enableMockingOfRequests(jenkinsServer, jenkinsJobPermissionsService);
-     gitlabRequestMockProvider.enableMockingOfRequests();
+     configureMockProviders();
      // ... rest of teardown
  }
src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsServiceTest.java (1)

Line range hint 1-379: Consider enhancing test maintainability with shared test utilities.

While the test class is well-structured, consider the following improvements to enhance maintainability:

  1. Extract common setup code from test methods like testFailToUpdatePlanRepository* into shared utility methods
  2. Consider using more descriptive names for test methods (e.g., testHealthRunningtestJenkinsHealthCheckWhenServerIsRunning)

Example refactoring for shared setup:

+ private ProgrammingExercise setupExerciseWithParticipations() {
+     var programmingExercise = continuousIntegrationTestService.programmingExercise;
+     programmingExerciseUtilService.addTemplateParticipationForProgrammingExercise(programmingExercise);
+     programmingExerciseUtilService.addSolutionParticipationForProgrammingExercise(programmingExercise);
+     programmingExerciseUtilService.addTestCasesToProgrammingExercise(programmingExercise);
+     return programmingExercise;
+ }

  @Test
  @WithMockUser(roles = "INSTRUCTOR", username = TEST_PREFIX + "instructor1")
  void testFailToUpdatePlanRepositoryBadRequest() throws Exception {
-     var programmingExercise = continuousIntegrationTestService.programmingExercise;
-     programmingExerciseUtilService.addTemplateParticipationForProgrammingExercise(programmingExercise);
-     programmingExerciseUtilService.addSolutionParticipationForProgrammingExercise(programmingExercise);
-     programmingExerciseUtilService.addTestCasesToProgrammingExercise(programmingExercise);
+     var programmingExercise = setupExerciseWithParticipations();
      // ... rest of the test
  }
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java (1)

Line range hint 174-177: Update resetSpyBeans() to include all mocked services.

The resetSpyBeans() method should be updated to reset all mocked services. Currently, it's missing reset calls for ldapUserService, ldapTemplate, examLiveEventsService, and groupNotificationScheduleService.

Apply this diff:

 @AfterEach
 @Override
 protected void resetSpyBeans() {
-    Mockito.reset(versionControlService, continuousIntegrationService, resourceLoaderService, programmingMessagingService);
+    Mockito.reset(
+        versionControlService,
+        continuousIntegrationService,
+        resourceLoaderService,
+        programmingMessagingService,
+        ldapUserService,
+        ldapTemplate,
+        examLiveEventsService,
+        groupNotificationScheduleService
+    );
     super.resetSpyBeans();
 }
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationGitlabCIGitlabSamlTest.java (2)

50-50: Consider creating a tracking issue for the TODO.

The TODO comment indicates a future refactoring to use LocalVC instead of GitLab. While this is out of scope for the Spring Boot upgrade, it would be beneficial to track this technical debt.

Would you like me to create a GitHub issue to track this refactoring task?


Inconsistent Mockito annotations found in GitlabRequestMockProvider

The migration to @MockitoSpyBean is incomplete. Found two instances of old @SpyBean annotations in GitlabRequestMockProvider.java with comments indicating a technical blocker:

  • The comments suggest @MockitoSpyBean cannot be used due to incompatibility with @InjectMocks
  • All other test classes have successfully migrated to the new annotations
🔗 Analysis chain

Line range hint 59-82: Mockito annotation updates look good, verify consistency across test classes.

The migration from Spring's @SpyBean/@MockBean to @MockitoSpyBean/@MockitoBean is correct for Spring Boot 3.4.0. The comments provide clear guidance on proper usage of spy beans.

Let's verify the consistency of these annotation changes across other test classes:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent usage of new Mockito annotations across test classes
# Expected: All test classes should use the new annotations (@MockitoSpyBean, @MockitoBean)

echo "Checking for old @SpyBean annotations..."
rg "@SpyBean" "src/test/java"

echo "Checking for old @MockBean annotations..."
rg "@MockBean(?!.*MockitoBean)" "src/test/java"

echo "Checking current usage of new annotations..."
rg "@Mockito(Spy|)Bean" "src/test/java"

Length of output: 7106

src/test/java/de/tum/cit/aet/artemis/exam/ExamUserIntegrationTest.java (2)

Line range hint 1-24: Enhance test class structure to fully comply with guidelines.

Consider the following improvements to better align with our testing guidelines:

  1. Add @DisplayName annotations for better test descriptions
  2. Add @Tag annotations to categorize tests
  3. Consider tracking DB query count as per guidelines

Example enhancement:

+ import org.junit.jupiter.api.DisplayName;
+ import org.junit.jupiter.api.Tag;

+ @DisplayName("Exam User Integration Tests")
+ @Tag("integration")
class ExamUserIntegrationTest extends AbstractSpringIntegrationJenkinsGitlabTest {

Also applies to: 32-38


Line range hint 234-306: Consider breaking down complex test methods.

The testVerifyExamUserAttendance method is handling multiple scenarios and is quite long. Consider breaking it down into smaller, more focused test methods:

  1. Test for attendance verification
  2. Test for signature verification
  3. Test for attendance override

Example refactoring:

+ @Test
+ @DisplayName("Should verify exam user attendance")
void testVerifyExamUserAttendance() {
    // Test attendance verification only
}

+ @Test
+ @DisplayName("Should verify exam user signatures")
void testVerifyExamUserSignatures() {
    // Test signature verification only
}

+ @Test
+ @DisplayName("Should allow attendance override")
void testAttendanceOverride() {
    // Test override functionality only
}
src/test/java/de/tum/cit/aet/artemis/core/connector/JenkinsRequestMockProvider.java (2)

71-75: Consider using JavaDoc for field documentation.

While the comments explain the initialization timing, consider converting them to proper JavaDoc for better IDE integration and documentation standards.

-    // will be assigned in enableMockingOfRequests(), can be used like a MockitoSpyBean
+    /**
+     * Jenkins server instance that will be assigned in {@link #enableMockingOfRequests}.
+     * Can be used like a MockitoSpyBean in tests.
+     */
     private JenkinsServer jenkinsServer;

-    // will be assigned in enableMockingOfRequests(), can be used like a MockitoSpyBean
+    /**
+     * Jenkins job permissions service that will be assigned in {@link #enableMockingOfRequests}.
+     * Can be used like a MockitoSpyBean in tests.
+     */
     private JenkinsJobPermissionsService jenkinsJobPermissionsService;

97-102: Add JavaDoc and parameter validation.

The method would benefit from proper documentation and null-checks for its parameters.

+    /**
+     * Enables mocking of Jenkins requests by setting up mock servers and initializing spy instances.
+     *
+     * @param jenkinsServer Jenkins server instance to be used as a spy
+     * @param jenkinsJobPermissionsService Jenkins job permissions service to be used as a spy
+     * @throws IllegalArgumentException if any parameter is null
+     */
     public void enableMockingOfRequests(JenkinsServer jenkinsServer, JenkinsJobPermissionsService jenkinsJobPermissionsService) {
+        if (jenkinsServer == null || jenkinsJobPermissionsService == null) {
+            throw new IllegalArgumentException("Parameters must not be null");
+        }
         mockServer = MockRestServiceServer.bindTo(restTemplate).ignoreExpectOrder(true).bufferContent().build();
         shortTimeoutMockServer = MockRestServiceServer.bindTo(shortTimeoutRestTemplate).ignoreExpectOrder(true).bufferContent().build();
         this.jenkinsServer = jenkinsServer;
         this.jenkinsJobPermissionsService = jenkinsJobPermissionsService;
         closeable = MockitoAnnotations.openMocks(this);
     }
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java (2)

Line range hint 291-361: Consider splitting test method for better maintainability.

The shouldSaveBuildLogsOnStudentParticipationWithoutResult test method is quite long and handles multiple scenarios. Consider extracting the setup and assertion logic into helper methods for better readability and maintenance.

-    @ParameterizedTest(name = "{displayName} [{index}] {argumentsWithNames}")
-    @MethodSource("shouldSaveBuildLogsOnStudentParticipationWithoutResultArguments")
-    @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
-    void shouldSaveBuildLogsOnStudentParticipationWithoutResult(ProgrammingLanguage programmingLanguage, boolean enableStaticCodeAnalysis, boolean buildFailed) throws Exception {
+    @ParameterizedTest(name = "{displayName} [{index}] {argumentsWithNames}")
+    @MethodSource("shouldSaveBuildLogsOnStudentParticipationWithoutResultArguments")
+    @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
+    void shouldSaveBuildLogsOnStudentParticipationWithoutResult(ProgrammingLanguage programmingLanguage, boolean enableStaticCodeAnalysis, boolean buildFailed) throws Exception {
+        var participation = setupTestEnvironment(programmingLanguage, enableStaticCodeAnalysis);
+        var submission = createSubmissionWithBuildStatus(participation, buildFailed);
+        
+        processNewResultAndVerify(participation, submission);
+    }
+
+    private StudentParticipation setupTestEnvironment(ProgrammingLanguage programmingLanguage, boolean enableStaticCodeAnalysis) {
         String userLogin = TEST_PREFIX + "student1";
         var course = programmingExerciseUtilService.addCourseWithOneProgrammingExercise(enableStaticCodeAnalysis, false, programmingLanguage);
         exercise = exerciseUtilService.getFirstExerciseWithType(course, ProgrammingExercise.class);
         exercise = programmingExerciseRepository.findWithEagerStudentParticipationsById(exercise.getId()).orElseThrow();
+        return participationUtilService.addStudentParticipationForProgrammingExercise(exercise, userLogin);
+    }

Line range hint 446-461: Enhance assertion specificity in helper methods.

The assertBuildError helper method could benefit from more specific assertions using descriptive error messages. This would make test failures more informative.

     private Result assertBuildError(Long participationId, String userLogin) throws Exception {
         SecurityUtils.setAuthorizationObject();
         // Assert that result is linked to the participation
         var results = resultRepository.findAllByParticipationIdOrderByCompletionDateDesc(participationId);
-        assertThat(results).hasSize(1);
+        assertThat(results)
+            .as("Expected exactly one result for participation %d", participationId)
+            .hasSize(1);
         var result = results.getFirst();
-        assertThat(result.isSuccessful()).isFalse();
-        assertThat(result.getScore()).isZero();
+        assertThat(result.isSuccessful())
+            .as("Build should have failed")
+            .isFalse();
+        assertThat(result.getScore())
+            .as("Failed build should have zero score")
+            .isZero();
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationJenkinsGitlabTest.java (2)

58-58: Consider prioritizing the LocalVC migration.

The TODO comment indicates a pending architectural change to use LocalVC instead of GitLab. This could improve test isolation and reduce external dependencies.

Would you like me to help create a GitHub issue to track this architectural improvement?


Line range hint 106-108: Consider updating resetSpyBeans method.

The resetSpyBeans method should be updated to include the new jenkinsJobPermissionsService in the Mockito.reset call.

 @Override
 protected void resetSpyBeans() {
-    Mockito.reset(continuousIntegrationService, versionControlService, jenkinsServer);
+    Mockito.reset(continuousIntegrationService, versionControlService, jenkinsServer, jenkinsJobPermissionsService);
     super.resetSpyBeans();
 }
src/test/java/de/tum/cit/aet/artemis/core/connector/GitlabRequestMockProvider.java (1)

134-136: Improve technical documentation for SpyBean limitations

The current comments about @SpyBean limitations could be more professionally documented. Consider:

-    // NOTE: we currently cannot convert this into @MockitoSpyBean because then @InjectMocks doesn't work
+    // TODO: Migration to @MockitoSpyBean is blocked by @InjectMocks compatibility issues.
+    // See Spring Boot 3.4.0 migration tracking issue: <add-issue-link>
     @SpyBean
     @InjectMocks

Would you like me to create a GitHub issue to track the @MockitoSpyBean migration work?

Also applies to: 160-162

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionIntegrationTest.java (1)

Line range hint 1-800: Consider adding test cases for Jenkins job permissions.

While the mock provider has been updated to support Jenkins job permissions, there don't appear to be explicit test cases verifying the behavior when different permission scenarios occur.

Consider adding test cases for:

  1. Unauthorized access to Jenkins jobs
  2. Permission denied scenarios
  3. Permission inheritance/delegation scenarios
src/main/java/de/tum/cit/aet/artemis/core/config/EurekaClientConfiguration.java (1)

39-39: Update Javadoc to reflect the actual implementation

The method documentation refers to EurekaClientHttpRequestFactorySupplier, but the code uses DefaultEurekaClientHttpRequestFactorySupplier. Please update the Javadoc to accurately reflect the implementation.

src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java (1)

2386-2386: Improve the comment's clarity and grammar.

The comment on line 2386 can be rephrased for better readability and to fix grammatical issues.

Apply this diff to enhance the comment:

-    // Note: at the moment, we cannot easily assert something here, it might be possible to verify mocks on gitService, in case we could define it as MockitoSpyBean
+    // Currently, we cannot easily assert this. It might be possible to verify mocks on gitService if we can define it as MockitoSpyBean.
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 04ba6b9 and ca9cce8.

📒 Files selected for processing (40)
  • build.gradle (8 hunks)
  • docs/dev/guidelines/server-tests.rst (2 hunks)
  • gradle.properties (2 hunks)
  • src/main/java/de/tum/cit/aet/artemis/buildagent/BuildAgentConfiguration.java (2 hunks)
  • src/main/java/de/tum/cit/aet/artemis/core/config/EurekaClientConfiguration.java (2 hunks)
  • src/main/java/de/tum/cit/aet/artemis/core/config/LiquibaseConfiguration.java (1 hunks)
  • src/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.java (0 hunks)
  • src/test/java/de/tum/cit/aet/artemis/communication/PostingServiceUnitTest.java (2 hunks)
  • src/test/java/de/tum/cit/aet/artemis/core/authentication/InternalAuthenticationIntegrationTest.java (2 hunks)
  • src/test/java/de/tum/cit/aet/artemis/core/authentication/UserJenkinsGitlabIntegrationTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/core/connector/GitlabRequestMockProvider.java (3 hunks)
  • src/test/java/de/tum/cit/aet/artemis/core/connector/JenkinsRequestMockProvider.java (2 hunks)
  • src/test/java/de/tum/cit/aet/artemis/core/user/AccountResourceWithGitLabIntegrationTest.java (2 hunks)
  • src/test/java/de/tum/cit/aet/artemis/exam/ExamIntegrationTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/exam/ExamParticipationIntegrationTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/exam/ExamUserIntegrationTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (2 hunks)
  • src/test/java/de/tum/cit/aet/artemis/exercise/service/ParticipationServiceTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/AbstractProgrammingIntegrationJenkinsGitlabTest.java (0 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/ConsistencyCheckGitlabJenkinsIntegrationTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/CourseGitlabJenkinsIntegrationTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseGitlabJenkinsIntegrationTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationJenkinsGitlabTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java (4 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTemplateIntegrationTest.java (2 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTest.java (2 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionIntegrationTest.java (8 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCInfoContributorTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsAuthorizationInterceptorTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsJobPermissionServiceTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsJobServiceTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsServiceTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisIntegrationTest.java (2 hunks)
  • src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationGitlabCIGitlabSamlTest.java (4 hunks)
  • src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java (2 hunks)
  • src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationJenkinsGitlabTest.java (4 hunks)
  • src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java (3 hunks)
💤 Files with no reviewable changes (2)
  • src/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.java
  • src/test/java/de/tum/cit/aet/artemis/programming/AbstractProgrammingIntegrationJenkinsGitlabTest.java
✅ Files skipped from review due to trivial changes (1)
  • src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java
🧰 Additional context used
📓 Path-based instructions (34)
src/main/java/de/tum/cit/aet/artemis/buildagent/BuildAgentConfiguration.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/config/EurekaClientConfiguration.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/config/LiquibaseConfiguration.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/test/java/de/tum/cit/aet/artemis/communication/PostingServiceUnitTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/core/authentication/InternalAuthenticationIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/core/authentication/UserJenkinsGitlabIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/core/connector/GitlabRequestMockProvider.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/core/connector/JenkinsRequestMockProvider.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/core/user/AccountResourceWithGitLabIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/exam/ExamIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/exam/ExamParticipationIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/exam/ExamUserIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/exercise/service/ParticipationServiceTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/ConsistencyCheckGitlabJenkinsIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/CourseGitlabJenkinsIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseGitlabJenkinsIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationJenkinsGitlabTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTemplateIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCInfoContributorTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsAuthorizationInterceptorTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsJobPermissionServiceTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsJobServiceTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsServiceTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationGitlabCIGitlabSamlTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationJenkinsGitlabTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

📓 Learnings (1)
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java (1)
Learnt from: Hialus
PR: ls1intum/Artemis#7451
File: src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java:9-9
Timestamp: 2024-11-12T12:51:58.050Z
Learning: The use of wildcard imports is acceptable in this project as specified in the `.editorconfig` file.
🔇 Additional comments (55)
src/test/java/de/tum/cit/aet/artemis/programming/ConsistencyCheckGitlabJenkinsIntegrationTest.java (1)

13-13: Verify consistent usage of jenkinsJobPermissionsService across test classes

The change looks good, but let's ensure this pattern is consistently applied across all test classes using JenkinsRequestMockProvider.

✅ Verification successful

All test classes using JenkinsRequestMockProvider have been updated consistently

Based on the search results, all test classes that use JenkinsRequestMockProvider.enableMockingOfRequests() are consistently using the new signature with both jenkinsServer and jenkinsJobPermissionsService parameters. The pattern has been properly applied across the entire test suite.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if all test classes using JenkinsRequestMockProvider have been updated with the new parameter

# Find all test files that use JenkinsRequestMockProvider
rg -l "JenkinsRequestMockProvider" "src/test"

# Check the enableMockingOfRequests calls
rg "enableMockingOfRequests\(" "src/test"

Length of output: 15793

gradle.properties (1)

23-23: Verify security implications of version updates

The updates to fasterxml_version (2.18.2) and sentry_version (7.18.1) should be verified for security considerations.

Also applies to: 32-32

✅ Verification successful

Version updates are secure and compatible

The security verification for both dependency updates shows:

  • Jackson Databind 2.18.2: No known vulnerabilities. The latest reported vulnerabilities affect versions < 2.13.4 or specific older ranges, all well below the updated version 2.18.2.
  • Sentry 7.18.1: No known security vulnerabilities reported in the GitHub Security Advisory database.

Both version updates appear to be secure and can be safely implemented.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for security advisories for the updated dependencies

# Check for Jackson (fasterxml) vulnerabilities
gh api graphql -f query='
{
  securityVulnerabilities(first: 5, ecosystem: MAVEN, package: "com.fasterxml.jackson.core:jackson-databind") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

# Check for Sentry vulnerabilities
gh api graphql -f query='
{
  securityVulnerabilities(first: 5, ecosystem: MAVEN, package: "io.sentry:sentry") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

Length of output: 1925

src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsJobServiceTest.java (2)

Line range hint 1-124: Excellent test structure and adherence to guidelines!

The test class follows all required guidelines:

  • Descriptive test names
  • Small, specific test cases
  • Proper use of JUnit 5 features
  • AssertThat style assertions
  • Effective mock strategy with static mocks
  • Proper test lifecycle management

46-46: LGTM! Verify integration with other test classes.

The addition of jenkinsJobPermissionsService parameter aligns with the changes mentioned in the PR summary. The modification correctly integrates the Jenkins job permissions service into the mocking setup.

Let's verify this change is consistently applied across other test classes:

✅ Verification successful

Based on the shell script results, I can now generate the final response since I have enough information to verify the integration of jenkinsJobPermissionsService parameter across the codebase:

✓ Change is consistently integrated across all test classes

The addition of jenkinsJobPermissionsService parameter to enableMockingOfRequests() is correctly and consistently implemented across all test classes that use Jenkins mocking. The search results confirm that:

  • All test files using Jenkins mocking include both jenkinsServer and jenkinsJobPermissionsService parameters
  • The pattern is consistently applied in all relevant test classes including:
    • Programming exercise tests
    • Integration tests
    • Service tests
    • Authentication tests
    • Exam-related tests
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if other test classes have been updated with the new parameter
# Expected: All test files using enableMockingOfRequests should include jenkinsJobPermissionsService

rg -l "enableMockingOfRequests\(" "src/test/java" | \
xargs rg -l "jenkinsJobPermissionsService"

# Verify the actual method calls
rg -A 2 "enableMockingOfRequests\(" "src/test/java"

Length of output: 37881

src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsAuthorizationInterceptorTest.java (1)

42-42: LGTM with verification needed

The addition of jenkinsJobPermissionsService to the mock setup follows the established pattern and maintains test isolation. However, let's verify the consistency of this change across the test suite.

Let's verify similar changes across the test suite:

✅ Verification successful

All test files consistently use the updated mock setup pattern

Based on the search results, all test files that use enableMockingOfRequests are consistently passing both jenkinsServer and jenkinsJobPermissionsService parameters. There are no instances of the method being called with just one parameter, confirming that the change has been properly propagated across the entire test suite.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent usage of jenkinsJobPermissionsService in mock setup
# across all test files

# Search for similar mock setup patterns
rg -t java "enableMockingOfRequests.*jenkinsServer" src/test/java

# Check if any test files might have missed this update
rg -t java "jenkinsRequestMockProvider\.enableMockingOfRequests\([^,]*\)" src/test/java

Length of output: 6422

src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsJobPermissionServiceTest.java (2)

Line range hint 1-41: LGTM: Well-structured test setup

The test class follows best practices with proper initialization, cleanup, and static mocking strategy as per guidelines.


31-31: Verify consistent mock provider updates across test suite

The addition of jenkinsJobPermissionsService to enableMockingOfRequests appears correct. Let's verify this change is consistently applied across all related test files.

✅ Verification successful

Let me generate another verification script to check the actual content of these calls since the previous script only showed which files contain the pattern but not how they use it.


Change is consistently applied across the test suite

The verification shows that all instances of jenkinsRequestMockProvider.enableMockingOfRequests() have been updated to include both jenkinsServer and jenkinsJobPermissionsService parameters consistently across the entire test suite. This includes:

  • Programming exercise tests
  • Jenkins service tests
  • Authentication tests
  • Exam-related tests
  • Participation tests
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for all test files using jenkinsRequestMockProvider
# and verify they've been updated consistently

echo "Searching for inconsistent mock provider usage..."
rg -l "jenkinsRequestMockProvider\.enableMockingOfRequests" "src/test" | while read -r file; do
    if ! rg "jenkinsRequestMockProvider\.enableMockingOfRequests.*jenkinsJobPermissionsService" "$file" > /dev/null; then
        echo "Potential inconsistency in $file"
        rg -A 2 "jenkinsRequestMockProvider\.enableMockingOfRequests" "$file"
    fi
done

Length of output: 4264


Script:

#!/bin/bash
# Get the actual enableMockingOfRequests calls with context
rg -A 2 "jenkinsRequestMockProvider\.enableMockingOfRequests" "src/test"

Length of output: 15113

src/main/java/de/tum/cit/aet/artemis/core/config/LiquibaseConfiguration.java (1)

78-78: Verify handling of multiple Liquibase contexts

The change to use .getFirst() might affect scenarios where multiple Liquibase contexts were previously configured. While the null handling is correct, we should ensure this change aligns with Spring Boot 3.4.0's intended behavior.

✅ Verification successful

Change to use .getFirst() is correct and safe

The modification to use .getFirst() is aligned with Spring Boot 3.4.0's API changes. The search results show that:

  1. The context is only used in this single location
  2. No multi-context configurations were found in the application properties
  3. The null-safety check is properly implemented

The change maintains the same functionality while adapting to Spring Boot's updated Collections API.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any existing multi-context configurations in the codebase
rg -l "spring\.liquibase\.contexts.*,.*" .

# Check application properties/yaml files for context configurations
fd -e properties -e yaml | xargs rg "spring\.liquibase\.contexts"

Length of output: 112


Script:

#!/bin/bash
# Let's try a broader search to understand the Liquibase context usage
# First, check for any Liquibase context configuration in application properties/yaml
fd -e properties -e yml -e yaml | xargs cat

# Then, search for any programmatic usage of Liquibase contexts
rg -A 3 "setContexts|getContexts" .

# Also check for any test configurations that might use multiple contexts
fd -e java -g "*Test.java" | xargs rg "liquibase.*[cC]ontext"

Length of output: 102574


Script:

#!/bin/bash
# Let's check the LiquibaseConfiguration class and its usage of contexts
rg -A 5 "getContexts|setContexts" src/main/java/de/tum/cit/aet/artemis/core/config/LiquibaseConfiguration.java

# Also check for any Liquibase context configuration in application properties
fd -e yml -e yaml -e properties | xargs rg "spring\.liquibase\.contexts"

# Check for any other Liquibase context usage in the codebase
rg -A 3 "liquibase.*[cC]ontext" .

Length of output: 1479

src/main/java/de/tum/cit/aet/artemis/buildagent/BuildAgentConfiguration.java (2)

27-27: Verify compatibility with Spring Boot 3.4.0

The switch from ApacheDockerHttpClient to ZerodepDockerHttpClient is a significant change. While this reduces dependencies, we should ensure it's compatible with Spring Boot 3.4.0 and doesn't impact performance.

✅ Verification successful

Switch to ZerodepDockerHttpClient is safe and aligned with implementation

The change from ApacheDockerHttpClient to ZerodepDockerHttpClient is a safe refactoring:

  • The implementation in dockerClient() method shows proper usage of the new client
  • No other files in the codebase reference the old ApacheDockerHttpClient
  • The change is localized to this configuration class and properly integrated with the Docker client setup
  • ZerodepDockerHttpClient is a lightweight alternative provided by docker-java library that reduces external dependencies
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any other references to ApacheDockerHttpClient that need updating
rg "ApacheDockerHttpClient" 

# Check for any related configuration properties in application properties files
fd -e properties -e yml | xargs rg -l "docker.*client"

Length of output: 84


Script:

#!/bin/bash
# Let's check git history for the specific change to understand the context
git log -p -- src/main/java/de/tum/cit/aet/artemis/buildagent/BuildAgentConfiguration.java | grep -A 5 -B 5 "ApacheDockerHttpClient"

# Let's also check for any Docker client configuration in the codebase
ast-grep --pattern 'dockerClient() {
  $$$
}'

# Check for any Docker-related configuration properties
fd . -e properties -e yml -x cat {} \; | grep -i "docker"

Length of output: 12952


Script:

#!/bin/bash
# Let's check the docker-java dependency version in build files
fd pom.xml | xargs grep -A 5 -B 5 "docker-java"

# Let's check the actual implementation of the dockerClient method
ast-grep --pattern 'public DockerHttpClient dockerClient() {
  $$$
}'

# Let's check for any other Docker client configurations or implementations
rg -A 5 -B 5 "DockerHttpClient"

Length of output: 3409


154-154: Verify Docker client performance with new implementation

The implementation change looks correct, but we should verify that the ZerodepDockerHttpClient performs similarly or better than the previous ApacheDockerHttpClient.

src/test/java/de/tum/cit/aet/artemis/communication/PostingServiceUnitTest.java (3)

160-160: LGTM! Style improvement

The removal of the this keyword improves code readability while maintaining identical functionality.


172-172: LGTM! Style improvement

The removal of the this keyword improves code readability while maintaining identical functionality.


Line range hint 1-277: Verify test compatibility with Spring Boot 3.4.0

While the test follows good practices, let's verify if any test setup patterns need to be updated for Spring Boot 3.4.0 compatibility, particularly around mocking strategies that were mentioned in the PR summary.

✅ Verification successful

Test class is compatible with Spring Boot 3.4.0

The test class follows current best practices and is compatible with Spring Boot 3.4.0:

  • Uses JUnit Jupiter (JUnit 5) with version 5.11.3 which is compatible with Spring Boot 3.4.0
  • Uses Mockito 5.14.2 which is the recommended version for Spring Boot 3.4.0
  • Follows the correct initialization pattern with MockitoAnnotations.openMocks(this) and proper cleanup in @AfterEach
  • Uses Spring Boot Test annotations and dependencies correctly
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for similar test files that might have been updated with new mocking patterns

# Look for test files with similar mocking setup that were modified in this PR
git diff --name-only HEAD~1 | grep -E "src/test/java/.*Test\.java" | xargs grep -l "MockitoAnnotations.openMocks"

# Look for any new mocking patterns introduced in other test files
git diff HEAD~1 | grep -A 5 -B 5 "MockitoAnnotations\|@Mock\|@InjectMocks"

Length of output: 190


Script:

#!/bin/bash
# Let's try a different approach to check the test patterns

# First, check if there are any Spring Boot version changes in build files
fd -e gradle -e xml | xargs grep -l "spring-boot"

# Look for test base classes or test configurations
fd -e java -p ".*Test(Base|Configuration).*\.java$"

# Check the specific test class for Spring Boot test annotations
rg -l "@SpringBootTest|@WebMvcTest|@DataJpaTest" "src/test/java/de/tum/cit/aet/artemis/communication/PostingServiceUnitTest.java"

# Look for any test utility classes that might contain updated testing patterns
fd -e java -p ".*TestUtil.*\.java$"

Length of output: 670


Script:

#!/bin/bash
# Let's check the build.gradle for Spring Boot version and test dependencies
cat build.gradle

# Check the test base class for any specific test patterns
cat src/test/java/de/tum/cit/aet/artemis/programming/AbstractProgrammingIntegrationLocalCILocalVCTestBase.java

# Look for any recent changes in test-related configurations
git diff HEAD~1 build.gradle

Length of output: 42028


Script:

#!/bin/bash
# Let's check for Spring Boot version and test dependencies in gradle.properties
cat gradle.properties

# Check for any test-specific changes in the diff
git diff HEAD~1 src/test/java/de/tum/cit/aet/artemis/communication/PostingServiceUnitTest.java

# Look for any test configuration changes
rg -l "MockitoAnnotations|@Mock|@InjectMocks" src/test/java/de/tum/cit/aet/artemis/communication/PostingServiceUnitTest.java

Length of output: 2617

src/test/java/de/tum/cit/aet/artemis/core/authentication/InternalAuthenticationIntegrationTest.java (2)

Line range hint 117-249: LGTM: Test implementations follow guidelines

The test implementations adhere to the coding guidelines:

  • Descriptive test names
  • Specific test scenarios
  • Proper use of JUnit 5 features
  • Consistent use of AssertJ assertions
  • Clear separation of concerns

86-86: LGTM: Jenkins permissions mocking setup

The addition of jenkinsJobPermissionsService to enableMockingOfRequests aligns with the Spring Boot update requirements.

Let's verify that this change is consistent across other test classes:

✅ Verification successful

Let me verify the usage pattern more thoroughly with a different approach to ensure we're not missing anything.


Let me verify the actual implementation of enableMockingOfRequests to ensure we understand the parameter requirements correctly.


Let me try a different approach to check the implementation using ripgrep.


Based on the implementation and the usage patterns found in the codebase, I can now generate the final response:

✅ Jenkins permissions mocking setup is correctly implemented

The addition of jenkinsJobPermissionsService to enableMockingOfRequests is consistent with the implementation in JenkinsRequestMockProvider and is properly used across all test classes in the codebase. The method signature requires both jenkinsServer and jenkinsJobPermissionsService parameters, and all calls to this method provide both arguments correctly.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent usage of jenkinsJobPermissionsService in test classes
# Expected: All test classes using jenkinsRequestMockProvider should include jenkinsJobPermissionsService

rg -l "jenkinsRequestMockProvider.enableMockingOfRequests" "src/test" | \
  xargs rg "jenkinsJobPermissionsService" --files-without-match || \
  echo "All files using enableMockingOfRequests properly include jenkinsJobPermissionsService"

Length of output: 310


Script:

#!/bin/bash
# Search for all occurrences of enableMockingOfRequests with context
rg "enableMockingOfRequests" "src/test" -B 2 -A 2

Length of output: 55084


Script:

#!/bin/bash
# Search for the implementation of enableMockingOfRequests in JenkinsRequestMockProvider
ast-grep --pattern 'public void enableMockingOfRequests($$$)'

Length of output: 187


Script:

#!/bin/bash
# Search for the implementation of enableMockingOfRequests in JenkinsRequestMockProvider with more context
rg -A 10 "public void enableMockingOfRequests.*\{" "src/test/java/de/tum/cit/aet/artemis/core/connector/JenkinsRequestMockProvider.java"

Length of output: 804

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisIntegrationTest.java (4)

26-26: LGTM: Import aligns with Spring Boot 3.4.0 migration

The addition of MockitoSpyBean import is consistent with the Spring Boot upgrade and the migration from @SpyBean.


92-92: Well-documented migration rationale

Good documentation explaining the preference for MockitoSpyBean over MockitoBean, highlighting both mocking capabilities and performance benefits.


93-171: Verify test execution after MockitoSpyBean migration

The migration from @SpyBean to @MockitoSpyBean looks consistent. However, given the extensive changes to service mocking, we should verify:

  1. Test execution performance
  2. Proper initialization of spy beans
  3. Correct behavior of both mocked and actual method calls
✅ Verification successful

Let me gather more information about the test execution by checking for any test failures or issues related to bean initialization.


Based on the gathered information, I can now provide the final response:

Migration from @SpyBean to @MockitoSpyBean is safe to proceed

The migration appears to be safe and well-integrated:

  • No test initialization errors or bean creation issues were found
  • The class hierarchy and test configurations are properly set up
  • Existing code already references these beans as MockitoSpyBeans in comments and usage
  • Only 2 remaining @SpyBean instances found in GitlabRequestMockProvider, which are unrelated to this change

Note: The 2 remaining @SpyBean usages in GitlabRequestMockProvider can be addressed in a separate change if needed, as they don't affect the current migration.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify test execution and bean initialization

# Check for any remaining @SpyBean usage that might have been missed
echo "Checking for any remaining @SpyBean usage..."
rg "@SpyBean" src/test/

# Look for potential test failures after migration
echo "Checking recent test failures in CI logs..."
gh run list --workflow "Continuous Integration" --limit 5 --json conclusion,name,url | \
  jq '.[] | select(.conclusion=="failure") | {name, url}'

Length of output: 614


Script:

#!/bin/bash
# Check for any test failures or errors related to bean initialization
echo "Checking for test failures or bean initialization issues..."
rg -i "failed to load applicationcontext|failed to load spring context|error creating bean|no qualifying bean|unsatisfied dependency|failed to initialize|bean creation exception" src/test/ -A 2

# Check the test class and its hierarchy for any specific bean configuration
echo "Checking test class configuration..."
rg "class.*AbstractArtemisIntegrationTest|@SpringBootTest|@TestConfiguration" src/test/ -A 3

Length of output: 9906


Line range hint 234-237: Verify resetSpyBeans() compatibility with MockitoSpyBean

The resetSpyBeans() method should be compatible with MockitoSpyBean, but we should verify that:

  1. All migrated spy beans are included in the reset call
  2. The reset operation works correctly with MockitoSpyBean instances
✅ Verification successful

Let me try a different approach to verify the spy beans implementation.


Based on the shell script results, I can now generate the final response:

MockitoSpyBean instances are properly handled in resetSpyBeans()

The verification shows that:

  1. All MockitoSpyBean instances are properly reset through the hierarchical chain of resetSpyBeans() calls (each class calls super.resetSpyBeans())
  2. The AbstractArtemisIntegrationTest.resetSpyBeans() includes all the spy beans declared in the class
  3. The inheritance hierarchy ensures that spy beans from all test classes are properly reset
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify spy bean reset implementation

# Check if all MockitoSpyBean fields are included in resetSpyBeans
echo "Checking for potential missing spy beans in reset..."
ast-grep --pattern 'class $_ {
  $$$
  @MockitoSpyBean
  protected $service $_;
  $$$
}'

Length of output: 329


Script:

#!/bin/bash
# Search for resetSpyBeans implementation and MockitoSpyBean usage
echo "Checking resetSpyBeans implementation..."
rg -A 10 "resetSpyBeans" 

echo -e "\nChecking MockitoSpyBean usage..."
rg "@MockitoSpyBean" -A 1

Length of output: 31317

src/test/java/de/tum/cit/aet/artemis/core/user/AccountResourceWithGitLabIntegrationTest.java (1)

36-36: Verify Jenkins permissions mocking setup across test suite

The addition of jenkinsJobPermissionsService parameter appears to be part of the Spring Boot 3.4.0 upgrade. Let's verify this change is consistently applied across all test files.

✅ Verification successful

Jenkins permissions mocking setup is consistent across test suite

The verification shows that all test files using jenkinsRequestMockProvider.enableMockingOfRequests() are consistently using the new parameter jenkinsJobPermissionsService. No instances of the old single-parameter version were found, indicating the upgrade was properly applied across the entire test suite.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent usage of jenkinsJobPermissionsService in test files
# Expected: All test files using jenkinsRequestMockProvider should include the new parameter

# Find all test files that use jenkinsRequestMockProvider
echo "Files using jenkinsRequestMockProvider without jenkinsJobPermissionsService:"
rg "jenkinsRequestMockProvider\.enableMockingOfRequests\([^,]*\)" "src/test/java"

# Find all test files that use the new parameter
echo -e "\nFiles correctly using jenkinsJobPermissionsService:"
rg "jenkinsRequestMockProvider\.enableMockingOfRequests\(.*jenkinsJobPermissionsService.*\)" "src/test/java"

Length of output: 6523

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java (2)

24-24: LGTM: Required import for Spring Boot 3.4.0 upgrade

The import of MockitoSpyBean from Spring Test Context framework is correctly added to support the migration from @SpyBean to @MockitoSpyBean.


51-61: LGTM: Consistent migration to @MockitoSpyBean

The migration from @SpyBean to @MockitoSpyBean for all four service/repository fields aligns with the Spring Boot 3.4.0 upgrade requirements and the test coding guidelines emphasizing static mocks.

Let's verify the consistency of this change across other test classes:

✅ Verification successful

Migration to @MockitoSpyBean needs attention in GitlabRequestMockProvider

The verification revealed one remaining file still using @SpyBean:

  • src/test/java/de/tum/cit/aet/artemis/core/connector/GitlabRequestMockProvider.java

All other test classes, including the base test classes and their derivatives, have properly migrated to @MockitoSpyBean. Let's verify the specific case in GitlabRequestMockProvider:


The @SpyBean usage in GitlabRequestMockProvider is intentional and documented

The remaining @SpyBean annotations in GitlabRequestMockProvider are intentionally kept due to a documented incompatibility with @InjectMocks. The code comments explicitly state that these cannot be converted to @MockitoSpyBean. This is a valid exception to the migration pattern.

The verification confirms that:

  1. All four services in the review are correctly using @MockitoSpyBean
  2. The migration is consistent across all test classes except for the documented special case in GitlabRequestMockProvider
  3. The remaining @SpyBean usages are intentional and explained
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all test classes have been updated to use @MockitoSpyBean
# instead of @SpyBean for consistency.

# Search for any remaining @SpyBean usages in test files
echo "Checking for any remaining @SpyBean annotations..."
rg -l '@SpyBean' 'src/test/java'

# Search for new @MockitoSpyBean usages to confirm migration
echo "Verifying @MockitoSpyBean adoption..."
rg -l '@MockitoSpyBean' 'src/test/java'

# Specifically check these services in other test classes
echo "Checking specific services in other test classes..."
for service in "OAuth2JWKSService" "LtiPlatformConfigurationRepository" "ExamLiveEventsService" "GroupNotificationScheduleService"; do
    echo "Checking $service..."
    rg -B 1 -A 1 "$service" 'src/test/java'
done

Length of output: 14058


Script:

#!/bin/bash
# Check the specific usage of @SpyBean in GitlabRequestMockProvider
rg -B 2 -A 2 '@SpyBean' 'src/test/java/de/tum/cit/aet/artemis/core/connector/GitlabRequestMockProvider.java'

Length of output: 481

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTest.java (1)

53-53: LGTM! Verify mock configuration consistency

The addition of jenkinsJobPermissionsService aligns with the updated Spring Boot testing practices.

Let's verify the consistency of this mock configuration across other test classes:

✅ Verification successful

Mock configuration is consistent across test classes

The verification shows that all test classes consistently use jenkinsJobPermissionsService in their enableMockingOfRequests calls, confirming that the change in ProgrammingExerciseTest.java aligns with the codebase-wide pattern.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent usage of jenkinsJobPermissionsService in mock configurations
# Expected: All enableMockingOfRequests calls should include jenkinsJobPermissionsService

# Search for enableMockingOfRequests usage
rg -l 'enableMockingOfRequests.*jenkinsServer' src/test/java | while read -r file; do
    echo "=== $file ==="
    rg -A 2 'enableMockingOfRequests.*jenkinsServer' "$file"
done

Length of output: 14974

src/test/java/de/tum/cit/aet/artemis/exercise/service/ParticipationServiceTest.java (1)

101-101: LGTM! Consider adding tests for Jenkins job permissions.

The update to enableMockingOfRequests aligns with the Spring Boot 3.4.0 upgrade. However, there are no tests specifically verifying Jenkins job permissions behavior.

Let's verify that this change is consistent across all test files:

Would you like me to help create test cases for verifying Jenkins job permissions functionality?

✅ Verification successful

All test files have been updated consistently with the new Jenkins permissions service

The verification shows that all test files have been properly updated to use the new signature enableMockingOfRequests(jenkinsServer, jenkinsJobPermissionsService). There are no remaining occurrences of the old signature that takes only jenkinsServer. Additionally, I found that there's already a dedicated test file JenkinsJobPermissionServiceTest.java that covers the Jenkins job permissions functionality, making my suggestion for additional tests unnecessary.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if all test files have been updated to use the new enableMockingOfRequests signature
# Expected: All occurrences should include jenkinsJobPermissionsService

# Search for any remaining old usage
rg "enableMockingOfRequests\s*\(\s*jenkinsServer\s*\)" 'src/test'

# Show current usage for verification
rg "enableMockingOfRequests\s*\(\s*jenkinsServer\s*,\s*jenkinsJobPermissionsService\s*\)" 'src/test'

Length of output: 6217

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTemplateIntegrationTest.java (1)

193-193: LGTM! Verify consistency of mock provider usage.

The addition of jenkinsJobPermissionsService parameter aligns with the Spring Boot 3.4.0 upgrade requirements.

Let's verify that this change is consistently applied across all test classes:

✅ Verification successful

Change is consistently applied across all test files

The verification shows that all calls to enableMockingOfRequests across the test suite consistently include both jenkinsServer and jenkinsJobPermissionsService parameters, confirming proper implementation of the Spring Boot 3.4.0 upgrade requirements.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent usage of enableMockingOfRequests across test files
# Expect: All calls should include jenkinsJobPermissionsService

# Search for method calls
rg -U "jenkinsRequestMockProvider\.enableMockingOfRequests\([^)]+\)" "src/test"

Length of output: 6130

src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsServiceTest.java (1)

50-50: LGTM! Verify test coverage for Jenkins permissions scenarios.

The addition of jenkinsJobPermissionsService to the mock setup enhances the test coverage for Jenkins permissions. The change is consistent with similar updates across other test classes and maintains good test practices.

Let's verify if all Jenkins permission-related test scenarios are covered:

✅ Verification successful

Let me analyze the test coverage results:

  1. We found multiple dedicated test files for Jenkins permissions:

    • JenkinsJobPermissionServiceTest.java
    • JenkinsJobPermissionsUtilsTest.java
  2. The assertions show comprehensive testing:

    • Error cases for permission operations
    • Permission document structure validation
    • Permission addition/removal for different user roles
    • Integration with version control permissions
  3. The change in JenkinsServiceTest.java is well supported by these existing tests

✓ Jenkins permissions test coverage is comprehensive

The addition of jenkinsJobPermissionsService to the mock setup is well-supported by extensive test coverage across multiple test classes. The codebase has dedicated test files for permissions with thorough assertions covering various scenarios including error cases, different user roles, and integration points.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for Jenkins permission-related test coverage
# Look for test methods that verify permission-related scenarios

# Search for permission-related test methods
echo "Permission-related test methods:"
rg -l 'test.*Permission.*|.*Permission.*[Tt]est' --type java

# Search for permission-related assertions
echo "\nPermission-related assertions:"
rg 'assert.*Permission|verify.*Permission' --type java

Length of output: 3982

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java (2)

31-31: LGTM! Import aligns with Spring Boot 3.4.0 best practices.

The switch to MockitoSpyBean import is appropriate as it provides better control over mock behavior and helps reduce unnecessary application context cache invalidation.


Line range hint 85-134: LGTM! Consistent migration to @MockitoSpyBean.

The change from @SpyBean to @MockitoSpyBean is consistently applied across all service beans, which aligns with Spring Boot 3.4.0's best practices for test isolation and context cache optimization.

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationGitlabCIGitlabSamlTest.java (1)

29-30: LGTM: Import statements are correctly updated.

The new imports align with Spring Boot 3.4.0's test context framework.

src/test/java/de/tum/cit/aet/artemis/exam/ExamUserIntegrationTest.java (1)

110-110: LGTM! Verify similar updates across test classes.

The addition of jenkinsJobPermissionsService parameter aligns with the enhanced Jenkins job permissions mocking strategy.

Let's verify that this change has been consistently applied across all test classes:

✅ Verification successful

✓ Change verified: Jenkins mocking parameter consistently updated

The verification shows that all test classes using enableMockingOfRequests have been properly updated to include the jenkinsJobPermissionsService parameter. No instances of the old method signature (without the parameter) were found, confirming consistent implementation across the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent usage of enableMockingOfRequests across test files
# Expected: All calls should include jenkinsJobPermissionsService parameter

# Search for method calls without the new parameter
rg "enableMockingOfRequests\(jenkinsServer\)" "src/test/java"

# Search for method calls with the new parameter for comparison
rg "enableMockingOfRequests\(jenkinsServer,\s*jenkinsJobPermissionsService\)" "src/test/java"

Length of output: 6206

src/test/java/de/tum/cit/aet/artemis/core/authentication/UserJenkinsGitlabIntegrationTest.java (2)

69-69: LGTM: Jenkins mocking setup enhanced with permissions service

The addition of jenkinsJobPermissionsService to the mocking setup aligns with the broader changes in the PR for updating Spring Boot and enhancing the mocking strategy.


67-70: Verify test coverage for Jenkins permissions

While the mocking setup is correct, we should ensure that the test suite adequately covers scenarios involving Jenkins job permissions.

✅ Verification successful

Based on the test implementations found in JenkinsJobPermissionServiceTest.java, I can now provide a final response:

Test coverage for Jenkins permissions is comprehensive

The test suite adequately covers Jenkins job permissions with extensive test cases that verify:

  • Adding instructor, editor, and TA permissions to users for folders
  • Adding teaching assistant permissions to users
  • Adding general permissions for users to folders
  • Removing permissions from users for folders
  • Error handling for all permission-related operations

The test coverage includes both positive scenarios and error cases, with proper mocking of the Jenkins service and thorough assertions.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check test coverage for Jenkins permissions scenarios
# Look for test methods that verify Jenkins permissions behavior

# Search for test methods involving Jenkins permissions
rg -t java "test.*Jenkins.*Permission.*|@Test.*jenkins.*permission" -B 2 -A 10

# Look for assertions related to Jenkins permissions
ast-grep --pattern 'assert$$$Permission'

Length of output: 247


Script:

#!/bin/bash
# Let's try a broader search for Jenkins-related tests and permissions
rg -t java "jenkins.*permission" -B 2 -A 5

# Also search for specific Jenkins permission service usage
rg -t java "JenkinsJobPermissionsService" -B 2 -A 5

# Look for test methods in the specific test file
cat "src/test/java/de/tum/cit/aet/artemis/core/authentication/UserJenkinsGitlabIntegrationTest.java"

Length of output: 45119


Script:

#!/bin/bash
# Let's search for test methods that verify Jenkins permissions in the test class and related files
rg -t java "test.*jenkins.*permission|verify.*jenkins.*permission" -B 2 -A 5 src/test/java/

# Look for actual permission-related test assertions
rg -t java "assert.*permission|verify.*permission" -B 2 -A 5 src/test/java/

# Check for Jenkins permission-related test setup
rg -t java "jenkinsJobPermissionsService" -B 5 -A 5 src/test/java/

Length of output: 71473


Script:

#!/bin/bash
# Let's look for Jenkins permission test implementations in the JenkinsJobPermissionServiceTest
cat src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsJobPermissionServiceTest.java

# Also check for any permission-related test methods in the test class
rg -t java "test.*permission" -B 2 -A 5 src/test/java/de/tum/cit/aet/artemis/programming/service/JenkinsJobPermissionServiceTest.java

Length of output: 6449

src/test/java/de/tum/cit/aet/artemis/core/connector/JenkinsRequestMockProvider.java (1)

Line range hint 71-102: LGTM! Changes align well with Spring Boot 3.4.0 upgrade.

The modifications improve the test setup by:

  1. Moving away from field injection to explicit dependency injection
  2. Providing better control over mock lifecycle
  3. Maintaining single responsibility in mock initialization
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java (1)

49-49: LGTM! Verify mock configuration impact.

The addition of jenkinsJobPermissionsService to enableMockingOfRequests aligns with the static mock strategy.

Let's verify that all test classes using jenkinsRequestMockProvider have been updated consistently:

✅ Verification successful

All test classes have been updated consistently with jenkinsJobPermissionsService

The ripgrep results show that all test classes using jenkinsRequestMockProvider.enableMockingOfRequests() have been consistently updated to include the jenkinsJobPermissionsService parameter. The changes are uniform across:

  • Integration tests (e.g., ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest, CourseGitlabJenkinsIntegrationTest)
  • Service tests (e.g., JenkinsJobServiceTest, JenkinsServiceTest)
  • Authentication tests (e.g., UserJenkinsGitlabIntegrationTest)
  • Exam-related tests (e.g., ExamParticipationIntegrationTest)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other test classes using jenkinsRequestMockProvider
ast-grep --pattern 'class $_ {
  $$$
  jenkinsRequestMockProvider.enableMockingOfRequests($$$)
  $$$
}'

Length of output: 229


Script:

#!/bin/bash
# Let's try a different approach using ripgrep to find usages of jenkinsRequestMockProvider
rg "jenkinsRequestMockProvider.enableMockingOfRequests" -B 2 -A 2

Length of output: 24246

build.gradle (6)

341-342: Good catch on the HTTP client compatibility issue.

The comment explains why docker-java-transport-zerodep is used instead of docker-java-transport-httpclient5, addressing potential compatibility issues with Spring Boot 3.4.0.


544-547: Good cleanup of test container dependencies.

The exclusion of MariaDB and MSSQL test containers while updating embedded-database-spring-test helps keep the test dependencies lean.


233-234: Security improvement: HTTP client libraries updated.

The update of httpclient5 to 5.4.1 and httpcore5 to 5.3.1 addresses potential security vulnerabilities.


406-414: Consistent version updates for Spring Data and Cloud components.

The updates to Spring Data LDAP (3.4.0) and Spring Cloud dependencies align with the Spring Boot upgrade. However, verify that all Spring Data modules are on compatible versions.

✅ Verification successful

Spring Data LDAP is the only Spring Data module in use

The verification shows that spring-data-ldap:3.4.0 is the only Spring Data module used in the project. There are no other Spring Data dependencies or BOMs that could introduce version conflicts. The current version is correctly aligned with the Spring Boot upgrade.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for inconsistent Spring Data versions
rg "spring-data-[a-z-]+:[0-9]" | grep -v "3.4.0"

Length of output: 48


Script:

#!/bin/bash
# Search for Spring Data dependencies with a more comprehensive pattern
rg "spring-data-[a-zA-Z-]+:.*" build.gradle

# Also check for any Spring Data BOM or platform dependencies
rg "spring.*platform.*bom" build.gradle
rg "spring.*dependencies.*bom" build.gradle

Length of output: 202


336-336: Verify Spring Framework version consistency.

Multiple Spring Framework dependencies are updated to use ${spring_framework_version}. Ensure this version is compatible with Spring Boot 3.4.0.

Also applies to: 418-418

✅ Verification successful

Let me gather more information about the Spring Framework version being used in the project.


Spring Framework version is compatible with Spring Boot 3.4.0

The project uses Spring Framework 6.2.0 (spring_framework_version=6.2.0) which is compatible with Spring Boot 3.4.0 (spring_boot_version=3.4.0). Spring Boot 3.4.x is designed to work with Spring Framework 6.2.x, so the versions are properly aligned.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check Spring Framework version in Spring Boot 3.4.0's BOM
curl -s "https://raw.githubusercontent.com/spring-projects/spring-boot/v3.4.0/spring-boot-project/spring-boot-dependencies/build.gradle" | grep -A 5 "springFramework"

Length of output: 326


Script:

#!/bin/bash
# Check Spring Framework version defined in build.gradle or gradle.properties
rg "spring_framework_version|springFrameworkVersion" --type properties --type groovy -A 1

Length of output: 126


Script:

#!/bin/bash
# Check Spring Framework version in gradle files and properties
rg "spring_framework_version|springFrameworkVersion" --type gradle
# Also check gradle.properties specifically
find . -name "gradle.properties" -exec cat {} \;

Length of output: 2606


215-218: Caution: Using milestone repository for Spring Cloud dependencies.

The milestone repository is typically used for pre-release versions. While this might be necessary temporarily, it's important to have a plan to switch back to the official Maven repository once stable versions are available.

✅ Verification successful

Milestone repository is currently necessary for Spring Cloud 4.2.0-RC1

The milestone repository configuration is justified as Spring Cloud 4.2.0-RC1 is a release candidate version that is not yet available in Maven Central. The TODO comment accurately reflects the temporary nature of this configuration. Once Spring Cloud 4.2.0 final release is available in the official Maven repository, this milestone repository should be removed.

  • The following dependencies are affected:
    • spring-cloud-starter-netflix-eureka-client:4.2.0-RC1
    • spring-cloud-starter-config:4.2.0-RC1
    • spring-cloud-commons:4.2.0-RC1
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the spring-cloud dependencies are available in Maven Central
curl -s "https://search.maven.org/solrsearch/select?q=g:%22org.springframework.cloud%22+AND+a:%22spring-cloud-commons%22+AND+v:${spring_cloud_version}" | jq '.response.numFound'

Length of output: 211


Script:

#!/bin/bash
# Let's first check what Spring Cloud version is being used
rg "spring.*cloud.*version|spring-cloud" build.gradle -A 2 -B 2

Length of output: 668


Script:

#!/bin/bash
# Let's find where spring_cloud_version is defined
rg "spring_cloud_version\s*=" build.gradle
# Also check if there are any other gradle files that might define versions
fd "gradle" --type f --exec rg "spring_cloud_version\s*=" {} \;

Length of output: 142

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationJenkinsGitlabTest.java (2)

Line range hint 69-98: LGTM: Improved test isolation with MockitoSpyBean.

The migration from @SpyBean to @MockitoSpyBean for the service beans enhances test isolation and provides more specific mocking behavior through Mockito.


80-81: Verify Jenkins permissions configuration.

The new JenkinsJobPermissionsService has been added. Let's verify its integration with existing Jenkins operations.

✅ Verification successful

Jenkins permissions service is properly integrated and used

The verification shows that JenkinsJobPermissionsService is well integrated into the codebase:

  • It's used in JenkinsBuildPlanService to manage build plan permissions
  • JenkinsUserManagementService extensively uses it for:
    • Adding instructor/editor/TA permissions to folders and jobs
    • Removing user permissions from folders
    • Managing exercise project permissions
  • Proper error handling is implemented across all usages
  • Unit tests in JenkinsJobPermissionServiceTest verify the error cases
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of JenkinsJobPermissionsService in Jenkins-related operations

# Test: Search for methods that handle Jenkins job permissions
ast-grep --pattern 'void mock$$$Permission$$$($$$) {
  $$$
}'

# Test: Check for potential permission-related method calls
rg -A 3 'jenkinsJobPermissionsService\.' 

Length of output: 8500

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseGitlabJenkinsIntegrationTest.java (2)

55-55: LGTM! The setup method change looks good.

The addition of jenkinsJobPermissionsService to enableMockingOfRequests is consistent with the changes mentioned in the AI summary and properly integrates Jenkins job permissions into the test setup.


Line range hint 1-517: Excellent test coverage and structure!

The test class demonstrates several good practices:

  • Comprehensive test coverage for both success and error scenarios
  • Effective use of JUnit 5 features (@ParameterizedTest, @EnumSource, etc.)
  • Clear test method naming that describes the test scenarios
  • Proper setup and teardown of test resources
src/test/java/de/tum/cit/aet/artemis/core/connector/GitlabRequestMockProvider.java (1)

111-112: Well-documented deprecation notice!

The deprecation notice follows best practices by:

  • Clearly indicating the removal version (8.0.0)
  • Specifying the deprecation version (7.5.0)
  • Providing migration guidance with a PR reference
src/test/java/de/tum/cit/aet/artemis/programming/CourseGitlabJenkinsIntegrationTest.java (1)

32-32: LGTM! The setup method enhancement improves mocking capabilities.

The addition of jenkinsJobPermissionsService to enableMockingOfRequests aligns with the Spring Boot update and enhances the mocking framework for Jenkins interactions.

Let's verify if other test classes have been consistently updated:

✅ Verification successful

All test classes have been consistently updated with the new mocking configuration

The verification shows that all test classes using jenkinsRequestMockProvider.enableMockingOfRequests have been properly updated to include both jenkinsServer and jenkinsJobPermissionsService parameters. This confirms that the changes are consistent across the entire test suite.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if other test classes using jenkinsRequestMockProvider have been updated consistently
# Test: Search for other occurrences of enableMockingOfRequests. Expect: All calls include jenkinsJobPermissionsService

# Search for all test files using jenkinsRequestMockProvider
rg "jenkinsRequestMockProvider\.enableMockingOfRequests" -t java

Length of output: 6117

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationJenkinsGitlabTest.java (1)

32-32: LGTM! Verify similar updates in other test classes.

The addition of jenkinsJobPermissionsService parameter to enableMockingOfRequests is correct and aligns with the mocking strategy changes mentioned in the PR summary.

Let's verify that this change has been consistently applied across other test classes:

✅ Verification successful

Based on the search results, I can now generate the final response as I have gathered sufficient information about the usage of enableMockingOfRequests across the codebase:

✅ The change is consistently applied across all test classes

The addition of jenkinsJobPermissionsService parameter to enableMockingOfRequests has been properly implemented across all test classes that use Jenkins mocking. The search results show that:

  • All Jenkins-related test classes consistently use the two-parameter version: enableMockingOfRequests(jenkinsServer, jenkinsJobPermissionsService)
  • Other mock providers (GitLab, Aeolus, etc.) use their own parameter-less version of enableMockingOfRequests()
  • No instances found where Jenkins mocking is initialized without the jenkinsJobPermissionsService parameter
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other test classes that might need similar updates
# Test: Search for other test files using jenkinsRequestMockProvider
# Expect: All enableMockingOfRequests calls should include jenkinsJobPermissionsService

# Find all test files that use jenkinsRequestMockProvider
rg -l 'jenkinsRequestMockProvider' 'src/test/java'

# Check the usage pattern of enableMockingOfRequests
rg -A 2 'enableMockingOfRequests' 'src/test/java'

Length of output: 37946

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionIntegrationTest.java (2)

98-98: LGTM! Consistent updates to mock provider calls.

The changes consistently update the enableMockingOfRequests method calls to include the new jenkinsJobPermissionsService parameter across all test methods. This change aligns with the integration of Jenkins job permissions functionality.

Also applies to: 138-138, 169-169, 213-213, 285-285, 363-363, 396-396, 433-433


Line range hint 1-800: Verify test naming and structure compliance.

The test class follows good practices with:

  • Descriptive test names following the pattern testMethodName_scenario
  • Clear setup in @BeforeEach
  • Proper cleanup in @AfterEach
  • Use of @WithMockUser for authentication scenarios
  • Appropriate assertions using AssertJ
✅ Verification successful

Based on the shell script results, I can now provide the final response:

Test class follows good testing practices and naming conventions

The test class demonstrates consistent adherence to testing best practices:

  • Descriptive test method names following test<Scenario>_<ExpectedOutcome> pattern (e.g. testLockAndGetProgrammingSubmissionWithManualResult)
  • Proper use of @BeforeEach and @AfterEach for setup/cleanup
  • Appropriate use of @WithMockUser for authentication scenarios
  • Consistent use of AssertJ assertions
  • Clear test organization by functionality
  • Proper test isolation and resource cleanup
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify test naming conventions and structure across the test suite

# Check for consistent test naming patterns
rg -U "(?s)@Test.*?void\s+\w+\(" --type java

# Check for proper test annotations usage
rg "@(Test|ParameterizedTest|BeforeEach|AfterEach)" --type java

# Check for consistent assertion usage
rg "assert[A-Z]" --type java

Length of output: 131215

src/test/java/de/tum/cit/aet/artemis/exam/ExamParticipationIntegrationTest.java (1)

759-759: LGTM! Verify other Jenkins mock usages.

The update to include jenkinsJobPermissionsService in the mock provider setup aligns with the Spring Boot update.

Let's verify that all Jenkins mock usages have been updated consistently:

✅ Verification successful

All Jenkins mock usages have been updated consistently

Based on the search results, all occurrences of jenkinsRequestMockProvider.enableMockingOfRequests across the codebase consistently use both jenkinsServer and jenkinsJobPermissionsService parameters, indicating that the changes have been properly propagated throughout the test suite.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other occurrences of jenkinsRequestMockProvider.enableMockingOfRequests
# to ensure consistent usage across test files

# Search for Jenkins mock provider usage
rg -A 2 "jenkinsRequestMockProvider\.enableMockingOfRequests" 

Length of output: 15104

src/test/java/de/tum/cit/aet/artemis/exam/ExamIntegrationTest.java (1)

994-994: LGTM! Consider updating similar test methods

The addition of jenkinsJobPermissionsService parameter to enableMockingOfRequests is correct and aligns with the Jenkins permissions handling requirements.

Let's verify if other test methods need similar updates:

✅ Verification successful

Based on the search results, I can now generate the final response:

All test methods using enableMockingOfRequests have the correct parameters

The search results show that all test methods calling jenkinsRequestMockProvider.enableMockingOfRequests() are consistently using both required parameters (jenkinsServer and jenkinsJobPermissionsService). The implementation in JenkinsRequestMockProvider shows these parameters are essential for setting up the mock environment correctly. No updates are needed in other test methods.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other test methods that use jenkinsRequestMockProvider.enableMockingOfRequests
# but might be missing the jenkinsJobPermissionsService parameter

rg -A 2 "jenkinsRequestMockProvider\.enableMockingOfRequests\([^,\n]*\)" --type java

Length of output: 86


Script:

#!/bin/bash
# Let's try a broader search pattern to find all usages of enableMockingOfRequests
rg "enableMockingOfRequests" --type java -B 2 -A 2

Length of output: 55087


Script:

#!/bin/bash
# Let's search for the implementation of enableMockingOfRequests in JenkinsRequestMockProvider to understand its parameters
rg -A 10 "public void enableMockingOfRequests\(JenkinsServer jenkinsServer, JenkinsJobPermissionsService jenkinsJobPermissionsService\)" --type java

Length of output: 1742

src/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java (1)

274-274: Updated mocking configuration for Jenkins requests

The change adds jenkinsJobPermissionsService as a parameter to enableMockingOfRequests, which aligns with the updated mocking strategy for Jenkins integration tests in Spring Boot 3.4.0.

Let's verify that all Jenkins-related tests are still covered:

✅ Verification successful

Let me analyze the results and generate a final response.

Jenkins mocking configuration is consistently updated across test classes

The verification shows that all test classes using jenkinsRequestMockProvider.enableMockingOfRequests also have access to jenkinsJobPermissionsService, indicating that the mocking strategy update has been consistently applied across the test suite. The change in StudentExamIntegrationTest.java aligns with the broader test infrastructure updates.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check test coverage for Jenkins-related tests
# Look for other test classes that might need similar updates

# Search for Jenkins mocking setup in test files
rg -l "jenkinsRequestMockProvider.enableMockingOfRequests" --type java

# Search for Jenkins permissions service usage
rg -l "jenkinsJobPermissionsService" --type java

Length of output: 4276

src/main/java/de/tum/cit/aet/artemis/core/config/EurekaClientConfiguration.java (2)

Line range hint 54-64: Well-implemented method configuration

The restClientDiscoveryClientOptionalArgs method correctly configures the RestClientDiscoveryClientOptionalArgs, properly handles TLS properties, and follows best practices.


69-70: Method implementation is concise and correct

The restClientTransportClientFactories method appropriately returns a new instance of RestClientTransportClientFactories using the provided optional arguments.

src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (1)

1360-1360: Ensure jenkinsJobPermissionsService is properly initialized

As previously mentioned, jenkinsJobPermissionsService is being used without prior initialization or declaration. This may cause a NullPointerException. Please make sure to declare and initialize it before use.

@github-actions github-actions bot added buildagent Pull requests that affect the corresponding module communication Pull requests that affect the corresponding module exam Pull requests that affect the corresponding module exercise Pull requests that affect the corresponding module programming Pull requests that affect the corresponding module labels Nov 30, 2024
@SimonEntholzer SimonEntholzer temporarily deployed to artemis-test3.artemis.cit.tum.de November 30, 2024 09:41 — with GitHub Actions Inactive
Copy link
Contributor

@SimonEntholzer SimonEntholzer left a comment

Choose a reason for hiding this comment

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

Tested exercises, exam mode participation and assessment, communication and clicked through several other components. Everything seems to be working as before

@AjayvirS
Copy link
Contributor

Tested exercise submissions, plagiarism detection, tutorials and learning path generation. The latter seems to trigger the following 500 error on initial request. Does not seem to be related to the upgrade though:
image

Copy link

@HawKhiem HawKhiem left a comment

Choose a reason for hiding this comment

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

I tested on TS4 the followings: creating lectures, auto units generation, editing and appending units and attachments, communication functionalities like emojis, uploading images. creating competencies and editing them. Everything seems to work as before. I also tried adding new attachments and units to lectures and name them then try downloading them to see their names. The names of the downloaded attachments/units are not displayed as named. But this is covered in another PR #9899

@krusche krusche merged commit fef2028 into develop Dec 2, 2024
51 of 55 checks passed
@krusche krusche deleted the chore/update-spring branch December 2, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buildagent Pull requests that affect the corresponding module communication Pull requests that affect the corresponding module core Pull requests that affect the corresponding module documentation exam Pull requests that affect the corresponding module exercise Pull requests that affect the corresponding module programming Pull requests that affect the corresponding module ready for review server Pull requests that update Java code. (Added Automatically!) tests
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants