Skip to content

Improve test coverage from 51% to 89%#5

Open
dcoccia wants to merge 1 commit intoldgobi:mainfrom
dcoccia:improve-test-coverage
Open

Improve test coverage from 51% to 89%#5
dcoccia wants to merge 1 commit intoldgobi:mainfrom
dcoccia:improve-test-coverage

Conversation

@dcoccia
Copy link

@dcoccia dcoccia commented Apr 2, 2025

gft_icon Generated for GFT AI Impact Bot for the f6b7d76

Description: This pull request improves the test coverage of the supplier-api module from 51% to 89%. It introduces new unit tests for utility classes, services, and controllers, adds the JaCoCo Maven plugin for code coverage reporting, refines exception handling, and enhances validation logic for CNPJ values.

Summary:

  • File: supplier-api/pom.xml (modified)

    • Added the JaCoCo Maven plugin to the build configuration for code coverage reporting. This includes the prepare-agent and report goals to generate coverage reports during the test phase.
  • File: supplier-api/src/main/java/com/example/supplier/GlobalExceptionHandler.java (modified)

    • Refined exception handling by adding a specific handler for RuntimeException. This handler checks for specific error messages (e.g., "Supplier not found") and returns appropriate HTTP status codes (404 NOT FOUND or 500 INTERNAL SERVER ERROR).
  • File: supplier-api/src/main/java/com/example/supplier/util/CodigoUtil.java (modified)

    • Enhanced the isValidCNPJ method to include a check for CNPJ values that are all zeros, which are invalid. This improves validation robustness.
  • File: supplier-api/src/test/java/com/example/supplier/controller/SupplierControllerTest.java (added)

    • Added comprehensive unit tests for the SupplierController. These tests cover all major endpoints, including GET, POST, PUT, and DELETE operations, as well as edge cases like invalid CNPJ values and non-existent suppliers.
  • File: supplier-api/src/test/java/com/example/supplier/service/SupplierServiceTest.java (added)

    • Added unit tests for the SupplierService. These tests validate the service's behavior for creating, updating, retrieving, and deleting suppliers, including handling invalid CNPJ values and non-existent suppliers.
  • File: supplier-api/src/test/java/com/example/supplier/util/CodigoUtilTest.java (added)

    • Added unit tests for the CodigoUtil utility class. These tests cover various edge cases for the isValidCNPJ method, including short, long, negative, and all-zero CNPJ values.

Recommendation:

  1. Code Quality:

    • Consider adding comments to the isValidCNPJ method to explain the logic behind the validation checks, especially the new check for all-zero CNPJ values. This will improve code readability for future developers.
    • Ensure consistent formatting in the GlobalExceptionHandler class, as the indentation of the new handleRuntimeException method is slightly misaligned.
  2. Testing:

    • While the test coverage is significantly improved, consider adding integration tests to validate the interaction between the controller, service, and repository layers. This will ensure end-to-end functionality.
    • Add tests for edge cases where the database might be unavailable or return unexpected results.
  3. Documentation:

    • Update the project's documentation to include instructions for generating and interpreting JaCoCo coverage reports. This will help developers understand how to use the new plugin effectively.

Explanation of vulnerabilities:

  1. Potential Information Disclosure in Exception Handling:

    • The handleRuntimeException method in GlobalExceptionHandler prints the stack trace of exceptions using e.printStackTrace(). This could expose sensitive information in production environments.
      Suggested Fix: Replace e.printStackTrace() with a logging mechanism that writes to a secure log file. For example:
      logger.error("Runtime exception occurred", e);
  2. Validation Weakness in isValidCNPJ:

    • The isValidCNPJ method does not handle cases where the input CNPJ contains non-numeric characters. While this is unlikely due to the method's signature (long cnpj), it is worth considering additional validation for robustness.
      Suggested Fix: Add a check to ensure the CNPJ string contains only numeric characters before proceeding with validation.
  3. Test Property Source Configuration:

    • The TestPropertySource annotations in the test classes use an in-memory H2 database with default credentials (username=sa, password=). While this is acceptable for testing, ensure that these credentials are not accidentally used in production environments.
      Suggested Fix: Add a comment or configuration check to ensure these properties are strictly limited to test environments.

By addressing these recommendations and vulnerabilities, the pull request will achieve higher quality and security standards.

- Added comprehensive controller tests (96% coverage)
- Added service layer tests (100% coverage)
- Enhanced exception handling
- Fixed test configuration to avoid database conflicts
- Added separate database configurations for test isolation
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 2, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant