Development: Enable ProgrammingExerciseLocalVcJenkinsIntegrationTest
#10600
Conversation
github-actions
bot
added
tests
programming
End-to-End (E2E) Test Results Summary
|
|||||||||||||||||||||||||||||||||||||||
github-actions
bot
added
server
End-to-End (E2E) Test Results Summary
|
|||||||||||||||||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
|||||||||||||||||||||||||||||||||||||||
…alvc-jenkins-integration-test
SimonEntholzer
changed the title
Development Enable ProgrammingExerciseLocalVcJenkinsIntegrationTestDevelopment: Enable ProgrammingExerciseLocalVcJenkinsIntegrationTest
|
""" WalkthroughThe pull request includes several minor adjustments. The documentation in the participation endpoint was clarified by fixing the parameter description. In addition, the repository service’s method for creating new repositories has been made public. Several test methods across different test files have been re-enabled by removing the Changes
Suggested labels
Suggested reviewers
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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 (1)
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java (1)
148-153: Commented out template repository code.The code for creating a template repository URI and template participation has been commented out. While this doesn't affect functionality since the template participation is set elsewhere, it would be better to either remove this commented code or add an explanatory comment about why it's being preserved.
Consider either:
- Removing the commented code completely if it's no longer needed
- Adding a comment explaining why this code is preserved for future reference
- // String templateRepuUri = String.format("%s/git/%s/%s-exercise.git", artemisVersionControlUrl, programmingExercise.getProjectKey(), repoName); - // var templateParticipation = new TemplateProgrammingExerciseParticipation(); - // templateParticipation.setRepositoryUri(templateRepuUri); - // templateParticipation.setExercise(programmingExercise); - // programmingExercise.setTemplateParticipation(templateParticipation);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/de/tum/cit/aet/artemis/exercise/web/ParticipationResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseRepositoryService.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseLocalVcJenkinsIntegrationTest.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/programming/icl/ProgrammingExerciseLocalVCLocalCIIntegrationTest.java(0 hunks)src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java(29 hunks)src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseUtilService.java(2 hunks)
💤 Files with no reviewable changes (1)
- src/test/java/de/tum/cit/aet/artemis/programming/icl/ProgrammingExerciseLocalVCLocalCIIntegrationTest.java
🧰 Additional context used
📓 Path-based instructions (2)
`src/main/java/**/*.java`: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,de...
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/exercise/web/ParticipationResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseRepositoryService.java
`src/test/java/**/*.java`: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_...
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/ProgrammingExerciseUtilService.javasrc/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.javasrc/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.javasrc/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseLocalVcJenkinsIntegrationTest.java
🧠 Learnings (1)
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java (1)
Learnt from: magaupp
PR: ls1intum/Artemis#9751
File: src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java:143-148
Timestamp: 2025-03-28T13:52:27.886Z
Learning: In `src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java`, the test package name assigned in `populateUnreleasedProgrammingExercise` does not need to adhere to naming conventions.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (22)
src/main/java/de/tum/cit/aet/artemis/exercise/web/ParticipationResource.java (1)
221-221: Documentation parameter description updated correctly.The JavaDoc parameter description has been properly updated to match the actual parameter name (
exerciseId) and its purpose, improving code documentation accuracy.src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseUtilService.java (1)
162-163: Dependency injection for RequestUtilService added.Adding this dependency injection follows the existing pattern in the class and will allow the service to utilize RequestUtilService functionality. This is a clean implementation that maintains the class structure.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseRepositoryService.java (1)
223-223: Method visibility increased from package-private to public.Increasing the visibility of
createRepositoriesForNewExerciseto public is appropriate as it supports the re-enabled tests in ProgrammingExerciseLocalVcJenkinsIntegrationTest. This change follows proper encapsulation principles while allowing necessary access for integration testing.src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java (1)
147-147: Swift test repository URI format updated.The test repository URI for Swift exercises now includes the "-test" suffix in the repository name, which provides better clarity and consistency with the actual repository naming conventions.
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java (13)
25-25: Good addition of reset import for test cleanupAdding the static import for Mockito's reset method is a good practice to help with proper test isolation.
141-141: Appropriate addition of repository dependencyThe import for TemplateProgrammingExerciseParticipationRepository is needed for the new dependency injection below.
324-325: Good dependency injection practiceAdding the TemplateProgrammingExerciseParticipationRepository follows the established pattern of dependency injection in this class, enhancing the functionality related to template participation management.
379-379: Improves test isolation with resetAdding the reset call on versionControlService ensures that any static mocks are cleared after tests run, which is essential for proper test isolation and preventing test interference.
478-478: Consistent mock behavior for commit hashThese additions ensure that the last commit hash is consistently mocked to return a predefined value during tests, making test behavior more deterministic and reliable.
Also applies to: 520-520, 529-529, 542-542, 551-551, 715-715, 745-745, 1285-1285
641-650: Enhanced test with proper mock sequenceThe test now properly handles multiple calls to the getLastCommitHash method, ensuring that the mock behavior is correctly set up for all invocations that occur during the test.
756-756: Cleanup of unused setup codeCommenting out the unused setupRepositoryMocks call simplifies the test and removes potentially unnecessary setup code, making the test more focused.
865-868: Proper mocking for auxiliary repository validationThis addition correctly mocks the validation of VCS repository URIs for auxiliary repositories, ensuring that tests involving auxiliary repositories function properly.
1082-1082: Proper mocking of VCS repository URIsThese additions ensure that the repository URI copying operations are properly mocked during tests, which is necessary for testing import and participation operations.
Also applies to: 1113-1113, 1150-1150, 1181-1181, 1225-1225, 1316-1316, 1324-1325
1393-1399: Improved assertion logic based on exercise modeThe conditional assertion logic correctly handles different expected initialization states based on the exercise mode (INDIVIDUAL vs TEAM), making the test more accurate for different scenarios.
1436-1442: Improved assertion logic for initialization stateSimilar to the previous improvement, this conditional assertion logic ensures tests accurately verify the correct initialization state based on exercise mode.
1766-1766: Minor code cleanupCommenting out an unnecessary repository save call improves test clarity and potentially performance by avoiding unneeded database operations.
2633-2634: Precise mock behavior for project existence checkThis change precisely defines the sequence of return values for the project existence check, which is more deterministic than the previous implementation.
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseLocalVcJenkinsIntegrationTest.java (5)
365-365: Appropriate documentation for disabled test.Good practice documenting that this test is failing specifically for TEAM functionality. This helps other developers understand why the test remains disabled and provides context for future improvements.
73-530: Re-enabling integration tests restores test coverage.The removal of
@Disabledannotations from these test methods is a positive change that restores test coverage for local version control with Jenkins integration after the GitLab removal. The tests follow proper JUnit 5 practices with appropriate annotations and naming conventions.The test methods follow a consistent pattern:
- Descriptive naming that clearly indicates what is being tested
- Proper use of parameterized tests where applicable
- Clear test setup and teardown
This change aligns well with the PR objective to re-enable tests that were previously disabled.
524-525: Keeping appropriate tests disabled.Keeping this test disabled is appropriate since the related code likely still requires updates. This ensures the test suite remains stable while work progresses.
563-565: Keeping appropriate tests disabled with clear TODO.Keeping this test disabled with a clear TODO is appropriate. This approach maintains test suite stability while acknowledging that this test should be addressed in future work.
610-612: Re-enabling build plan URL test.Re-enabling this test is appropriate since it tests important functionality for verifying build plan URL updates, which is essential for the Jenkins integration workflow.
End-to-End (E2E) Test Results Summary
|
|||||||||||||||||||||||||||||||||
tobias-lippert
left a comment
tobias-lippert
left a comment
There was a problem hiding this comment.
I have a few change requests and in general I'm a bit confused why all these tests now pass because the only change I recognize is outcommenting code.
...ava/de/tum/cit/aet/artemis/programming/ProgrammingExerciseLocalVcJenkinsIntegrationTest.java
Show resolved
Hide resolved
...ava/de/tum/cit/aet/artemis/programming/ProgrammingExerciseLocalVcJenkinsIntegrationTest.java
Show resolved
Hide resolved
...ava/de/tum/cit/aet/artemis/programming/ProgrammingExerciseLocalVcJenkinsIntegrationTest.java
Show resolved
Hide resolved
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java
Outdated
Show resolved
Hide resolved
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseUtilService.java
Outdated
Show resolved
Hide resolved
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
…alvc-jenkins-integration-test
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseLocalVcJenkinsIntegrationTest.java (2)
1-602: Good progress on re-enabling the integration tests.According to the AI summary, this PR re-enables multiple test methods that were previously disabled. This is a positive change that improves test coverage. I notice that some tests still remain disabled with comments explaining why (e.g., line 366 with the TEAM test failure).
For the tests that remain disabled without a specific comment explaining why they couldn't be fixed (like lines 159-168, 207-213, etc.), consider adding more detailed TODO comments similar to line 366. This would help future developers understand what specific issues need to be resolved to enable these tests.
65-67: Consider resetting static mocks in tearDown.The
tearDownmethod currently resets the Jenkins and Aeolus request mock providers, but I don't see any reset for the static mocks that might be used in the tests. According to the coding guidelines, static mocks should be properly managed to avoid test interference.Consider adding reset for any static mocks to ensure test isolation.
@AfterEach void tearDown() throws Exception { programmingExerciseTestService.tearDown(); jenkinsRequestMockProvider.reset(); aeolusRequestMockProvider.reset(); + // Reset any static mocks if they are used in the tests }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseLocalVcJenkinsIntegrationTest.java(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`src/test/java/**/*.java`: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_...
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/ProgrammingExerciseLocalVcJenkinsIntegrationTest.java
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: client-compilation
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: client-style
- GitHub Check: client-tests-selected
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (1)
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseLocalVcJenkinsIntegrationTest.java (1)
366-367: Helpful clarification about the disabled test.The added comment clearly indicates why the test remains disabled. This is good practice as it helps other developers understand the status of the test and prevents confusion about why it hasn't been re-enabled along with the others.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
…alvc-jenkins-integration-test
…alvc-jenkins-integration-test
…alvc-jenkins-integration-test
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
…alvc-jenkins-integration-test
krusche
left a comment
krusche
left a comment
There was a problem hiding this comment.
According to the Bamboo server test build plan, several server tests consistently fail. Please have a look again and make sure all tests adapted here pass reliably.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
github-project-automation
bot
moved this from Ready For Review
to Work In Progress
in Artemis Development
krusche
deleted the
chore/bugfix/enalbe-tests/Programming-localvc-jenkins-integration-test
branch
Checklist
General
Motivation and Context
For the gitlab removal may tests were disabled.
Description
Re-enables a large portion of the tests in the ProgrammingExerciseLocalVcJenkinsIntegrationTest.
Steps for Testing
Testserver 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
Manual Tests (run tests locally)
Summary by CodeRabbit