Development: Isolated Build Agent Integration Tests
#10460
Conversation
github-actions
bot
added
tests
server
WalkthroughThe changes add a new Spring profile constant ( Changes
Sequence Diagram(s)sequenceDiagram
participant Test as BuildAgentIntegrationTest
participant Base as AbstractArtemisBuildAgentTest
participant Docker as DockerClientTestService
participant Client as DockerClient
Test->>Base: Initialize test environment
Base->>Docker: Call mockDockerClient()
Docker->>Client: Return mocked DockerClient with preconfigured commands
Test->>Base: Execute build agent workflow (e.g., processing build jobs)
Base->>Docker: Invoke methods (inspect image, pull image, etc.)
Docker-->>Base: Return simulated responses
Base->>Test: Return build results
sequenceDiagram
participant Config as CacheConfiguration
participant Hazelcast as HazelcastInstance
participant Profiles as ActiveProfiles
Config->>Profiles: Retrieve active profiles
alt Active profiles contain PROFILE_CORE or PROFILE_TEST_BUILDAGENT
Config->>Hazelcast: Do not join as lite member
else Active profiles include PROFILE_BUILDAGENT only
Config->>Hazelcast: Join cluster as lite member
end
Suggested labels
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/test/java/de/tum/cit/aet/artemis/exercise/util/ExerciseUtilService.java (1)
366-366: Consider addressing the TODO commentThere's an existing TODO comment about finding a generic solution for duplicated code in the find*ExerciseWithTitle methods (lines 375-458). While not directly related to your current changes, refactoring this duplicated code would improve maintainability.
A potential solution would be to create a generic method:
@NotNull public <T extends Exercise> T findExerciseWithTitle(Collection<Exercise> exercises, String title, Class<T> exerciseType) { return exercises.stream() .filter(e -> e.getTitle().equals(title)) .filter(exerciseType::isInstance) .map(exerciseType::cast) .findFirst() .orElseGet(() -> { fail("Could not find " + exerciseType.getSimpleName() + " exercise with title " + title); // This line is never reached due to the fail() call return null; }); }src/main/java/de/tum/cit/aet/artemis/core/config/CacheConfiguration.java (1)
264-268: Logical implementation of special case for test build agentsThe change correctly implements a special case for build agents running in test mode, preventing them from being configured as lite members. This aligns with the PR objective to create isolated build agent tests.
Consider adding a brief comment explaining why test build agents are excluded from the lite member configuration, such as:
// build agents should not hold partitions and only be a lite member + // except for test build agents which need to hold partitions for testing purposes if (!activeProfiles.contains(PROFILE_TEST_BUILDAGENT) && !activeProfiles.contains(PROFILE_CORE) && activeProfiles.contains(PROFILE_BUILDAGENT)) {src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisBuildAgentTest.java (1)
114-136: Exception handling in mockBuildJobGitService could be improved.The method silently ignores exceptions in the try-catch blocks, which may hide potential issues during test setup. While this might be intentional for test stability, it would be better to log these exceptions or use a more explicit handling approach.
Consider modifying the exception handling to log ignored exceptions:
try { doReturn(repository).when(buildJobGitService).cloneRepository(any(), any()); } catch (Exception ignored) { - // Exception silently ignored + // We still want to continue even if this mock setup fails + log.debug("Exception while setting up cloneRepository mock", ignored); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/test/resources/config/application-buildagent.ymlis excluded by!**/*.yml
📒 Files selected for processing (52)
src/main/java/de/tum/cit/aet/artemis/core/config/CacheConfiguration.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/assessment/util/ComplaintUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/assessment/util/GradingScaleUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/assessment/util/StudentScoreUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/atlas/competency/util/CompetencyProgressUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/atlas/competency/util/CompetencyUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/atlas/competency/util/PrerequisiteUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/atlas/competency/util/StandardizedCompetencyUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/atlas/learningpath/util/LearningPathUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/atlas/profile/util/LearnerProfileUtilService.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/atlas/science/util/ScienceUtilService.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentIntegrationTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/communication/util/ConversationUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/core/authorization/AuthorizationTestService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/core/organization/util/OrganizationUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/core/user/util/UserTestService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/core/user/util/UserUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/core/util/CourseTestService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/core/util/CourseUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/core/util/PageableSearchUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/core/util/RequestUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/exam/util/ExamUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/exercise/participation/util/ParticipationUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/exercise/team/TeamUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/exercise/util/ExerciseIntegrationTestService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/exercise/util/ExerciseUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/fileupload/util/FileUploadExerciseUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/fileupload/util/ZipFileTestUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/lecture/util/LectureUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/modeling/util/ModelingExerciseUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/plagiarism/PlagiarismUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/programming/ContinuousIntegrationTestService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/programming/icl/DockerClientTestService.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIIntegrationTest.java(5 hunks)src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java(12 hunks)src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCITestService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/programming/icl/ProgrammingExerciseLocalVCLocalCIIntegrationTest.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/programming/icl/util/SshSettingsTestService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/ConsistencyCheckTestService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/programming/util/GitUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseResultTestService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingSubmissionAndResultIntegrationTestService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingUtilTestService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/quiz/util/QuizExerciseUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisBuildAgentTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/text/util/TextExerciseUtilService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/tutorialgroup/util/TutorialGroupUtilService.java(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`src/test/java/**/*.java`: test_naming: descriptive; test_si...
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/fileupload/util/ZipFileTestUtilService.javasrc/test/java/de/tum/cit/aet/artemis/programming/ContinuousIntegrationTestService.javasrc/test/java/de/tum/cit/aet/artemis/atlas/competency/util/PrerequisiteUtilService.javasrc/test/java/de/tum/cit/aet/artemis/modeling/util/ModelingExerciseUtilService.javasrc/test/java/de/tum/cit/aet/artemis/exercise/team/TeamUtilService.javasrc/test/java/de/tum/cit/aet/artemis/assessment/util/StudentScoreUtilService.javasrc/test/java/de/tum/cit/aet/artemis/programming/util/GitUtilService.javasrc/test/java/de/tum/cit/aet/artemis/quiz/util/QuizExerciseUtilService.javasrc/test/java/de/tum/cit/aet/artemis/core/organization/util/OrganizationUtilService.javasrc/test/java/de/tum/cit/aet/artemis/programming/icl/util/SshSettingsTestService.javasrc/test/java/de/tum/cit/aet/artemis/core/util/PageableSearchUtilService.javasrc/test/java/de/tum/cit/aet/artemis/core/authorization/AuthorizationTestService.javasrc/test/java/de/tum/cit/aet/artemis/atlas/competency/util/StandardizedCompetencyUtilService.javasrc/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingUtilTestService.javasrc/test/java/de/tum/cit/aet/artemis/atlas/profile/util/LearnerProfileUtilService.javasrc/test/java/de/tum/cit/aet/artemis/core/user/util/UserTestService.javasrc/test/java/de/tum/cit/aet/artemis/core/util/RequestUtilService.javasrc/test/java/de/tum/cit/aet/artemis/atlas/competency/util/CompetencyUtilService.javasrc/test/java/de/tum/cit/aet/artemis/communication/util/ConversationUtilService.javasrc/test/java/de/tum/cit/aet/artemis/core/util/CourseUtilService.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/ConsistencyCheckTestService.javasrc/test/java/de/tum/cit/aet/artemis/exercise/util/ExerciseUtilService.javasrc/test/java/de/tum/cit/aet/artemis/plagiarism/PlagiarismUtilService.javasrc/test/java/de/tum/cit/aet/artemis/atlas/science/util/ScienceUtilService.javasrc/test/java/de/tum/cit/aet/artemis/fileupload/util/FileUploadExerciseUtilService.javasrc/test/java/de/tum/cit/aet/artemis/tutorialgroup/util/TutorialGroupUtilService.javasrc/test/java/de/tum/cit/aet/artemis/exercise/participation/util/ParticipationUtilService.javasrc/test/java/de/tum/cit/aet/artemis/atlas/competency/util/CompetencyProgressUtilService.javasrc/test/java/de/tum/cit/aet/artemis/atlas/learningpath/util/LearningPathUtilService.javasrc/test/java/de/tum/cit/aet/artemis/text/util/TextExerciseUtilService.javasrc/test/java/de/tum/cit/aet/artemis/programming/icl/ProgrammingExerciseLocalVCLocalCIIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.javasrc/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisBuildAgentTest.javasrc/test/java/de/tum/cit/aet/artemis/lecture/util/LectureUtilService.javasrc/test/java/de/tum/cit/aet/artemis/exercise/util/ExerciseIntegrationTestService.javasrc/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseUtilService.javasrc/test/java/de/tum/cit/aet/artemis/assessment/util/ComplaintUtilService.javasrc/test/java/de/tum/cit/aet/artemis/assessment/util/GradingScaleUtilService.javasrc/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/exam/util/ExamUtilService.javasrc/test/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingSubmissionAndResultIntegrationTestService.javasrc/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.javasrc/test/java/de/tum/cit/aet/artemis/core/user/util/UserUtilService.javasrc/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.javasrc/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseResultTestService.javasrc/test/java/de/tum/cit/aet/artemis/core/util/CourseTestService.javasrc/test/java/de/tum/cit/aet/artemis/programming/icl/DockerClientTestService.javasrc/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCITestService.java
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
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/Constants.javasrc/main/java/de/tum/cit/aet/artemis/core/config/CacheConfiguration.java
🧠 Learnings (2)
src/test/java/de/tum/cit/aet/artemis/atlas/competency/util/CompetencyProgressUtilService.java (1)
Learnt from: julian-christl
PR: ls1intum/Artemis#8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-11-12T12:51:58.050Z
Learning: Modifications to parameters in `competencyProgressUtilService.createCompetencyProgress` for debugging purposes are considered irrelevant to the test outcomes but helpful for clarity during debugging.
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.
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: client-style
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Analyse
- GitHub Check: Mend Security Check
🔇 Additional comments (139)
src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)
323-324: New Spring profile for build agent tests looks good.Adding a dedicated profile constant for build agent tests aligns well with the PR objective of introducing isolated integration tests for the build agent. The naming convention and placement are consistent with other profile constants in the file.
src/test/java/de/tum/cit/aet/artemis/core/util/RequestUtilService.java (3)
6-6: Import for SPRING_PROFILE_TEST looks good.This import properly enables the profile annotation to be used in this service class.
27-27: Import for Profile annotation looks good.This import is necessary for applying the profile restriction to the service class.
55-55: Profile annotation correctly restricts instantiation to test environment.Adding this profile annotation ensures the service is only instantiated during tests, which aligns with the purpose of isolating components for build agent integration tests.
src/test/java/de/tum/cit/aet/artemis/programming/util/GitUtilService.java (3)
4-4: Import for SPRING_PROFILE_TEST looks good.This import properly enables the profile annotation to be used in this service class.
23-23: Import for Profile annotation looks good.This import is necessary for applying the profile restriction to the service class.
38-38: Profile annotation correctly restricts instantiation to test environment.Adding this profile annotation ensures the service is only instantiated during tests, which aligns with the purpose of isolating components for build agent integration tests.
src/test/java/de/tum/cit/aet/artemis/assessment/util/StudentScoreUtilService.java (3)
3-3: Import for SPRING_PROFILE_TEST looks good.This import properly enables the profile annotation to be used in this service class.
6-6: Import for Profile annotation looks good.This import is necessary for applying the profile restriction to the service class.
19-19: Profile annotation correctly restricts instantiation to test environment.Adding this profile annotation ensures the service is only instantiated during tests, which aligns with the purpose of isolating components for build agent integration tests.
src/test/java/de/tum/cit/aet/artemis/exercise/team/TeamUtilService.java (3)
3-4: Appropriate import for test profile constant.The import for
SPRING_PROFILE_TESTfromtech.jhipster.config.JHipsterConstantsis correct and follows the project's convention for referencing Spring profile constants.
12-12: Import necessary for Profile annotation.The import for
org.springframework.context.annotation.Profileis required for the new annotation being added to the service class.
28-28: Properly restricting service to test environment.Adding the
@Profile(SPRING_PROFILE_TEST)annotation is appropriate for a test utility service. This ensures the service is only instantiated when running with the test profile, which helps minimize context restarts during testing and prevents this test-specific service from being available in production environments.src/test/java/de/tum/cit/aet/artemis/core/util/PageableSearchUtilService.java (3)
3-4: Appropriate import for test profile constant.The import for
SPRING_PROFILE_TESTfromtech.jhipster.config.JHipsterConstantsis correct and follows the project's convention for referencing Spring profile constants.
7-7: Import necessary for Profile annotation.The import for
org.springframework.context.annotation.Profileis required for the new annotation being added to the service class.
23-23: Properly restricting service to test environment.Adding the
@Profile(SPRING_PROFILE_TEST)annotation ensures this service is only instantiated when running with the test profile. This is appropriate for a test utility service that initializes database test data, as it prevents the service from being available in non-test environments and helps minimize context restarts during testing.src/test/java/de/tum/cit/aet/artemis/atlas/competency/util/CompetencyProgressUtilService.java (3)
3-4: Appropriate import for test profile constant.The import for
SPRING_PROFILE_TESTfromtech.jhipster.config.JHipsterConstantsis correct and follows the project's convention for referencing Spring profile constants.
6-6: Import necessary for Profile annotation.The import for
org.springframework.context.annotation.Profileis required for the new annotation being added to the service class.
18-18: Properly restricting service to test environment.Adding the
@Profile(SPRING_PROFILE_TEST)annotation ensures this service is only instantiated when running with the test profile. This is appropriate for a test utility service that initializes competency progress test data, preventing it from being available in non-test environments.I notice there's a previous learning that modifications to parameters in
createCompetencyProgressfor debugging purposes are considered irrelevant to test outcomes but helpful for clarity during debugging. The current change doesn't affect this behavior.src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingUtilTestService.java (3)
9-9: Appropriate import for test profile constant.The import for
SPRING_PROFILE_TESTfromtech.jhipster.config.JHipsterConstantsis correct and follows the project's convention for referencing Spring profile constants.
21-21: Import necessary for Profile annotation.The import for
org.springframework.context.annotation.Profileis required for the new annotation being added to the service class.
48-48: Properly restricting service to test environment.Adding the
@Profile(SPRING_PROFILE_TEST)annotation ensures this service is only instantiated when running with the test profile. This is appropriate for a test utility service that manages repository setup for programming exercises, as it prevents the service from being available in non-test environments and helps minimize context restarts during testing.This change aligns with the coding guidelines that specify test services should minimize context restarts and be properly isolated.
src/test/java/de/tum/cit/aet/artemis/quiz/util/QuizExerciseUtilService.java (3)
4-4: Added JHipster constant import for profile annotation.The import is needed to use the
SPRING_PROFILE_TESTconstant for the new profile annotation.
16-16: Added Spring Profile annotation import.The import is required for the newly added
@Profileannotation.
63-63: Added test profile annotation to restrict service instantiation.The
@Profile(SPRING_PROFILE_TEST)annotation ensures this utility service is only instantiated when the application is running in the test profile, preventing it from being loaded in production environments. This is part of a broader effort to properly isolate test services.src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java (3)
26-26: Added JHipster constant import for profile annotation.The import is needed to use the
SPRING_PROFILE_TESTconstant for the new profile annotation.
65-65: Added Spring Profile annotation import.The import is required for the newly added
@Profileannotation.
165-165: Added test profile annotation to restrict service instantiation.The
@Profile(SPRING_PROFILE_TEST)annotation ensures this test service is only instantiated when the application is running in the test profile. Given this class provides test infrastructure for scenarios with Jenkins + Gitlab, the annotation helps maintain proper separation between test environments.src/test/java/de/tum/cit/aet/artemis/programming/ContinuousIntegrationTestService.java (3)
6-6: Added JHipster constant import for profile annotation.The import is needed to use the
SPRING_PROFILE_TESTconstant for the new profile annotation.
18-18: Added Spring Profile annotation import.The import is required for the newly added
@Profileannotation.
35-35: Added test profile annotation to restrict service instantiation.The
@Profile(SPRING_PROFILE_TEST)annotation ensures this continuous integration test service is only instantiated when the application is running in the test profile. This aligns with the PR's objective of implementing isolated build agent integration tests, as it properly isolates the test services to the appropriate testing context.src/test/java/de/tum/cit/aet/artemis/tutorialgroup/util/TutorialGroupUtilService.java (1)
5-5: Good enhancement: Profile annotation added to restrict service to test environmentAdding
@Profile(SPRING_PROFILE_TEST)ensures this test utility service is only instantiated when running in the test profile, which aligns with the build agent integration test isolation objectives of this PR.Also applies to: 16-16, 40-40
src/test/java/de/tum/cit/aet/artemis/programming/service/ConsistencyCheckTestService.java (1)
4-4: Good enhancement: Profile annotation added to restrict service to test environmentAdding
@Profile(SPRING_PROFILE_TEST)ensures this test service is only instantiated when running in the test profile, which improves test isolation and aligns with the build agent integration test objectives.Also applies to: 12-12, 32-32
src/test/java/de/tum/cit/aet/artemis/assessment/util/GradingScaleUtilService.java (1)
4-4: Good enhancement: Profile annotation added to restrict service to test environmentAdding
@Profile(SPRING_PROFILE_TEST)ensures this grading scale utility service is only instantiated when running in the test profile, which helps with test isolation and supports the build agent integration testing objectives.Also applies to: 20-20, 35-35
src/test/java/de/tum/cit/aet/artemis/core/user/util/UserTestService.java (1)
6-6: Good enhancement: Profile annotation added to restrict service to test environmentAdding
@Profile(SPRING_PROFILE_TEST)ensures this large user test service is only instantiated when running in the test profile, which supports test isolation and is consistent with the other changes in this PR.Also applies to: 21-21, 72-72
src/test/java/de/tum/cit/aet/artemis/exercise/util/ExerciseIntegrationTestService.java (1)
4-4: Good addition of @Profile(SPRING_PROFILE_TEST)Adding the profile annotation ensures this test service is only instantiated during test runs, which helps with the isolation of integration tests and reduces unnecessary bean creation in non-test environments.
Also applies to: 10-10, 26-26
src/test/java/de/tum/cit/aet/artemis/assessment/util/ComplaintUtilService.java (1)
3-4: Appropriate restriction of test utility service to test profileAdding the @Profile annotation ensures this test utility service is only loaded when the Spring test profile is active, which aligns with best practices for test isolation.
Also applies to: 6-6, 28-28
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingSubmissionAndResultIntegrationTestService.java (1)
5-5: Good isolation of test service with profile annotationThe addition of @Profile(SPRING_PROFILE_TEST) ensures this integration test service is only active during test runs, which follows the PR objective of improving test isolation.
Also applies to: 12-12, 35-35
src/test/java/de/tum/cit/aet/artemis/atlas/profile/util/LearnerProfileUtilService.java (1)
3-4: Correctly configured profile restriction for test serviceAdding the @Profile annotation to this utility service ensures it's only loaded in the test context, which maintains clean separation between test and production code.
Also applies to: 9-9, 17-17
src/test/java/de/tum/cit/aet/artemis/plagiarism/PlagiarismUtilService.java (1)
3-4: Good addition of test profile annotationAdding
@Profile(SPRING_PROFILE_TEST)ensures this test utility service is only instantiated in the test environment, which is a good practice for managing test components.Also applies to: 41-41
src/test/java/de/tum/cit/aet/artemis/atlas/competency/util/StandardizedCompetencyUtilService.java (1)
3-4: Appropriate profile restriction for test utility serviceThe
@Profile(SPRING_PROFILE_TEST)annotation correctly restricts this utility service to the test environment, preventing unnecessary instantiation in production.Also applies to: 25-25
src/test/java/de/tum/cit/aet/artemis/exercise/util/ExerciseUtilService.java (1)
5-5: Good profile annotation for test utility serviceAdding
@Profile(SPRING_PROFILE_TEST)ensures this test utility service only runs in the test profile, which is appropriate for test utilities.Also applies to: 16-17, 67-67
src/test/java/de/tum/cit/aet/artemis/atlas/competency/util/PrerequisiteUtilService.java (1)
3-4: Well applied profile annotation for test serviceThe addition of
@Profile(SPRING_PROFILE_TEST)is consistent with the pattern applied to other test utility services in this PR, ensuring proper test environment isolation.Also applies to: 9-10, 20-20
src/test/java/de/tum/cit/aet/artemis/core/organization/util/OrganizationUtilService.java (1)
3-4: Well-structured profile restriction for test utility serviceThe addition of the
@Profile(SPRING_PROFILE_TEST)annotation properly restricts this utility service to test environments only. This is a good practice that prevents test utilities from being accidentally instantiated in production environments.Also applies to: 8-9, 18-18
src/test/java/de/tum/cit/aet/artemis/core/util/CourseTestService.java (3)
16-16: Appropriate import for test profile constant.The import
SPRING_PROFILE_TESTfromtech.jhipster.config.JHipsterConstantsis correctly added to support the profile annotation for the test service class.
49-49: Added necessary Profile annotation import.The import of
Profilefrom Spring's annotation package is correctly added to support the profile annotation for the test service class.
173-173: Appropriate profile restriction for test service.Adding
@Profile(SPRING_PROFILE_TEST)to the CourseTestService class ensures it's only loaded during testing, which helps with test isolation. This aligns with the PR objective of implementing isolated build agent integration tests.src/test/java/de/tum/cit/aet/artemis/fileupload/util/ZipFileTestUtilService.java (1)
4-4: Good addition of test profile annotationAdding the
@Profile(SPRING_PROFILE_TEST)annotation ensures this test utility service is only instantiated during tests, which aligns with the class's purpose as indicated in its JavaDoc "used only for testing purposes".Also applies to: 15-15, 23-23
src/test/java/de/tum/cit/aet/artemis/atlas/science/util/ScienceUtilService.java (1)
3-4: Appropriate use of test profile annotationThe addition of
@Profile(SPRING_PROFILE_TEST)to this test utility service ensures it's only instantiated during tests, which helps optimize the Spring context and isolate test components properly.Also applies to: 10-10, 18-18
src/test/java/de/tum/cit/aet/artemis/core/user/util/UserUtilService.java (1)
4-4: Good profile restriction for test utility serviceAdding the
@Profile(SPRING_PROFILE_TEST)annotation to this large utility service is an appropriate change that ensures it's only instantiated during tests. This aligns with the context minimization guideline in the project's test coding standards.Also applies to: 14-14, 35-35
src/test/java/de/tum/cit/aet/artemis/modeling/util/ModelingExerciseUtilService.java (1)
4-4: Appropriate test profile restrictionAdding the
@Profile(SPRING_PROFILE_TEST)annotation is consistent with the changes in other test utility services and ensures this service is only instantiated during test execution. This helps with context optimization and test isolation.Also applies to: 15-15, 56-56
src/test/java/de/tum/cit/aet/artemis/core/authorization/AuthorizationTestService.java (2)
5-5: Import added for test profile constantThe addition of the
SPRING_PROFILE_TESTconstant import prepares for the profile annotation to be added to the service class.
36-36: Added test profile restriction to service classGood addition of the
@Profile(SPRING_PROFILE_TEST)annotation to the service. This ensures the service is only instantiated during test execution, which aligns with the PR goal of isolating build agent integration tests. This is particularly important for test services that shouldn't be loaded in production environments.src/test/java/de/tum/cit/aet/artemis/exercise/participation/util/ParticipationUtilService.java (3)
9-9: Import added for test profile constantThe addition of the
SPRING_PROFILE_TESTconstant import prepares for the profile annotation to be added to the service class.
21-21: Import added for Profile annotationThe addition of the
Profileannotation import is necessary for adding the profile restriction to the service class.
82-82: Added test profile restriction to service classGood addition of the
@Profile(SPRING_PROFILE_TEST)annotation to the service. This ensures the service is only instantiated during test execution, which improves isolation for the build agent integration tests. This service contains many test utility methods and should only be available in test contexts.src/test/java/de/tum/cit/aet/artemis/atlas/learningpath/util/LearningPathUtilService.java (3)
3-4: Import added for test profile constantThe addition of the
SPRING_PROFILE_TESTconstant import prepares for the profile annotation to be added to the service class.
6-6: Import added for Profile annotationThe addition of the
Profileannotation import is necessary for adding the profile restriction to the service class.
19-19: Added test profile restriction to service classGood addition of the
@Profile(SPRING_PROFILE_TEST)annotation to the service. This ensures the service is only instantiated during test execution, aligning with the PR's goal of creating isolated build agent integration tests. This is a test utility service that should not be loaded in production environments.src/test/java/de/tum/cit/aet/artemis/communication/util/ConversationUtilService.java (3)
4-4: Import added for test profile constantThe addition of the
SPRING_PROFILE_TESTconstant import prepares for the profile annotation to be added to the service class.
16-16: Import added for Profile annotationThe addition of the
Profileannotation import is necessary for adding the profile restriction to the service class.
57-57: Added test profile restriction to service classGood addition of the
@Profile(SPRING_PROFILE_TEST)annotation to the service. This ensures the ConversationUtilService is only instantiated during test execution, which is critical for test isolation. This service initializes test data related to conversations and should only be available in test contexts.src/test/java/de/tum/cit/aet/artemis/programming/icl/util/SshSettingsTestService.java (1)
4-4: Good isolation of test service with Spring profile.The addition of
@Profile(SPRING_PROFILE_TEST)ensures this test service is only instantiated in test environments, which aligns with the PR objective of creating isolated integration tests for the build agent.Also applies to: 10-10, 26-26
src/test/java/de/tum/cit/aet/artemis/exam/util/ExamUtilService.java (1)
5-5: Appropriate profile constraint for test utilities.Adding the
@Profile(SPRING_PROFILE_TEST)annotation to this utility service helps ensure it's only available during testing, which is consistent with good testing practices and the overall objective of isolating test components.Also applies to: 16-16, 81-81
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseUtilService.java (1)
11-11: Well-defined test service scope with profile annotation.This change properly constrains the service to the test profile, consistent with the other test utility services. This systematic approach ensures all test components are properly isolated, which is critical for the integration tests for the build agent.
Also applies to: 26-26, 82-82
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java (1)
26-26: Proper Spring profile configuration added for test serviceThe addition of
@Profile(SPRING_PROFILE_TEST)ensures this service is only instantiated during testing. This follows Spring best practices for isolating test-specific beans and prevents unintended inclusion in non-test environments.Also applies to: 59-59, 137-137
src/test/java/de/tum/cit/aet/artemis/atlas/competency/util/CompetencyUtilService.java (1)
3-3: Proper Spring profile configuration added for utility serviceThe addition of
@Profile(SPRING_PROFILE_TEST)ensures this utility service is only instantiated during testing. This follows the same pattern applied to other test utilities in this PR, maintaining consistency across test components.Also applies to: 10-10, 34-34
src/test/java/de/tum/cit/aet/artemis/core/util/CourseUtilService.java (3)
5-5: Clean addition of constant import.Importing the
SPRING_PROFILE_TESTconstant from JHipster's configuration ensures consistent profile naming across the application.
20-20: Profile annotation import added correctly.Adding the Spring Profile import to support the profile annotation.
97-97: Good practice: Service properly annotated with test profile.Adding
@Profile(SPRING_PROFILE_TEST)to this utility service ensures it's only instantiated during test execution, which is appropriate since this service is specifically designed for creating test data. This change promotes better isolation between test and production code, and helps manage Spring context in integration tests.src/test/java/de/tum/cit/aet/artemis/text/util/TextExerciseUtilService.java (3)
4-4: Clean addition of constant import.Importing the
SPRING_PROFILE_TESTconstant from JHipster's configuration ensures consistent profile naming across the application.
13-13: Profile annotation import added correctly.Adding the Spring Profile import to support the profile annotation.
59-59: Good practice: Service properly annotated with test profile.Adding
@Profile(SPRING_PROFILE_TEST)to this utility service ensures it's only instantiated during test execution, which is appropriate since this service is specifically designed for creating text exercise test data. This change promotes better isolation between test and production code.src/test/java/de/tum/cit/aet/artemis/fileupload/util/FileUploadExerciseUtilService.java (3)
4-4: Clean addition of constant import.Importing the
SPRING_PROFILE_TESTconstant from JHipster's configuration ensures consistent profile naming across the application.
15-15: Profile annotation import added correctly.Adding the Spring Profile import to support the profile annotation.
42-42: Good practice: Service properly annotated with test profile.Adding
@Profile(SPRING_PROFILE_TEST)to this utility service ensures it's only instantiated during test execution. This is appropriate for a service that creates file upload exercise test data, and supports the PR's goal of creating isolated integration tests.src/test/java/de/tum/cit/aet/artemis/lecture/util/LectureUtilService.java (3)
4-4: Clean addition of constant import.Importing the
SPRING_PROFILE_TESTconstant from JHipster's configuration ensures consistent profile naming across the application.
16-16: Profile annotation import added correctly.Adding the Spring Profile import to support the profile annotation.
59-59: Good practice: Service properly annotated with test profile.Adding
@Profile(SPRING_PROFILE_TEST)to this utility service ensures it's only instantiated during test execution. This change is consistent with the PR's goal of creating isolated integration tests by ensuring test services are only activated with the appropriate profile.src/test/java/de/tum/cit/aet/artemis/programming/icl/ProgrammingExerciseLocalVCLocalCIIntegrationTest.java (5)
154-157: Good refactoring: Migrated Docker client mocking to dedicated service.The change to use
dockerClientTestServiceinstead oflocalVCLocalCITestServiceproperly separates responsibilities. The Docker-specific mocking logic now resides in a dedicated service, improving maintainability and adhering to the single responsibility principle.
159-159: Consistent use of dockerClientTestService.This change maintains consistency with the Docker client mocking refactoring.
163-166: Appropriate separation of testing responsibilities.Moving test results mocking to the dedicated Docker client service aligns with the refactoring pattern throughout the codebase.
253-258: Consistent method references in testImportProgrammingExercise.These changes maintain the consistent use of the dockerClientTestService for Docker-related operations in the import test.
262-265: Coherent test result mocking in import test.The test result mocking follows the same refactoring pattern as seen elsewhere in the file.
src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIIntegrationTest.java (5)
131-138: Appropriate refactoring of Docker client mocking.This change aligns with the overall refactoring strategy to move Docker client mocking logic to a specialized service. The initialization code now uses the dedicated
dockerClientTestServicefor all Docker-related operations, improving separation of concerns.
460-460: Consistent use of dockerClientTestService for test results.The test result mocking in
testFaultyResultFilesnow consistently uses the dedicated Docker client service.
470-470: Coherent refactoring in testLegacyResultFormat.The test mocking follows the same pattern established throughout the class.
509-509: Consistent mocking in testStaticCodeAnalysis.The static code analysis test properly uses the specialized Docker client service for mocking test results.
521-521: Coherent mocking in testEmptyResultFile.The empty result file test correctly uses the dedicated Docker client service, maintaining consistency with the refactoring pattern.
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseResultTestService.java (1)
17-17: Good practice: Added test profile annotation.Adding the
@Profile(SPRING_PROFILE_TEST)annotation ensures this service is only active during testing, which is a good practice for test-specific services. This prevents unnecessary instantiation in non-test environments and aligns with the Spring profile-based configuration approach.Also applies to: 27-27, 75-75
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java (2)
57-57: Good design: Introduced dedicated Docker client testing service.Adding the
dockerClientTestServiceas an autowired dependency properly supports the refactoring to centralize Docker client mocking logic. This follows the dependency injection pattern and improves testability.Also applies to: 96-97
206-206: Improved code maintainability through centralization.Replacing complex Docker client mocking setup with a single method call to
DockerClientTestService.mockDockerClient()simplifies the code and centralizes the Docker mocking logic. This change improves maintainability by keeping all Docker mocking details in one place.src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java (13)
127-128: Method reference updated to use dockerClientTestService.The code now correctly uses the extracted dockerClientTestService instead of the previous localVCLocalCITestService to mock the Docker client's behavior for retrieving container archives. This change aligns with the architectural refactoring to separate Docker mocking functionality into its own dedicated service.
154-154: Method reference updated to use dockerClientTestService.The call to mock image inspection now uses the specialized dockerClientTestService, which provides better separation of concerns compared to the previous implementation where this functionality was likely part of the localVCLocalCITestService.
187-188: Method reference updated to use dockerClientTestService.The mockInputStreamReturnedFromContainer call has been updated to use the new dockerClientTestService, maintaining the same parameters and functionality while benefiting from the separation of Docker-specific mocking code.
192-195: Helper method calls updated to use dockerClientTestService.The test result mapping creation and container mocking logic now uses the specialized dockerClientTestService, which improves code organization by properly separating Docker client mocking from other test utilities.
234-237: Method reference updated to use dockerClientTestService.Mock configuration for container archive operations has been migrated to use dockerClientTestService, maintaining the same test behavior while improving architectural organization.
265-268: Method reference updated to use dockerClientTestService.Docker client mocking for the template repository test has been updated to use the specialized dockerClientTestService, following the same pattern as other similar changes in this file.
318-321: Method reference updated to use dockerClientTestService.Test result mapping and container mocking for the auxiliary repository test now uses the dockerClientTestService, maintaining consistent usage of the new service throughout the test class.
348-352: Method reference updated to use dockerClientTestService.Docker mocking operations in the student assignment repository test have been updated to use dockerClientTestService, continuing the consistent usage of the specialized service.
430-433: Method reference updated to use dockerClientTestService.Docker client mocking in the submission limit test has been updated to use dockerClientTestService, maintaining the consistent refactoring pattern throughout the file.
677-680: Method reference updated to use dockerClientTestService.The exam mode test's Docker client mocking now uses dockerClientTestService, completing the consistent refactoring approach across all test cases in this file.
791-794: Method reference updated to use dockerClientTestService.The instructor exam test run's Docker client mocking has been updated to use dockerClientTestService, ensuring consistent usage of the specialized service throughout all test scenarios.
841-844: Method reference updated to use dockerClientTestService.Docker client mocking for the student practice repository test now uses dockerClientTestService, maintaining the consistent refactoring pattern established throughout the file.
1017-1020: Method reference updated to use dockerClientTestService.The build priority test's Docker client mocking now uses dockerClientTestService, completing the consistent application of the refactoring approach across all test methods.
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisBuildAgentTest.java (9)
1-67: Well-structured abstract base class for build agent testing.This abstract class provides an excellent foundation for build agent tests, with appropriate Spring annotations, test properties, and profile configurations. The class properly uses
@SpringBootTest,@ActiveProfiles, and@TestPropertySourceto set up the test environment, including the newly definedPROFILE_TEST_BUILDAGENT.
69-85: Appropriate dependency injection setup for testing.The class correctly autowires necessary dependencies and uses
@MockitoSpyBeanfor components that need partial mocking. This approach allows tests to control specific behaviors while preserving other functionality.
86-91: Well-defined test data paths for reuse.The static paths for test resources are properly defined for reuse across test methods, following good practice by keeping test data paths centralized.
92-96: Static Docker client mocking in BeforeAll.The
mockDockerClientmethod is correctly annotated with@BeforeAllto set up the Docker client mock once for all tests, which improves test efficiency.
97-112: Thorough mocking setup in BeforeEach method.The
mockServicesmethod properly sets up all required mocks before each test, ensuring a clean testing state. The method includes simulating a small delay for container starting, which helps test timeouts and other timing-dependent behaviors.
138-150: Comprehensive helper method for test queue item creation.The
createBaseBuildJobQueueItemForTriggermethod provides a good factory method for creating test data, with appropriate defaults for different test scenarios.
152-164: Dedicated helper for testing missing commit hash scenario.The
createBuildJobQueueItemWithNoCommitHashmethod appropriately creates a test item with null commit hashes, which is useful for testing error handling in the build agent.
166-178: Dedicated helper for testing timeout scenario.The
createBuildJobQueueItemForTimeoutmethod sets a very short timeout value (1 minute), which is appropriate for testing timeout handling without making tests wait too long.
180-194: Comprehensive helper for network configuration testing.The
createBuildJobQueueItemWithNetworkDisabledmethod correctly sets up a test item with network disabled and environment variables, which is useful for testing network isolation features.src/test/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentIntegrationTest.java (13)
1-47: Well-structured integration test class with appropriate annotations.The class properly extends
AbstractArtemisBuildAgentTestand includes comprehensive explanations via code comments for the@TestInstanceand@Executionannotations. The sequential execution mode is correctly used to avoid race conditions when testing asynchronous build agent operations.
48-67: Appropriate initialization of Hazelcast data structures.The class properly injects and initializes the necessary Hazelcast queues, maps, and topics for testing build agent functionality. These data structures match what would be used in the production environment.
68-77: Comprehensive setup in @BeforeAll method.The
initmethod correctly initializes all the required Hazelcast data structures that will be used across multiple test methods, following best practices for shared test resources.
79-84: Good test hygiene with queue clearing before each test.The
clearQueuesmethod ensures that each test starts with empty queues, preventing test interference and providing a clean testing environment for each test.
86-108: Comprehensive test for basic build agent functionality.The
testBuildAgentBasicFlowtest thoroughly checks that a build job is properly processed, focusing on key aspects like job queuing, processing status, build agent information updates, and result handling. The Awaitility library is appropriately used to handle asynchronous operations.
110-124: Thorough error handling test.The
testBuildAgentErrorFlowtest properly simulates a container start failure and verifies that the build job is correctly marked as failed, demonstrating robust error handling in the build agent.
126-144: Good timeout handling test.The
testBuildAgentTimeoutFlowtest simulates a long-running container operation and verifies that the build job is correctly marked as timed out when it exceeds its configured timeout.
146-157: Network configuration test.The
testBuildAgentDisableNetworktest verifies that builds with network disabled are processed correctly, an important feature for secure building environments.
159-177: Job cancellation test.The
testBuildAgentJobCancelledBeforeStartingtest verifies that a build job can be cancelled via the cancellation topic, ensuring that the job is marked as cancelled rather than failing with an error.
179-206: Image pulling behavior test.The
testBuildAgentPullImagetest simulates initial image not found errors followed by successful image inspection, verifying that the build agent properly retries image pulls and continues processing when the image becomes available.
208-238: Pause and resume functionality test.The
testPauseAndResumeBuildAgenttest verifies that a build agent can be paused, which prevents it from processing new jobs, and then resumed to continue processing. This is an important operational feature for maintenance scenarios.
240-269: Build agent pause behavior test.The
testPauseBuildAgentBehaviortest provides additional verification of the pause functionality, ensuring that jobs are returned to the queue when the build agent is paused during job processing.
271-282: Missing commit hash handling test.The
testBuildAgentNoCommitHashtest verifies that the build agent can process jobs even when no commit hash is provided, an important edge case for robust error handling.src/test/java/de/tum/cit/aet/artemis/programming/icl/DockerClientTestService.java (9)
1-62: Well-structured Docker client test service with appropriate annotations.The class is properly annotated with
@Serviceand@Profile, ensuring it's only active in test environments. It correctly imports the newPROFILE_TEST_BUILDAGENTconstant and follows standard Spring patterns for test utilities.
63-182: Comprehensive Docker client mocking setup.The
mockDockerClientmethod provides a thorough mocking of all required Docker client operations, including image inspection, container creation, execution, and file operations. This approach enables tests to simulate Docker operations without requiring an actual Docker daemon.
184-193: Streamlined test results mocking.The
mockTestResultsmethod provides a convenient way to mock test result retrieval from a Docker container, abstracting away the complexities of creating TarArchive streams and file content mapping.
195-204: Good flexibility with overloaded methods.The overloaded version of
mockTestResultsallows testing multiple test result directories, which enhances the flexibility of the testing approach.
206-219: Concise image inspection mocking.The
mockInspectImagemethod provides a simple way to mock Docker image inspection responses, focusing on the architectural aspects that are relevant for testing.
221-256: Robust container file mocking with regex support.The
mockInputStreamReturnedFromContainermethod provides flexible mocking of container file operations, with support for regex patterns to match container paths and multiple data maps for sequential calls. The method includes appropriate validation of input parameters.
258-287: Detailed TarArchive stream creation.The
createInputStreamForTarArchiveFromMapmethod properly creates a TarArchive stream from a data map, which is essential for simulating Docker container file operations in tests.
289-330: Comprehensive test results folder mapping.The
createMapFromTestResultsFoldermethod intelligently walks through a test results directory and creates a map of file paths to contents, handling XML files specially to preserve their content.
332-344: Flexible multi-folder test results mapping.The
createMapFromMultipleTestResultFoldersmethod extends the single-folder mapping to handle multiple directories, enhancing testing flexibility.src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCITestService.java (3)
8-9: Proper test profile import added for component isolation.Adding the import for SPRING_PROFILE_TEST constant is appropriate here, as it's used to annotate this test service class.
30-30: Required Profile annotation import added.The import for Spring's Profile annotation is necessary to support the @Profile annotation being added to the class.
58-58: Good application of test profile annotation.The addition of @Profile(SPRING_PROFILE_TEST) ensures this test service is only instantiated when the test profile is active. This aligns with the coding guideline to minimize context restarts and properly isolate test components.
This change supports the PR objective of building isolated integration tests for the build agent by ensuring test services are properly scoped to test environments.
BBesrour
changed the title
Development: Isolated Build Agent Integration TestsDevelopment: Isolated Build Agent Integration Tests
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisBuildAgentTest.java
Show resolved
Hide resolved
src/test/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentIntegrationTest.java
Show resolved
Hide resolved
src/test/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentIntegrationTest.java
Show resolved
Hide resolved
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisBuildAgentTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/test/java/de/tum/cit/aet/artemis/programming/icl/DockerClientTestService.java (3)
61-62: Consider removing@Servicein favor of a plain utility classThis class is used for mocking Docker client interactions in tests and does not appear to provide a domain-specific “service” role. Converting it to a plain utility or configuration class might avoid confusion and unnecessary bean instantiation overhead in the test context.
70-172: Extract smaller helper methods frommockDockerClient()The
mockDockerClient()method is quite large and covers many scenarios for container creation, image pulling, and other Docker actions. Splitting this into smaller, dedicated helper methods (or reusing the existing ones more extensively) would improve readability and maintainability.
349-379: Handle hidden or unexpected files in test result foldersIn
createMapFromTestResultsFolder, any file that is not an XML gets returned as"dummy-data". If hidden or temporary files are encountered (e.g., system files), they might get included inadvertently. Consider filtering out hidden file entries or clarifying the handling logic for non-XML files to prevent spurious data from showing up in the test results.src/test/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentIntegrationTest.java (2)
69-77: Inconsistent use ofthisqualifier.Some fields use
this.hazelcastInstancewhile others directly usehazelcastInstance. While functionally correct, maintaining consistency improves readability.void init() { processingJobs = this.hazelcastInstance.getMap("processingJobs"); buildJobQueue = this.hazelcastInstance.getQueue("buildJobQueue"); resultQueue = this.hazelcastInstance.getQueue("buildResultQueue"); buildAgentInformation = this.hazelcastInstance.getMap("buildAgentInformation"); - canceledBuildJobsTopic = hazelcastInstance.getTopic("canceledBuildJobsTopic"); - pauseBuildAgentTopic = hazelcastInstance.getTopic("pauseBuildAgentTopic"); - resumeBuildAgentTopic = hazelcastInstance.getTopic("resumeBuildAgentTopic"); + canceledBuildJobsTopic = this.hazelcastInstance.getTopic("canceledBuildJobsTopic"); + pauseBuildAgentTopic = this.hazelcastInstance.getTopic("pauseBuildAgentTopic"); + resumeBuildAgentTopic = this.hazelcastInstance.getTopic("resumeBuildAgentTopic"); }
261-291: Replace Thread.sleep with Awaitility for more reliable testing.Using
Thread.sleepcan lead to flaky tests if the system is under load. Consider using Awaitility like in other tests to wait for the condition rather than sleeping for a fixed time.buildJobQueue.add(queueItem); -Thread.sleep(100); - -assertThat(buildJobQueue.peek()).isNotNull(); -assertThat(buildJobQueue.peek().id()).isEqualTo(queueItem.id()); +await().until(() -> { + var queuePeek = buildJobQueue.peek(); + return queuePeek != null && queuePeek.id().equals(queueItem.id()); +}); resumeBuildAgentTopic.publish(buildAgentShortName);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/test/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentIntegrationTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/icl/DockerClientTestService.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisBuildAgentTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisBuildAgentTest.java
🧰 Additional context used
📓 Path-based instructions (1)
`src/test/java/**/*.java`: test_naming: descriptive; test_si...
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/buildagent/service/BuildAgentIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/icl/DockerClientTestService.java
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyse
- GitHub Check: server-style
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: server-tests
🔇 Additional comments (12)
src/test/java/de/tum/cit/aet/artemis/programming/icl/DockerClientTestService.java (1)
319-338: Ensure large content streaming is handled properlyWhile this method correctly writes TAR entries, be cautious when handling large files, as all data is held in memory via
ByteArrayOutputStream. If there's a chance of large test artifacts, consider streaming directly or applying an upper size limit to avoid high memory usage.src/test/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentIntegrationTest.java (11)
42-48: Effective test isolation with proper annotations.The use of
@TestInstance(TestInstance.Lifecycle.PER_CLASS)and@Execution(ExecutionMode.SAME_THREAD)is well-justified with clear comments explaining their purpose. This ensures reliable test execution by preventing race conditions and resource conflicts.
79-84: Test isolation ensured with proper queue clearing.The
clearQueuesmethod properly ensures test isolation by clearing all Hazelcast structures before each test, which prevents test interdependence.
86-108: Well-structured basic flow test with appropriate assertions.The basic flow test effectively validates the complete lifecycle of a build job using Awaitility for handling asynchronous operations. The assertions verify both job processing and build agent state updates.
110-153: Comprehensive concurrent builds test.This test effectively validates the build agent's ability to handle multiple concurrent builds with different container images and different test results. The comments explain the test setup clearly.
155-169: Effective error flow testing.The test properly simulates a container start failure and verifies the build agent correctly marks the job as failed.
171-189: Good timeout handling test.The test effectively simulates a build timeout scenario and verifies the correct build status is set.
191-202: Network disabled test is appropriate.This test verifies the build agent can successfully process jobs with network access disabled.
204-230: Job cancellation test is comprehensive.The test effectively verifies that jobs can be canceled through the Hazelcast topic, and that canceled jobs are properly marked with the CANCELLED status.
232-259: Image pulling behavior test is well-structured.The test effectively verifies the build agent's image pulling behavior, including the retry mechanism when an image is not found initially.
293-322: Paused build agent behavior test is thorough.The test effectively verifies the build agent's behavior when paused, ensuring jobs remain in the queue and aren't processed.
324-335: No commit hash scenario test is appropriate.This test verifies the build agent can successfully process jobs when no commit hash is provided.
Checklist
General
Changes affecting Programming Exercises
Motivation and Context
This PR introduces isolated integration tests for the build agent, ensuring they run exclusively with the buildAgent profile. These tests are not intended to be exhaustive but serve to verify that the main flow of the build agent remains functional.
Description
We had to add a
TESTprofile to all test services so that they aren't loaded in the our new test context as they are not needed. We also deleted the testapplication-buildagent.ymlso that we load the default one undersrcTestserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Performance Tests
Test Coverage
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tests
Chores