Refactor JUnitReport to reuse shared report helper implementations#8950
Merged
Conversation
9 tasks
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor JUnitReportEngine to eliminate duplicate code
Refactor JUnitReport to reuse shared report helper implementations
Jun 9, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request refactors the Microsoft.Testing.Extensions.JUnitReport extension to reuse the shared report helper implementations already used by other report extensions, reducing duplicated logic and drift risk across extensions.
Changes:
- Linked shared helpers into
Microsoft.Testing.Extensions.JUnitReport.csproj(filename templating/sanitization, TFM moniker resolution, and retry timeout constant). - Updated
JUnitReportEngineto call the shared helpers instead of maintaining private duplicate implementations. - Extended the cross-extension sanitizer parity unit tests to include JUnitReport.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/Microsoft.Testing.Extensions.UnitTests/ReportFileNameSanitizationConsistencyTests.cs | Adds Trx vs JUnit sanitizer parity validation via reflection to keep behavior aligned across extensions. |
| test/UnitTests/Microsoft.Testing.Extensions.UnitTests/Microsoft.Testing.Extensions.UnitTests.csproj | Adds a project reference to the JUnitReport extension so the parity test can access JUnitReportEngine. |
| src/Platform/Microsoft.Testing.Extensions.JUnitReport/Microsoft.Testing.Extensions.JUnitReport.csproj | Links shared helper source files from SharedExtensionHelpers into the JUnitReport project. |
| src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportEngine.cs | Replaces local helper logic with calls to shared helpers and uses the shared retry timeout constant in the error path. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 0
Member
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Contributor
Author
Merge conflicts are resolved in commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JUnitReportEnginehad local copies of report filename sanitization, target framework moniker resolution, template-based filename resolution, and the retry timeout already defined inSharedExtensionHelpers. This change removes that drift risk by wiring JUnitReport to the same shared implementations already used by sibling report extensions.Align JUnitReport with shared helper usage
ReportFileNameHelper,ReportFileNameSanitizer,TargetFrameworkMonikerHelper, andReportFileWriterHelperintoMicrosoft.Testing.Extensions.JUnitReport.csprojRemove duplicate logic from
JUnitReportEngineReportFileWriterHelper.FileWriteRetryTimeoutExtend parity coverage