Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jul 15, 2025

This PR updates the Logback smoke tests to use the new assertion framework introduced in #4141, making the tests more readable and maintainable while keeping changes minimal.

Changes Made

Framework Extensions

  • Added MessageAssert class: Provides fluent assertions for MessageData with methods like hasMessage(), hasSeverityLevel(), hasProperty(), etc.
  • Added ExceptionAssert class: Provides fluent assertions for ExceptionData with methods like hasExceptionType(), hasExceptionMessage(), hasSeverityLevel(), etc.
  • Extended TraceAssert class: Added support for message and exception assertions with hasMessageSatisfying(), hasExceptionSatisfying(), hasMessageCount(), hasExceptionCount() methods.

Test Updates

All Logback smoke tests have been converted from manual envelope parsing to the new fluent assertion API:

  • LogbackTest.java: Converted complex manual assertions to clean testing.waitAndAssertTrace() calls
  • LogbackDisabledTest.java: Simplified assertions for disabled logging behavior validation
  • LogbackLevelOffTest.java: Updated to use new assertion framework for level-off configuration testing

Before/After Comparison

Before (old manual approach):

List<Envelope> rdList = testing.mockedIngestion.waitForItems("RequestData", 1);
Envelope rdEnvelope = rdList.get(0);
String operationId = rdEnvelope.getTags().get("ai.operation.id");
List<Envelope> mdList = testing.mockedIngestion.waitForMessageItemsInRequest(3, operationId);

RequestData rd = (RequestData) ((Data<?>) rdEnvelope.getData()).getBaseData();
List<MessageData> logs = testing.mockedIngestion.getMessageDataInRequest(3);
logs.sort(Comparator.comparing(MessageData::getSeverityLevel));

assertThat(logs.get(0).getMessage()).isEqualTo("This is logback warn.");
assertThat(logs.get(0).getSeverityLevel()).isEqualTo(SeverityLevel.WARNING);
// ... many more manual assertions

After (new fluent API):

testing.waitAndAssertTrace(
    trace ->
        trace
            .hasRequestSatisying(
                request ->
                    request
                        .hasName("GET /Logback/test")
                        .hasSuccess(true)
                        .hasNoSampleRate())
            .hasMessageCount(3)
            .hasMessageSatisfying(
                message ->
                    message
                        .hasMessage("This is logback warn.")
                        .hasSeverityLevel(SeverityLevel.WARNING)
                        .hasProperty("SourceType", "Logger")
                        .hasProperty("LoggerName", "smoketestapp")));

Benefits

  • Cleaner Code: Reduced from 186 lines to 140 lines across test files
  • Better Readability: Fluent API makes test intentions clearer
  • Consistency: Aligned with new assertion framework pattern used in other tests
  • Maintainability: Easier to extend and modify assertions in the future

Testing

  • ✅ All Logback smoke tests pass (LogbackTest, LogbackDisabledTest, LogbackLevelOffTest)
  • ✅ Code formatting verified with ./gradlew spotlessApply
  • ✅ Framework extensions compile successfully

The assertion framework extensions (MessageAssert, ExceptionAssert) can be reused by other smoke tests that need to validate message and exception data.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI changed the title [WIP] Update the Logback smoke test to use the new assertion framework introduced in @microsoft/ApplicationInsights-Java/pull/4141 Keep changes minimal to make it easier to review. Extend the assertion framework if needed. Run the Logback smoke test after ... Update Logback smoke tests to use new assertion framework Jul 15, 2025
Copilot AI requested a review from trask July 15, 2025 03:37
@trask
Copy link
Member

trask commented Jul 15, 2025

@copilot preserve existing comments where possible

Copy link
Contributor Author

Copilot AI commented Jul 15, 2025

@copilot preserve existing comments where possible

Added the missing comments back to both LogbackDisabledTest.java and LogbackLevelOffTest.java to preserve the important context about span exception behavior. The comments // check that span exception is still captured and // this is not a logger-based exception are now included in the updated test files. (commit 5067e07)

Copilot finished work on behalf of trask July 15, 2025 05:21
@trask trask marked this pull request as ready for review July 15, 2025 15:49
@trask trask marked this pull request as draft July 15, 2025 17:57
@trask trask marked this pull request as ready for review July 15, 2025 17:57
@trask trask requested a review from xiang17 as a code owner July 15, 2025 17:57
@trask trask merged commit eb427e2 into main Jul 16, 2025
275 of 278 checks passed
@trask trask deleted the copilot/fix-a68cef08-b91f-4b67-9a52-a1af86138041 branch July 16, 2025 01:01
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.

4 participants