Skip to content

Further test expansion#1035

Open
Avery-Dunn wants to merge 7 commits into
avdunn/test-expansionfrom
avdunn/more-test-expansion
Open

Further test expansion#1035
Avery-Dunn wants to merge 7 commits into
avdunn/test-expansionfrom
avdunn/more-test-expansion

Conversation

@Avery-Dunn

Copy link
Copy Markdown
Contributor

This PR continues the systematic unit test expansion from Standardize and expand unit tests
, targeting the remaining coverage gaps identified in a second JaCoCo analysis. It adds ~150 new unit tests across 4 class groups covering response/error parsing, credentials, instance discovery, WS-Trust/federation, and miscellaneous gaps — bringing overall project coverage from 76.2% → 81.3% line and 63.0% → 67.7% branch.

Several pre-existing bugs were discovered and documented during testing (ErrorResponse.toJson double-start, RequestedClaim.toJson invalid write, OidcDiscoveryResponse.toJson missing issuer). Tests document current behavior without masking or fixing these bugs.

Test Results

Metric Before After Change
Total unit tests 522 669 +147 (both single-config and parameterized test methods)
Line coverage 76.2% 81.3% +5.1%
Branch coverage 63.0% 67.7% +4.7%
Combined (unit + IT) ~80% 86.1% line, 72% branch

Changes by Group

Group A: Response & Error Parsing (ResponseParsingTest.java — 47 tests total, 34 new)

Tests JSON-serializable response and claim classes: ErrorResponse, UserDiscoveryResponse, OidcDiscoveryResponse, RequestedClaimAdditionalInfo, RequestedClaim, ClientInfo, MsalServiceException, MsalServiceExceptionFactory.

Class Line Coverage
ErrorResponse 45.3% → 74.7%
UserDiscoveryResponse 43.9% → 100%
OidcDiscoveryResponse 0% → 100%
RequestedClaimAdditionalInfo 46.7% → 95.6%
RequestedClaim 21.7% → 56.5%
ClientInfo 69.2% → 100%
MsalServiceException 65.2% → 100%
MsalServiceExceptionFactory 66.7% → 95.2%

Includes private mockHttpResponse() and jsonContentTypeHeaders() helpers to reduce duplication in exception factory tests.

Bugs documented:

  • ErrorResponse.toJson() — calls writeStartObject() twice, causing IllegalStateException
  • RequestedClaim.toJson() — writes raw string in object context (invalid JSON)
  • OidcDiscoveryResponse.toJson() — omits issuer field

Group B: Token Acquisition & Credentials (ClientCredentialTest.java — 35 tests, +29 new; ClientCertificateTest.java — 13 tests, +6 new)

Tests ClientAssertion (all 3 provider types), ClientSecret, ClientCredentialFactory, and ClientCertificate.

Class Line Coverage
ClientAssertion 51.7% → 93.3%
ClientSecret 46.2% → 100%
ClientCredentialFactory 53.3% → 80%
ClientCertificate 63.6% → 80%

Covers: context-aware assertion providers, callable error paths, equals/hashCode contracts, factory validation, SHA-1/SHA-256 hashing, cert chain encoding, assertion generation.

Group C: Instance Discovery & Managed Identity Parsing (InstanceDiscoveryParsingTest.java — 11 new; ManagedIdentityParsingTest.java — 13 new)

Tests AadInstanceDiscoveryResponse, InstanceDiscoveryMetadataEntry, ManagedIdentityResponse, and ManagedIdentityErrorResponse JSON parsing.

Class Line Coverage
AadInstanceDiscoveryResponse 59.5% → 100%
InstanceDiscoveryMetadataEntry ~70% → 100%
ManagedIdentityResponse 63.6% → 100%
ManagedIdentityErrorResponse 61.8% → 100%

Covers: all fields, partial/unknown fields, round-trip serialization, nested error object format, string error format.

Group D: WS-Trust & Federation (WsTrustFederationTest.java — 19 new; WSTrustRequestTest.java — +6 new)

Tests Credential, BindingPolicy, IntegratedWindowsAuthorizationGrant, IdToken, WSTrustVersion, and WSTrustRequest message building.

Class Line Coverage
Credential 32.6% → 100%
BindingPolicy 56.2% → 100%
IntegratedWindowsAuthorizationGrant 0% → 100%
IdToken 61.2% → 100%
WSTrustVersion already 100%

Covers: JSON serialization/round-trip, all getters/setters, IWA grant parameters, namespace URIs, security header, XML escaping.

Miscellaneous Coverage Gaps (distributed to existing test classes)

Tests for remaining low-coverage utility/helper classes, placed in contextually appropriate homes:

Tests Destination Class Under Test
5 tests AcquireTokenSilentlyTest SilentRequestHelper (all CacheRefreshReason paths)
3 tests ParameterBuilderTest ParameterValidationUtils (null/empty set validation)
Class Line Coverage
SilentRequestHelper 62.5% → 100%
ParameterValidationUtils 69.2% → 92.3%

WSTrustResponseTest.java — Expanded (1 → 9 tests)

Existing test expanded to cover token parsing, error handling, token type detection, and XML extraction.

Class Line Coverage
WSTrustResponse 57.7% → 76.3%

Code Quality Improvements

  • Removed @TestInstance(PER_CLASS) from 4 test classes that had no shared mutable state (ClientCertificateTest, ClientCertificatePkcs12Test, SovereignCloudInstanceDiscoveryTest, WSTrustRequestTest)
  • Consolidated assertion imports — replaced individual import static lines with wildcard import static org.junit.jupiter.api.Assertions.* in AcquireTokenSilentlyTest
  • Replaced fully-qualified references — moved java.util.Base64, java.nio.charset.StandardCharsets, org.slf4j.Logger to proper imports
  • Extracted mock helpersmockHttpResponse() and jsonContentTypeHeaders() in ResponseParsingTest eliminate repetitive mock setup
  • Shared JSON test utilitiesTestHelper.parseJson(), TestHelper.writeToJson(), and TestHelper.JsonParser<T> interface (added in this branch) used by 4+ test files

@Avery-Dunn Avery-Dunn requested a review from a team as a code owner June 8, 2026 23:47
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.

2 participants