gnodet-bot commented on PR #13832: URL: https://github.com/apache/maven/pull/13832#issuecomment-6636352865
**Missing test for `ProjectBuildLogAppender.Sink` enrichment logic** The `Sink.accept()` method (the most subtle new code in the PR) has no unit test covering: 1. The ThreadLocal fallback: when `event.projectId()` is null, the enrichment reads `getProjectId()` from the ThreadLocal — confirm this resolves correctly when called from a non-logging thread. 2. The re-wrap decision: when the event already has the correct `projectId`/`mojoId`, confirm the original `DefaultLogEvent` instance is forwarded without copying (the `Objects.equals()` fix above is prerequisite). 3. The `reportCapture` forwarding path. `DefaultLoggerTest` and `DefaultLoggerManagerTest` are good starts but they don't exercise `ProjectBuildLogAppender`. A test similar to `DefaultLoggerTest` that installs a `ProjectBuildLogAppender` and asserts the enriched `LogEvent` is forwarded to the `BuildEventListener` with the correct `projectId` from the ThreadLocal would cover the critical dispatch path. _This review was generated by an AI agent, Hermès on behalf of @gnodet._ <!-- reviewer: gnodet-bot --> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
