Skip to content

Conversation

uglide
Copy link
Contributor

@uglide uglide commented Sep 12, 2025

Coverage is merged now and mvn verify target should be used to run all tests.

- Use test target only for running integration tests
- Use failsafe for running integration tests
- Clean up pom.xml
@uglide uglide requested review from ggivo, atakavci and Copilot and removed request for ggivo September 12, 2025 10:36
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR separates unit and integration tests by adding @tag annotations to categorize tests and updating the Maven build configuration to run them separately. The primary goal is to provide better test organization and enable different execution strategies for unit vs integration tests.

  • Tests are tagged with appropriate @tag annotations ("integration", "scenario", etc.)
  • Maven Surefire/Failsafe plugins are configured to run tagged and filename-based test categorization
  • JaCoCo coverage collection is enhanced to track unit and integration test coverage separately

Reviewed Changes

Copilot reviewed 106 out of 106 changed files in this pull request and generated 3 comments.

File Description
Various test files Added @tag("integration") annotations to classify integration tests
pom.xml Enhanced Maven configuration for separate unit/integration test execution and coverage collection
.github/workflows/ Updated CI workflows to use new test execution strategy
Makefile Updated commands to use verify instead of test phase

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.api.*;
Copy link

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

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

[nitpick] Using wildcard imports (.*) is generally discouraged as it can lead to namespace pollution and unclear dependencies. Consider importing specific classes instead.

Suggested change
import org.junit.jupiter.api.*;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

Copilot uses AI. Check for mistakes.

Comment on lines +335 to +339
<exclude>**/examples/*.java</exclude>
<exclude>**/scenario/*Test.java</exclude>
<!-- Exclude integration tests from unit-test phase -->
<exclude>**/*IntegrationTest.java</exclude>
<exclude>**/*IntegrationTests.java</exclude>
Copy link

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

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

The exclusion patterns for integration tests are duplicated between surefire and failsafe plugins. Consider extracting these patterns into properties to maintain consistency and reduce duplication.

Copilot uses AI. Check for mistakes.

uglide and others added 2 commits September 12, 2025 12:46
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link

github-actions bot commented Sep 12, 2025

Test Results

  273 files   - 2      1 errors  272 suites   - 3   10m 42s ⏱️ -12s
5 076 tests ±0  4 735 ✅ ±0  341 💤 ±0  0 ❌ ±0 
2 622 runs  ±0  2 622 ✅ ±0    0 💤 ±0  0 ❌ ±0 

For more details on these parsing errors, see this check.

Results for commit 7f86964. ± Comparison against base commit b04e64b.

♻️ This comment has been updated with latest results.

atakavci
atakavci previously approved these changes Sep 12, 2025
Copy link
Contributor

@atakavci atakavci left a comment

Choose a reason for hiding this comment

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

LGTM.
did you check why codeQl fails ? may be we should exclude failsafe-summary file.

Co-authored-by: atakavci <a_takavci@yahoo.com>
@uglide uglide merged commit 5c0cf87 into master Sep 12, 2025
10 checks passed
@uglide uglide deleted the im/separate_unit_and_integration_tests branch September 12, 2025 15:51
@ggivo ggivo added maintenance skip-changelog Ignore pull request from release note labels Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance skip-changelog Ignore pull request from release note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants