Development: Fix issues with server test flakiness

[ole-ve]

Motivation and Context

Some server tests are flaky after changing the test order by restructuring into modules.

Description

Following improvements:

• Clear the pending participant scores calculations between each test
• Enable the participant score scheduler service for each test
• Make assertions and mockito-verify's not depend on an absolut state
• Ignore garbage collector file gc.log.lock generated by JGit in each repository on zipping and deletion
• Add own test prefix for some LocalCILocalVC-tests that re-using the same prefix
• Increase timeout for some specific tests

Out-of-scope

• Find the issue within the LocalCILocalVC implementation that causes the first test of LocalVCLocalCIIntegrationTest to fail (no matter of the order)
• Override Object#equals() in each atlas entity/domain object to ensure that Mockito.verify does not depend on the entity/domain object being in memory (and not "unproxied" or whatever might happen in the background)

Summary by CodeRabbit

• New Features

  • Enhanced file zipping process to exclude specific files, improving efficiency.
  • Improved notification testing framework with new constants and verification logic for tutorial group management.

• Bug Fixes

  • Updated equality checks for expirationDate to ensure consistent behavior across date types.

• Tests

  • Streamlined test setup and teardown processes for various integration tests, enhancing clarity and reducing complexity.
  • Enhanced integration tests for student participation management in exams, ensuring accurate deletion of participations and submissions.

• Chores

  • Removed unused imports and unnecessary method calls in tests to simplify codebase.

[coderabbitai]

Actionable comments posted: 4

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 61988fa and f6d320c.

📒 Files selected for processing (1)

• src/test/java/de/tum/cit/aet/artemis/programming/localvcci/LocalVCSshIntegrationTest.java (7 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

src/test/java/de/tum/cit/aet/artemis/programming/localvcci/LocalVCSshIntegrationTest.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

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between f6d320c and d3cec06.

📒 Files selected for processing (1)

• src/test/java/de/tum/cit/aet/artemis/programming/localvcci/LocalVCSshIntegrationTest.java (7 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

src/test/java/de/tum/cit/aet/artemis/programming/localvcci/LocalVCSshIntegrationTest.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

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 1ed4d48 and 63b9708.

📒 Files selected for processing (6)

• src/test/java/de/tum/cit/aet/artemis/atlas/competency/CourseCompetencyIntegrationTest.java (0 hunks)
• src/test/java/de/tum/cit/aet/artemis/communication/notification/NotificationScheduleServiceTest.java (0 hunks)
• src/test/java/de/tum/cit/aet/artemis/communication/notification/SingleUserNotificationServiceTest.java (6 hunks)
• src/test/java/de/tum/cit/aet/artemis/communication/notifications/service/TutorialGroupNotificationServiceTest.java (0 hunks)
• src/test/java/de/tum/cit/aet/artemis/communication/service/EmailSummaryServiceTest.java (0 hunks)
• src/test/java/de/tum/cit/aet/artemis/core/MetricsIntegrationTest.java (0 hunks)

💤 Files with no reviewable changes (5)

• src/test/java/de/tum/cit/aet/artemis/atlas/competency/CourseCompetencyIntegrationTest.java
• src/test/java/de/tum/cit/aet/artemis/communication/notification/NotificationScheduleServiceTest.java
• src/test/java/de/tum/cit/aet/artemis/communication/notifications/service/TutorialGroupNotificationServiceTest.java
• src/test/java/de/tum/cit/aet/artemis/communication/service/EmailSummaryServiceTest.java
• src/test/java/de/tum/cit/aet/artemis/core/MetricsIntegrationTest.java

🧰 Additional context used

📓 Path-based instructions (1)

src/test/java/de/tum/cit/aet/artemis/communication/notification/SingleUserNotificationServiceTest.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

🔇 Additional comments (1)

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

137-142: Proper addition of @Captor annotations

The inclusion of @Captor annotations for appleNotificationCaptor and firebaseNotificationCaptor correctly sets up argument captors for verifying notifications in the tests. This enhances the test's ability to assert that the correct notifications are being sent.

[coderabbitai]

🧹 Nitpick (assertive)

Rename parameter title to text for clarity

In the filterRelevantNotifications method, the parameter title is compared with notification.getText(). To avoid confusion between notification.getTitle() and notification.getText(), consider renaming the parameter to text to accurately reflect its purpose.

Apply this diff to rename the parameter:

-private List<SingleUserNotification> filterRelevantNotifications(List<Notification> notifications, String title, User recipient) {
+private List<SingleUserNotification> filterRelevantNotifications(List<Notification> notifications, String text, User recipient) {
    return notifications.stream()
-        .filter(notification -> title.equals(notification.getText()) && recipient.getId().equals(notification.getRecipient().getId()))
+        .filter(notification -> text.equals(notification.getText()) && recipient.getId().equals(notification.getRecipient().getId()))
        .toList();
}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	private List<SingleUserNotification> filterRelevantNotifications(List<Notification> notifications, String title, User recipient) {
	return notifications.stream().filter(notification -> notification instanceof SingleUserNotification).map(notification -> (SingleUserNotification) notification)
	.filter(notification -> title.equals(notification.getText()) && recipient.getId().equals(notification.getRecipient().getId())).toList();
	private List<SingleUserNotification> filterRelevantNotifications(List<Notification> notifications, String text, User recipient) {
	return notifications.stream().filter(notification -> notification instanceof SingleUserNotification).map(notification -> (SingleUserNotification) notification)
	.filter(notification -> text.equals(notification.getText()) && recipient.getId().equals(notification.getRecipient().getId())).toList();

[cremertim]

Lgtm

[SimonEntholzer]

Code