Skip to content

test: increase line coverage to 97% with unit and integration tests#211

Merged
rjayasinghe merged 5 commits into
mainfrom
test/increase-coverage
Jul 1, 2026
Merged

test: increase line coverage to 97% with unit and integration tests#211
rjayasinghe merged 5 commits into
mainfrom
test/increase-coverage

Conversation

@rjayasinghe

Copy link
Copy Markdown
Contributor

Summary

  • Adds 93 tests (60 unit / 33 integration) bringing line coverage from ~3 covered classes to 97% overall
  • MockServer-based HTTP tests run as integration tests via maven-failsafe-plugin (*IT.java); pure Mockito tests remain unit tests via surefire (*Test.java)
  • One bug fixed in RestClient.handleJsonResponse: empty response body no longer throws MismatchedInputException — returns {} instead

New test classes

Class Runner What it covers
RestClientIT Failsafe GET/POST/DELETE, status codes, non-JSON content type, empty body
AemManagementClientIT Failsafe SEMP v2 queue idempotency, DMQ, subscriptions, URL encoding, required attributes
AemValidationClientIT Failsafe Handshake payload (hostName, subaccountId inclusion/omission, error responses)
AemMessagingServiceTest Surefire skipManagement guards, DMQ pre-creation, emitTopicMessage prefix, validate one-shot guard, getMessageTopic (text/bytes/unknown), stop lifecycle
AemValidationOAuth2PropertySupplierTest Surefire isOAuth2Binding, URI validation, CredentialsView extraction
AemMessagingConnectionProviderTest Surefire Bearer token extraction, null/malformed header edge cases
Extended AemMessagingServiceConfigurationTest Surefire kind: aem and kind: advanced-event-mesh config paths

Production changes (test seams only — no behaviour change)

  • RestClient: package-private constructor accepting HttpDestination; empty body fix
  • AemManagementClient, AemValidationClient: package-private test constructors
  • AemMessagingService: package-private test constructor; AemValidationClient extracted to field instead of constructed inline in validate()
  • AemMessagingConnectionProvider: package-private test constructor; fetchToken/getToken made package-private

Test plan

  • mvn clean verify passes (60 unit + 33 integration, 0 failures)
  • JaCoCo coverage check passes (≥30% instruction threshold, ≤4 missed classes)
  • Only *IT classes appear in failsafe output; no *IT in surefire output

Adds 93 tests (60 unit via surefire, 33 integration via failsafe) covering
all production business logic that was previously untested.

New test classes:
- RestClientIT: HTTP behavior (GET/POST/DELETE, status codes, empty body)
- AemManagementClientIT: SEMP v2 logic (queue idempotency, subscriptions, URL encoding)
- AemValidationClientIT: handshake payload construction and host extraction
- AemMessagingServiceTest: skipManagement guards, DMQ pre-creation, topic prefix,
  validate one-shot guard, getMessageTopic for all message types, stop lifecycle
- AemValidationOAuth2PropertySupplierTest: credential extraction, URI validation
- AemMessagingConnectionProviderTest: bearer token extraction edge cases
- Extended AemMessagingServiceConfigurationTest: kind-based config paths

Production changes (test seams only, no behaviour change):
- RestClient: package-private constructor accepting HttpDestination; fix empty
  response body returning {} instead of throwing MismatchedInputException
- AemManagementClient, AemValidationClient: package-private test constructors
- AemMessagingService: package-private test constructor; extract AemValidationClient
  to field instead of constructing inline in validate()
- AemMessagingConnectionProvider: package-private test constructor; expose
  fetchToken/getToken as package-private

Build: failsafe plugin added; surefire excludes *IT.java; JaCoCo wired for
integration test coverage via prepare-agent-integration/report-integration.
@hyperspace-insights

Copy link
Copy Markdown
Contributor

Summary

The following content is AI-generated and provides a summary of the pull request:


Increase Line Coverage to 97% with Unit and Integration Tests

New Features

🧪 Test: Added 93 tests (60 unit / 33 integration) raising line coverage from ~3 covered classes to 97% overall. Integration tests use MockServer and run via maven-failsafe-plugin (*IT.java), while pure Mockito unit tests continue to run via Surefire (*Test.java). A bug fix in RestClient.handleJsonResponse prevents MismatchedInputException on empty response bodies by returning {} instead.

Changes

  • pom.xml: Configured maven-surefire-plugin to exclude *IT.java tests; added maven-failsafe-plugin for integration test execution; added JaCoCo prepare-agent-integration and report-integration executions.
  • RestClient.java: Added package-private constructor accepting HttpDestination (test seam); fixed handleJsonResponse to return an empty JSON object {} when the response body is null or blank, preventing MismatchedInputException.
  • AemManagementClient.java: Added @VisibleForTesting package-private constructor accepting HttpDestination, management URI, and VPN for test seam.
  • AemValidationClient.java: Added @VisibleForTesting package-private constructor accepting HttpDestination for test seam.
  • AemMessagingConnectionProvider.java: Added @VisibleForTesting package-private constructor accepting a ServiceBinding and Destination; exposed fetchToken() and getToken() as package-private for testing.
  • AemMessagingService.java: Refactored to extract AemValidationClient as a field (instead of constructing inline in validate()); added @VisibleForTesting package-private constructor; exposed getMessageTopic() as package-private.
  • RestClientIT.java (new): Integration tests covering GET/POST/DELETE, status codes, non-JSON content types, and empty body handling.
  • AemManagementClientIT.java (new): Integration tests covering SEMP v2 queue idempotency, DMQ, subscriptions, URL encoding, and required POST attributes.
  • AemValidationClientIT.java (new): Integration tests covering handshake payload construction (hostName, subaccountId inclusion/omission) and error responses.
  • AemMessagingServiceTest.java (new): Unit tests covering skipManagement guards, DMQ pre-creation, emitTopicMessage topic prefix, one-shot validation guard, getMessageTopic for text/bytes/unknown types, and stop lifecycle.
  • AemValidationOAuth2PropertySupplierTest.java (new): Unit tests for isOAuth2Binding, URI validation, and CredentialsView extraction.
  • AemMessagingConnectionProviderTest.java (new): Unit tests for Bearer token extraction and null/malformed header edge cases.
  • AemMessagingServiceConfigurationTest.java: Extended with tests for kind: aem and kind: advanced-event-mesh configuration paths, and kind-based config behavior with a single binding.

  • 🔄 Regenerate and Update Summary
  • ✏️ Insert as PR Description (deletes this comment)
  • 🗑️ Delete comment
PR Bot Information

Version: 1.22.5

  • Summary Prompt: Default Prompt
  • LLM: anthropic--claude-4.6-sonnet
  • Event Trigger: pull_request.opened
  • Output Template: Default Template
  • Correlation ID: f8d25c77-5e86-4a7d-b2fe-b6c7dbcbf88c
  • File Content Strategy: Full file content

@rjayasinghe rjayasinghe requested review from dimamost and t-bonk June 11, 2026 13:37

@hyperspace-insights hyperspace-insights Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The PR adds a solid test suite with good coverage. The main issues found are:

  1. Misleading test (testKindBasedConfigIsIgnoredWhenMultipleBindingsPresent) — the test name claims multi-binding isolation is tested, but the assertion actually verifies the opposite (single-binding positive case); this test does not prove the stated behaviour.
  2. Missing pre-condition assertions in AemValidationClientIT — two tests access recorded[0] without first asserting recorded.length == 1, risking misleading ArrayIndexOutOfBoundsException failures.
  3. Incomplete test assertion in AemMessagingServiceTestemitTopicMessage_triggers_validate_on_first_call verifies validate was called but not that the broker's emitTopicMessage was also invoked.
  4. Duplicated property-parsing logic across both AemMessagingService constructors — should be extracted to a shared private helper to avoid divergence.
  5. Inconsistent plugin version managementmaven-failsafe-plugin hardcodes version 3.5.6 while maven-surefire-plugin relies on the parent BOM, risking version drift between the two tightly-coupled plugins.
PR Bot Information

Version: 1.22.5

  • Event Trigger: pull_request.opened
  • File Content Strategy: Full file content
  • LLM: anthropic--claude-4.6-sonnet
  • Correlation ID: f8d25c77-5e86-4a7d-b2fe-b6c7dbcbf88c

Comment thread cds-feature-advanced-event-mesh/pom.xml Outdated
- Remove misleading test that claimed multi-binding isolation but only
  exercised single-binding positive case (already covered elsewhere).
- Add recorded.length assertions before recorded[0] access in
  AemValidationClientIT to avoid misleading AIOOBE failures.
- Verify broker.emitTopicMessage invocation in
  emitTopicMessage_triggers_validate_on_first_call.
- Extract duplicated connection-property parsing in AemMessagingService
  to a shared applyConnectionProperties helper.
- Move maven-failsafe-plugin version into parent pluginManagement so it
  stays aligned with maven-surefire-plugin.
- Pin byte-buddy-agent as an explicit test dependency and pass it via
  -javaagent in surefire/failsafe argLine so Mockito no longer
  self-attaches at runtime (preparing for future JDKs that disallow
  dynamic agent loading).
- Add a minimal log4j.properties under src/test/resources to provide
  a console appender for slf4j-reload4j, removing the "No appenders
  could be found" warnings.
@rjayasinghe rjayasinghe merged commit 048003a into main Jul 1, 2026
5 checks passed
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.

3 participants