Skip to content

Extract shared report-provider registration helper (#9131) - #9152

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131
Jun 15, 2026
Merged

Extract shared report-provider registration helper (#9131)#9152
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9131.

Follow-up to #9134, which fixed the missing TestApplicationBuilder guard in CtrfReportExtensions but left the structural duplication (recommendation #2) unaddressed.

This extracts the repeated CompositeExtensionFactory construction, the TestApplicationBuilder guard, and the data-consumer / test-session-lifetime / command-line registration block — shared by the CTRF, JUnit, and HTML report extensions — into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers.

Each Add*ReportProvider method is now a single expression-bodied call that supplies only the generator factory and the per-extension resource strings. Behavior is unchanged (same guard, same registration order).

Validation

  • dotnet build of all three report extension projects: 0 warnings, 0 errors
  • Microsoft.Testing.Extensions.UnitTests (net8.0): all passed

Factor the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and data-consumer/session-lifetime/command-line registration shared by the CTRF, JUnit, and HTML report extensions into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers. Each Add*ReportProvider method is now a single expression-bodied call supplying only the generator factory and resource strings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 15, 2026 12:53

CopilotAI 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.

Pull request overview

This PR refactors the CTRF/JUnit/HTML report extensions in Microsoft.Testing.Platform to remove duplicated registration boilerplate by extracting the shared “guard + composite factory + registrations” logic into a single helper in SharedExtensionHelpers.

Changes:

  • Introduces ReportProviderRegistration.AddReportProvider<TGenerator> to centralize the TestApplicationBuilder guard and the standard registration sequence.
  • Updates CTRF/JUnit/HTML Add*ReportProvider extension methods to delegate to the shared helper.
  • Links the new shared helper into each of the three report extension projects.
Show a summary per file
FileDescription
src/Platform/SharedExtensionHelpers/ReportProviderRegistration.csAdds the shared helper that performs the guard and common registrations.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/Microsoft.Testing.Extensions.CtrfReport.csprojLinks the new shared helper into the CTRF extension project.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportExtensions.csRefactors AddCtrfReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/Microsoft.Testing.Extensions.JUnitReport.csprojLinks the new shared helper into the JUnit extension project.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportExtensions.csRefactors AddJUnitReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/Microsoft.Testing.Extensions.HtmlReport.csprojLinks the new shared helper into the HTML extension project.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/HtmlReportExtensions.csRefactors AddHtmlReportProvider to delegate to the shared helper.

Copilot's findings

  • Files reviewed: 7/7 changed files
  • Comments generated: 1

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary — PR #9152: Extract shared report-provider registration helper

Verdict: ✅ No blocking or major issues found. LGTM with one minor observation.

This is a clean, well-scoped refactoring that faithfully extracts the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and three-call registration block from the CTRF, JUnit, and HTML extension methods into a single generic helper. Behavior is identical to the code it replaces.


22-Dimension Analysis

#DimensionStatusNotes
1Algorithmic CorrectnessGuard, factory, and registration order are all preserved exactly
2Threading & ConcurrencyN/AConfiguration-time only; no concurrency
3Memory & Resource ManagementNo new disposables or allocations beyond the pre-existing lambda capture
4Public API SurfaceReportProviderRegistration is internal; all three public extension-method signatures are unchanged
5Backward CompatibilityNo public API changes
6Security & Trust BoundariesN/ANo security-sensitive paths
7Performance & AllocationcommandLine is still instantiated eagerly at the call site and captured in a single lambda — semantically identical to the old local-variable pattern
8Error Handling & ResilienceInvalidOperationException guard is preserved; consistent with codebase style of not adding null-argument guards for internal helpers
9Naming & ReadabilityReportProviderRegistration, AddReportProvider<TGenerator>, and invalidBuilderTypeErrorMessage are clear
10Code DuplicationThis is exactly what the PR addresses — three near-identical blocks collapsed into one
11Test Coveragei️No new unit tests for ReportProviderRegistration itself (guard path). This was a pre-existing gap; no regression introduced. The three Add*ReportProvider calls in the unit-test Program.cs exercise the happy path.
12Documentation & XML Commentsi️See inline comment — <see cref="TestApplicationBuilder"/> references an internal type. Works at compile time, minor cosmetic concern only.
13Cross-TFM CorrectnessNo TFM-conditional code; targets netstandard2.0 + net frameworks consistently
14Localization & Resource StringsNo new strings; existing ExtensionResources.InvalidTestApplicationBuilderType is passed through unchanged
15MSBuild / Project File Quality<Compile Include> is inserted at the same relative position in all three .csproj files, consistent with the existing Link="Helpers\*.cs" pattern
16Dependency ManagementNo new package references
17Pattern ConsistencyTrxReport and AzureDevOps have extra registrations (tools, OS-conditional branches, multiple composites) that correctly remain unreduced
18Scope DisciplineStrictly limited to the structural deduplication described in the PR
19TODO PolicyNo TODOs introduced
20Assertion Quality (tests)N/ANo new tests
21License HeadersNew file has correct Microsoft copyright header
22File OrganizationReportProviderRegistration.cs placed alongside sibling helpers in SharedExtensionHelpers/

One Pre-existing Gap (not introduced by this PR)

The builder is not TestApplicationBuilder guard is not unit-tested for any of the three affected extensions, either before or after this change. The refactoring is a good opportunity to track this as a follow-up: a single ReportProviderRegistrationTests class could cover the guard once (passing a mock ITestApplicationBuilder that is not a TestApplicationBuilder) rather than requiring three separate tests. Consider filing a tracking issue if desired.


Overall: The abstraction is well-designed, the generic constraint where TGenerator : class, IDataConsumer, ITestSessionLifetimeHandler is exactly right, and the InternalsVisibleTo grants that make TestApplicationBuilder accessible in all three consumer assemblies are already in place. No changes required.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 713 AIC · ⌖ 13 AIC ·

Comment threadsrc/Platform/SharedExtensionHelpers/ReportProviderRegistration.cs Outdated
…guard
Convert AddReportProvider's commandLine parameter to a Func factory so the
*ReportGeneratorCommandLine instance is only created after the
TestApplicationBuilder guard passes, restoring the original short-circuit
behavior. Also replace internal-type cref docs with prose.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink) merged commit 216ac8c into mainJun 15, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/dedup-report-extensions-9131 branch June 15, 2026 14:14
Sign up for freeto 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.

[duplicate-code] Duplicate Code: Report extension registration boilerplate repeated across Ctrf/JUnit/Html with inconsistent guard

3 participants

@Evangelink@JanKrivanek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Extract shared report-provider registration helper (#9131) by Evangelink · Pull Request #9152 · microsoft/testfx · GitHub
Skip to content

Extract shared report-provider registration helper (#9131) - #9152

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131
Jun 15, 2026
Merged

Extract shared report-provider registration helper (#9131)#9152
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9131.

Follow-up to #9134, which fixed the missing TestApplicationBuilder guard in CtrfReportExtensions but left the structural duplication (recommendation #2) unaddressed.

This extracts the repeated CompositeExtensionFactory construction, the TestApplicationBuilder guard, and the data-consumer / test-session-lifetime / command-line registration block — shared by the CTRF, JUnit, and HTML report extensions — into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers.

Each Add*ReportProvider method is now a single expression-bodied call that supplies only the generator factory and the per-extension resource strings. Behavior is unchanged (same guard, same registration order).

Validation

  • dotnet build of all three report extension projects: 0 warnings, 0 errors
  • Microsoft.Testing.Extensions.UnitTests (net8.0): all passed

Factor the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and data-consumer/session-lifetime/command-line registration shared by the CTRF, JUnit, and HTML report extensions into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers. Each Add*ReportProvider method is now a single expression-bodied call supplying only the generator factory and resource strings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 15, 2026 12:53

CopilotAI 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.

Pull request overview

This PR refactors the CTRF/JUnit/HTML report extensions in Microsoft.Testing.Platform to remove duplicated registration boilerplate by extracting the shared “guard + composite factory + registrations” logic into a single helper in SharedExtensionHelpers.

Changes:

  • Introduces ReportProviderRegistration.AddReportProvider<TGenerator> to centralize the TestApplicationBuilder guard and the standard registration sequence.
  • Updates CTRF/JUnit/HTML Add*ReportProvider extension methods to delegate to the shared helper.
  • Links the new shared helper into each of the three report extension projects.
Show a summary per file
FileDescription
src/Platform/SharedExtensionHelpers/ReportProviderRegistration.csAdds the shared helper that performs the guard and common registrations.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/Microsoft.Testing.Extensions.CtrfReport.csprojLinks the new shared helper into the CTRF extension project.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportExtensions.csRefactors AddCtrfReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/Microsoft.Testing.Extensions.JUnitReport.csprojLinks the new shared helper into the JUnit extension project.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportExtensions.csRefactors AddJUnitReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/Microsoft.Testing.Extensions.HtmlReport.csprojLinks the new shared helper into the HTML extension project.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/HtmlReportExtensions.csRefactors AddHtmlReportProvider to delegate to the shared helper.

Copilot's findings

  • Files reviewed: 7/7 changed files
  • Comments generated: 1

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary — PR #9152: Extract shared report-provider registration helper

Verdict: ✅ No blocking or major issues found. LGTM with one minor observation.

This is a clean, well-scoped refactoring that faithfully extracts the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and three-call registration block from the CTRF, JUnit, and HTML extension methods into a single generic helper. Behavior is identical to the code it replaces.


22-Dimension Analysis

#DimensionStatusNotes
1Algorithmic CorrectnessGuard, factory, and registration order are all preserved exactly
2Threading & ConcurrencyN/AConfiguration-time only; no concurrency
3Memory & Resource ManagementNo new disposables or allocations beyond the pre-existing lambda capture
4Public API SurfaceReportProviderRegistration is internal; all three public extension-method signatures are unchanged
5Backward CompatibilityNo public API changes
6Security & Trust BoundariesN/ANo security-sensitive paths
7Performance & AllocationcommandLine is still instantiated eagerly at the call site and captured in a single lambda — semantically identical to the old local-variable pattern
8Error Handling & ResilienceInvalidOperationException guard is preserved; consistent with codebase style of not adding null-argument guards for internal helpers
9Naming & ReadabilityReportProviderRegistration, AddReportProvider<TGenerator>, and invalidBuilderTypeErrorMessage are clear
10Code DuplicationThis is exactly what the PR addresses — three near-identical blocks collapsed into one
11Test Coveragei️No new unit tests for ReportProviderRegistration itself (guard path). This was a pre-existing gap; no regression introduced. The three Add*ReportProvider calls in the unit-test Program.cs exercise the happy path.
12Documentation & XML Commentsi️See inline comment — <see cref="TestApplicationBuilder"/> references an internal type. Works at compile time, minor cosmetic concern only.
13Cross-TFM CorrectnessNo TFM-conditional code; targets netstandard2.0 + net frameworks consistently
14Localization & Resource StringsNo new strings; existing ExtensionResources.InvalidTestApplicationBuilderType is passed through unchanged
15MSBuild / Project File Quality<Compile Include> is inserted at the same relative position in all three .csproj files, consistent with the existing Link="Helpers\*.cs" pattern
16Dependency ManagementNo new package references
17Pattern ConsistencyTrxReport and AzureDevOps have extra registrations (tools, OS-conditional branches, multiple composites) that correctly remain unreduced
18Scope DisciplineStrictly limited to the structural deduplication described in the PR
19TODO PolicyNo TODOs introduced
20Assertion Quality (tests)N/ANo new tests
21License HeadersNew file has correct Microsoft copyright header
22File OrganizationReportProviderRegistration.cs placed alongside sibling helpers in SharedExtensionHelpers/

One Pre-existing Gap (not introduced by this PR)

The builder is not TestApplicationBuilder guard is not unit-tested for any of the three affected extensions, either before or after this change. The refactoring is a good opportunity to track this as a follow-up: a single ReportProviderRegistrationTests class could cover the guard once (passing a mock ITestApplicationBuilder that is not a TestApplicationBuilder) rather than requiring three separate tests. Consider filing a tracking issue if desired.


Overall: The abstraction is well-designed, the generic constraint where TGenerator : class, IDataConsumer, ITestSessionLifetimeHandler is exactly right, and the InternalsVisibleTo grants that make TestApplicationBuilder accessible in all three consumer assemblies are already in place. No changes required.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 713 AIC · ⌖ 13 AIC ·

Comment threadsrc/Platform/SharedExtensionHelpers/ReportProviderRegistration.cs Outdated
…guard
Convert AddReportProvider's commandLine parameter to a Func factory so the
*ReportGeneratorCommandLine instance is only created after the
TestApplicationBuilder guard passes, restoring the original short-circuit
behavior. Also replace internal-type cref docs with prose.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink) merged commit 216ac8c into mainJun 15, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/dedup-report-extensions-9131 branch June 15, 2026 14:14
Sign up for freeto 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.

[duplicate-code] Duplicate Code: Report extension registration boilerplate repeated across Ctrf/JUnit/Html with inconsistent guard

3 participants

@Evangelink@JanKrivanek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Extract shared report-provider registration helper (#9131) by Evangelink · Pull Request #9152 · microsoft/testfx · GitHub
Skip to content

Extract shared report-provider registration helper (#9131) - #9152

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131
Jun 15, 2026
Merged

Extract shared report-provider registration helper (#9131)#9152
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9131.

Follow-up to #9134, which fixed the missing TestApplicationBuilder guard in CtrfReportExtensions but left the structural duplication (recommendation #2) unaddressed.

This extracts the repeated CompositeExtensionFactory construction, the TestApplicationBuilder guard, and the data-consumer / test-session-lifetime / command-line registration block — shared by the CTRF, JUnit, and HTML report extensions — into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers.

Each Add*ReportProvider method is now a single expression-bodied call that supplies only the generator factory and the per-extension resource strings. Behavior is unchanged (same guard, same registration order).

Validation

  • dotnet build of all three report extension projects: 0 warnings, 0 errors
  • Microsoft.Testing.Extensions.UnitTests (net8.0): all passed

Factor the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and data-consumer/session-lifetime/command-line registration shared by the CTRF, JUnit, and HTML report extensions into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers. Each Add*ReportProvider method is now a single expression-bodied call supplying only the generator factory and resource strings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 15, 2026 12:53

CopilotAI 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.

Pull request overview

This PR refactors the CTRF/JUnit/HTML report extensions in Microsoft.Testing.Platform to remove duplicated registration boilerplate by extracting the shared “guard + composite factory + registrations” logic into a single helper in SharedExtensionHelpers.

Changes:

  • Introduces ReportProviderRegistration.AddReportProvider<TGenerator> to centralize the TestApplicationBuilder guard and the standard registration sequence.
  • Updates CTRF/JUnit/HTML Add*ReportProvider extension methods to delegate to the shared helper.
  • Links the new shared helper into each of the three report extension projects.
Show a summary per file
FileDescription
src/Platform/SharedExtensionHelpers/ReportProviderRegistration.csAdds the shared helper that performs the guard and common registrations.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/Microsoft.Testing.Extensions.CtrfReport.csprojLinks the new shared helper into the CTRF extension project.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportExtensions.csRefactors AddCtrfReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/Microsoft.Testing.Extensions.JUnitReport.csprojLinks the new shared helper into the JUnit extension project.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportExtensions.csRefactors AddJUnitReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/Microsoft.Testing.Extensions.HtmlReport.csprojLinks the new shared helper into the HTML extension project.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/HtmlReportExtensions.csRefactors AddHtmlReportProvider to delegate to the shared helper.

Copilot's findings

  • Files reviewed: 7/7 changed files
  • Comments generated: 1

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary — PR #9152: Extract shared report-provider registration helper

Verdict: ✅ No blocking or major issues found. LGTM with one minor observation.

This is a clean, well-scoped refactoring that faithfully extracts the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and three-call registration block from the CTRF, JUnit, and HTML extension methods into a single generic helper. Behavior is identical to the code it replaces.


22-Dimension Analysis

#DimensionStatusNotes
1Algorithmic CorrectnessGuard, factory, and registration order are all preserved exactly
2Threading & ConcurrencyN/AConfiguration-time only; no concurrency
3Memory & Resource ManagementNo new disposables or allocations beyond the pre-existing lambda capture
4Public API SurfaceReportProviderRegistration is internal; all three public extension-method signatures are unchanged
5Backward CompatibilityNo public API changes
6Security & Trust BoundariesN/ANo security-sensitive paths
7Performance & AllocationcommandLine is still instantiated eagerly at the call site and captured in a single lambda — semantically identical to the old local-variable pattern
8Error Handling & ResilienceInvalidOperationException guard is preserved; consistent with codebase style of not adding null-argument guards for internal helpers
9Naming & ReadabilityReportProviderRegistration, AddReportProvider<TGenerator>, and invalidBuilderTypeErrorMessage are clear
10Code DuplicationThis is exactly what the PR addresses — three near-identical blocks collapsed into one
11Test Coveragei️No new unit tests for ReportProviderRegistration itself (guard path). This was a pre-existing gap; no regression introduced. The three Add*ReportProvider calls in the unit-test Program.cs exercise the happy path.
12Documentation & XML Commentsi️See inline comment — <see cref="TestApplicationBuilder"/> references an internal type. Works at compile time, minor cosmetic concern only.
13Cross-TFM CorrectnessNo TFM-conditional code; targets netstandard2.0 + net frameworks consistently
14Localization & Resource StringsNo new strings; existing ExtensionResources.InvalidTestApplicationBuilderType is passed through unchanged
15MSBuild / Project File Quality<Compile Include> is inserted at the same relative position in all three .csproj files, consistent with the existing Link="Helpers\*.cs" pattern
16Dependency ManagementNo new package references
17Pattern ConsistencyTrxReport and AzureDevOps have extra registrations (tools, OS-conditional branches, multiple composites) that correctly remain unreduced
18Scope DisciplineStrictly limited to the structural deduplication described in the PR
19TODO PolicyNo TODOs introduced
20Assertion Quality (tests)N/ANo new tests
21License HeadersNew file has correct Microsoft copyright header
22File OrganizationReportProviderRegistration.cs placed alongside sibling helpers in SharedExtensionHelpers/

One Pre-existing Gap (not introduced by this PR)

The builder is not TestApplicationBuilder guard is not unit-tested for any of the three affected extensions, either before or after this change. The refactoring is a good opportunity to track this as a follow-up: a single ReportProviderRegistrationTests class could cover the guard once (passing a mock ITestApplicationBuilder that is not a TestApplicationBuilder) rather than requiring three separate tests. Consider filing a tracking issue if desired.


Overall: The abstraction is well-designed, the generic constraint where TGenerator : class, IDataConsumer, ITestSessionLifetimeHandler is exactly right, and the InternalsVisibleTo grants that make TestApplicationBuilder accessible in all three consumer assemblies are already in place. No changes required.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 713 AIC · ⌖ 13 AIC ·

Comment threadsrc/Platform/SharedExtensionHelpers/ReportProviderRegistration.cs Outdated
…guard
Convert AddReportProvider's commandLine parameter to a Func factory so the
*ReportGeneratorCommandLine instance is only created after the
TestApplicationBuilder guard passes, restoring the original short-circuit
behavior. Also replace internal-type cref docs with prose.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink) merged commit 216ac8c into mainJun 15, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/dedup-report-extensions-9131 branch June 15, 2026 14:14
Sign up for freeto 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.

[duplicate-code] Duplicate Code: Report extension registration boilerplate repeated across Ctrf/JUnit/Html with inconsistent guard

3 participants

@Evangelink@JanKrivanek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Extract shared report-provider registration helper (#9131) by Evangelink · Pull Request #9152 · microsoft/testfx · GitHub
Skip to content

Extract shared report-provider registration helper (#9131) - #9152

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131
Jun 15, 2026
Merged

Extract shared report-provider registration helper (#9131)#9152
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9131.

Follow-up to #9134, which fixed the missing TestApplicationBuilder guard in CtrfReportExtensions but left the structural duplication (recommendation #2) unaddressed.

This extracts the repeated CompositeExtensionFactory construction, the TestApplicationBuilder guard, and the data-consumer / test-session-lifetime / command-line registration block — shared by the CTRF, JUnit, and HTML report extensions — into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers.

Each Add*ReportProvider method is now a single expression-bodied call that supplies only the generator factory and the per-extension resource strings. Behavior is unchanged (same guard, same registration order).

Validation

  • dotnet build of all three report extension projects: 0 warnings, 0 errors
  • Microsoft.Testing.Extensions.UnitTests (net8.0): all passed

Factor the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and data-consumer/session-lifetime/command-line registration shared by the CTRF, JUnit, and HTML report extensions into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers. Each Add*ReportProvider method is now a single expression-bodied call supplying only the generator factory and resource strings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 15, 2026 12:53

CopilotAI 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.

Pull request overview

This PR refactors the CTRF/JUnit/HTML report extensions in Microsoft.Testing.Platform to remove duplicated registration boilerplate by extracting the shared “guard + composite factory + registrations” logic into a single helper in SharedExtensionHelpers.

Changes:

  • Introduces ReportProviderRegistration.AddReportProvider<TGenerator> to centralize the TestApplicationBuilder guard and the standard registration sequence.
  • Updates CTRF/JUnit/HTML Add*ReportProvider extension methods to delegate to the shared helper.
  • Links the new shared helper into each of the three report extension projects.
Show a summary per file
FileDescription
src/Platform/SharedExtensionHelpers/ReportProviderRegistration.csAdds the shared helper that performs the guard and common registrations.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/Microsoft.Testing.Extensions.CtrfReport.csprojLinks the new shared helper into the CTRF extension project.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportExtensions.csRefactors AddCtrfReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/Microsoft.Testing.Extensions.JUnitReport.csprojLinks the new shared helper into the JUnit extension project.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportExtensions.csRefactors AddJUnitReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/Microsoft.Testing.Extensions.HtmlReport.csprojLinks the new shared helper into the HTML extension project.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/HtmlReportExtensions.csRefactors AddHtmlReportProvider to delegate to the shared helper.

Copilot's findings

  • Files reviewed: 7/7 changed files
  • Comments generated: 1

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary — PR #9152: Extract shared report-provider registration helper

Verdict: ✅ No blocking or major issues found. LGTM with one minor observation.

This is a clean, well-scoped refactoring that faithfully extracts the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and three-call registration block from the CTRF, JUnit, and HTML extension methods into a single generic helper. Behavior is identical to the code it replaces.


22-Dimension Analysis

#DimensionStatusNotes
1Algorithmic CorrectnessGuard, factory, and registration order are all preserved exactly
2Threading & ConcurrencyN/AConfiguration-time only; no concurrency
3Memory & Resource ManagementNo new disposables or allocations beyond the pre-existing lambda capture
4Public API SurfaceReportProviderRegistration is internal; all three public extension-method signatures are unchanged
5Backward CompatibilityNo public API changes
6Security & Trust BoundariesN/ANo security-sensitive paths
7Performance & AllocationcommandLine is still instantiated eagerly at the call site and captured in a single lambda — semantically identical to the old local-variable pattern
8Error Handling & ResilienceInvalidOperationException guard is preserved; consistent with codebase style of not adding null-argument guards for internal helpers
9Naming & ReadabilityReportProviderRegistration, AddReportProvider<TGenerator>, and invalidBuilderTypeErrorMessage are clear
10Code DuplicationThis is exactly what the PR addresses — three near-identical blocks collapsed into one
11Test Coveragei️No new unit tests for ReportProviderRegistration itself (guard path). This was a pre-existing gap; no regression introduced. The three Add*ReportProvider calls in the unit-test Program.cs exercise the happy path.
12Documentation & XML Commentsi️See inline comment — <see cref="TestApplicationBuilder"/> references an internal type. Works at compile time, minor cosmetic concern only.
13Cross-TFM CorrectnessNo TFM-conditional code; targets netstandard2.0 + net frameworks consistently
14Localization & Resource StringsNo new strings; existing ExtensionResources.InvalidTestApplicationBuilderType is passed through unchanged
15MSBuild / Project File Quality<Compile Include> is inserted at the same relative position in all three .csproj files, consistent with the existing Link="Helpers\*.cs" pattern
16Dependency ManagementNo new package references
17Pattern ConsistencyTrxReport and AzureDevOps have extra registrations (tools, OS-conditional branches, multiple composites) that correctly remain unreduced
18Scope DisciplineStrictly limited to the structural deduplication described in the PR
19TODO PolicyNo TODOs introduced
20Assertion Quality (tests)N/ANo new tests
21License HeadersNew file has correct Microsoft copyright header
22File OrganizationReportProviderRegistration.cs placed alongside sibling helpers in SharedExtensionHelpers/

One Pre-existing Gap (not introduced by this PR)

The builder is not TestApplicationBuilder guard is not unit-tested for any of the three affected extensions, either before or after this change. The refactoring is a good opportunity to track this as a follow-up: a single ReportProviderRegistrationTests class could cover the guard once (passing a mock ITestApplicationBuilder that is not a TestApplicationBuilder) rather than requiring three separate tests. Consider filing a tracking issue if desired.


Overall: The abstraction is well-designed, the generic constraint where TGenerator : class, IDataConsumer, ITestSessionLifetimeHandler is exactly right, and the InternalsVisibleTo grants that make TestApplicationBuilder accessible in all three consumer assemblies are already in place. No changes required.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 713 AIC · ⌖ 13 AIC ·

Comment threadsrc/Platform/SharedExtensionHelpers/ReportProviderRegistration.cs Outdated
…guard
Convert AddReportProvider's commandLine parameter to a Func factory so the
*ReportGeneratorCommandLine instance is only created after the
TestApplicationBuilder guard passes, restoring the original short-circuit
behavior. Also replace internal-type cref docs with prose.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink) merged commit 216ac8c into mainJun 15, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/dedup-report-extensions-9131 branch June 15, 2026 14:14
Sign up for freeto 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.

[duplicate-code] Duplicate Code: Report extension registration boilerplate repeated across Ctrf/JUnit/Html with inconsistent guard

3 participants

@Evangelink@JanKrivanek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Extract shared report-provider registration helper (#9131) by Evangelink · Pull Request #9152 · microsoft/testfx · GitHub
Skip to content

Extract shared report-provider registration helper (#9131) - #9152

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131
Jun 15, 2026
Merged

Extract shared report-provider registration helper (#9131)#9152
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9131.

Follow-up to #9134, which fixed the missing TestApplicationBuilder guard in CtrfReportExtensions but left the structural duplication (recommendation #2) unaddressed.

This extracts the repeated CompositeExtensionFactory construction, the TestApplicationBuilder guard, and the data-consumer / test-session-lifetime / command-line registration block — shared by the CTRF, JUnit, and HTML report extensions — into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers.

Each Add*ReportProvider method is now a single expression-bodied call that supplies only the generator factory and the per-extension resource strings. Behavior is unchanged (same guard, same registration order).

Validation

  • dotnet build of all three report extension projects: 0 warnings, 0 errors
  • Microsoft.Testing.Extensions.UnitTests (net8.0): all passed

Factor the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and data-consumer/session-lifetime/command-line registration shared by the CTRF, JUnit, and HTML report extensions into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers. Each Add*ReportProvider method is now a single expression-bodied call supplying only the generator factory and resource strings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 15, 2026 12:53

CopilotAI 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.

Pull request overview

This PR refactors the CTRF/JUnit/HTML report extensions in Microsoft.Testing.Platform to remove duplicated registration boilerplate by extracting the shared “guard + composite factory + registrations” logic into a single helper in SharedExtensionHelpers.

Changes:

  • Introduces ReportProviderRegistration.AddReportProvider<TGenerator> to centralize the TestApplicationBuilder guard and the standard registration sequence.
  • Updates CTRF/JUnit/HTML Add*ReportProvider extension methods to delegate to the shared helper.
  • Links the new shared helper into each of the three report extension projects.
Show a summary per file
FileDescription
src/Platform/SharedExtensionHelpers/ReportProviderRegistration.csAdds the shared helper that performs the guard and common registrations.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/Microsoft.Testing.Extensions.CtrfReport.csprojLinks the new shared helper into the CTRF extension project.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportExtensions.csRefactors AddCtrfReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/Microsoft.Testing.Extensions.JUnitReport.csprojLinks the new shared helper into the JUnit extension project.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportExtensions.csRefactors AddJUnitReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/Microsoft.Testing.Extensions.HtmlReport.csprojLinks the new shared helper into the HTML extension project.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/HtmlReportExtensions.csRefactors AddHtmlReportProvider to delegate to the shared helper.

Copilot's findings

  • Files reviewed: 7/7 changed files
  • Comments generated: 1

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary — PR #9152: Extract shared report-provider registration helper

Verdict: ✅ No blocking or major issues found. LGTM with one minor observation.

This is a clean, well-scoped refactoring that faithfully extracts the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and three-call registration block from the CTRF, JUnit, and HTML extension methods into a single generic helper. Behavior is identical to the code it replaces.


22-Dimension Analysis

#DimensionStatusNotes
1Algorithmic CorrectnessGuard, factory, and registration order are all preserved exactly
2Threading & ConcurrencyN/AConfiguration-time only; no concurrency
3Memory & Resource ManagementNo new disposables or allocations beyond the pre-existing lambda capture
4Public API SurfaceReportProviderRegistration is internal; all three public extension-method signatures are unchanged
5Backward CompatibilityNo public API changes
6Security & Trust BoundariesN/ANo security-sensitive paths
7Performance & AllocationcommandLine is still instantiated eagerly at the call site and captured in a single lambda — semantically identical to the old local-variable pattern
8Error Handling & ResilienceInvalidOperationException guard is preserved; consistent with codebase style of not adding null-argument guards for internal helpers
9Naming & ReadabilityReportProviderRegistration, AddReportProvider<TGenerator>, and invalidBuilderTypeErrorMessage are clear
10Code DuplicationThis is exactly what the PR addresses — three near-identical blocks collapsed into one
11Test Coveragei️No new unit tests for ReportProviderRegistration itself (guard path). This was a pre-existing gap; no regression introduced. The three Add*ReportProvider calls in the unit-test Program.cs exercise the happy path.
12Documentation & XML Commentsi️See inline comment — <see cref="TestApplicationBuilder"/> references an internal type. Works at compile time, minor cosmetic concern only.
13Cross-TFM CorrectnessNo TFM-conditional code; targets netstandard2.0 + net frameworks consistently
14Localization & Resource StringsNo new strings; existing ExtensionResources.InvalidTestApplicationBuilderType is passed through unchanged
15MSBuild / Project File Quality<Compile Include> is inserted at the same relative position in all three .csproj files, consistent with the existing Link="Helpers\*.cs" pattern
16Dependency ManagementNo new package references
17Pattern ConsistencyTrxReport and AzureDevOps have extra registrations (tools, OS-conditional branches, multiple composites) that correctly remain unreduced
18Scope DisciplineStrictly limited to the structural deduplication described in the PR
19TODO PolicyNo TODOs introduced
20Assertion Quality (tests)N/ANo new tests
21License HeadersNew file has correct Microsoft copyright header
22File OrganizationReportProviderRegistration.cs placed alongside sibling helpers in SharedExtensionHelpers/

One Pre-existing Gap (not introduced by this PR)

The builder is not TestApplicationBuilder guard is not unit-tested for any of the three affected extensions, either before or after this change. The refactoring is a good opportunity to track this as a follow-up: a single ReportProviderRegistrationTests class could cover the guard once (passing a mock ITestApplicationBuilder that is not a TestApplicationBuilder) rather than requiring three separate tests. Consider filing a tracking issue if desired.


Overall: The abstraction is well-designed, the generic constraint where TGenerator : class, IDataConsumer, ITestSessionLifetimeHandler is exactly right, and the InternalsVisibleTo grants that make TestApplicationBuilder accessible in all three consumer assemblies are already in place. No changes required.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 713 AIC · ⌖ 13 AIC ·

Comment threadsrc/Platform/SharedExtensionHelpers/ReportProviderRegistration.cs Outdated
…guard
Convert AddReportProvider's commandLine parameter to a Func factory so the
*ReportGeneratorCommandLine instance is only created after the
TestApplicationBuilder guard passes, restoring the original short-circuit
behavior. Also replace internal-type cref docs with prose.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink) merged commit 216ac8c into mainJun 15, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/dedup-report-extensions-9131 branch June 15, 2026 14:14
Sign up for freeto 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.

[duplicate-code] Duplicate Code: Report extension registration boilerplate repeated across Ctrf/JUnit/Html with inconsistent guard

3 participants

@Evangelink@JanKrivanek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Extract shared report-provider registration helper (#9131) by Evangelink · Pull Request #9152 · microsoft/testfx · GitHub
Skip to content

Extract shared report-provider registration helper (#9131) - #9152

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131
Jun 15, 2026
Merged

Extract shared report-provider registration helper (#9131)#9152
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9131.

Follow-up to #9134, which fixed the missing TestApplicationBuilder guard in CtrfReportExtensions but left the structural duplication (recommendation #2) unaddressed.

This extracts the repeated CompositeExtensionFactory construction, the TestApplicationBuilder guard, and the data-consumer / test-session-lifetime / command-line registration block — shared by the CTRF, JUnit, and HTML report extensions — into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers.

Each Add*ReportProvider method is now a single expression-bodied call that supplies only the generator factory and the per-extension resource strings. Behavior is unchanged (same guard, same registration order).

Validation

  • dotnet build of all three report extension projects: 0 warnings, 0 errors
  • Microsoft.Testing.Extensions.UnitTests (net8.0): all passed

Factor the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and data-consumer/session-lifetime/command-line registration shared by the CTRF, JUnit, and HTML report extensions into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers. Each Add*ReportProvider method is now a single expression-bodied call supplying only the generator factory and resource strings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 15, 2026 12:53

CopilotAI 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.

Pull request overview

This PR refactors the CTRF/JUnit/HTML report extensions in Microsoft.Testing.Platform to remove duplicated registration boilerplate by extracting the shared “guard + composite factory + registrations” logic into a single helper in SharedExtensionHelpers.

Changes:

  • Introduces ReportProviderRegistration.AddReportProvider<TGenerator> to centralize the TestApplicationBuilder guard and the standard registration sequence.
  • Updates CTRF/JUnit/HTML Add*ReportProvider extension methods to delegate to the shared helper.
  • Links the new shared helper into each of the three report extension projects.
Show a summary per file
FileDescription
src/Platform/SharedExtensionHelpers/ReportProviderRegistration.csAdds the shared helper that performs the guard and common registrations.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/Microsoft.Testing.Extensions.CtrfReport.csprojLinks the new shared helper into the CTRF extension project.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportExtensions.csRefactors AddCtrfReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/Microsoft.Testing.Extensions.JUnitReport.csprojLinks the new shared helper into the JUnit extension project.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportExtensions.csRefactors AddJUnitReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/Microsoft.Testing.Extensions.HtmlReport.csprojLinks the new shared helper into the HTML extension project.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/HtmlReportExtensions.csRefactors AddHtmlReportProvider to delegate to the shared helper.

Copilot's findings

  • Files reviewed: 7/7 changed files
  • Comments generated: 1

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary — PR #9152: Extract shared report-provider registration helper

Verdict: ✅ No blocking or major issues found. LGTM with one minor observation.

This is a clean, well-scoped refactoring that faithfully extracts the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and three-call registration block from the CTRF, JUnit, and HTML extension methods into a single generic helper. Behavior is identical to the code it replaces.


22-Dimension Analysis

#DimensionStatusNotes
1Algorithmic CorrectnessGuard, factory, and registration order are all preserved exactly
2Threading & ConcurrencyN/AConfiguration-time only; no concurrency
3Memory & Resource ManagementNo new disposables or allocations beyond the pre-existing lambda capture
4Public API SurfaceReportProviderRegistration is internal; all three public extension-method signatures are unchanged
5Backward CompatibilityNo public API changes
6Security & Trust BoundariesN/ANo security-sensitive paths
7Performance & AllocationcommandLine is still instantiated eagerly at the call site and captured in a single lambda — semantically identical to the old local-variable pattern
8Error Handling & ResilienceInvalidOperationException guard is preserved; consistent with codebase style of not adding null-argument guards for internal helpers
9Naming & ReadabilityReportProviderRegistration, AddReportProvider<TGenerator>, and invalidBuilderTypeErrorMessage are clear
10Code DuplicationThis is exactly what the PR addresses — three near-identical blocks collapsed into one
11Test Coveragei️No new unit tests for ReportProviderRegistration itself (guard path). This was a pre-existing gap; no regression introduced. The three Add*ReportProvider calls in the unit-test Program.cs exercise the happy path.
12Documentation & XML Commentsi️See inline comment — <see cref="TestApplicationBuilder"/> references an internal type. Works at compile time, minor cosmetic concern only.
13Cross-TFM CorrectnessNo TFM-conditional code; targets netstandard2.0 + net frameworks consistently
14Localization & Resource StringsNo new strings; existing ExtensionResources.InvalidTestApplicationBuilderType is passed through unchanged
15MSBuild / Project File Quality<Compile Include> is inserted at the same relative position in all three .csproj files, consistent with the existing Link="Helpers\*.cs" pattern
16Dependency ManagementNo new package references
17Pattern ConsistencyTrxReport and AzureDevOps have extra registrations (tools, OS-conditional branches, multiple composites) that correctly remain unreduced
18Scope DisciplineStrictly limited to the structural deduplication described in the PR
19TODO PolicyNo TODOs introduced
20Assertion Quality (tests)N/ANo new tests
21License HeadersNew file has correct Microsoft copyright header
22File OrganizationReportProviderRegistration.cs placed alongside sibling helpers in SharedExtensionHelpers/

One Pre-existing Gap (not introduced by this PR)

The builder is not TestApplicationBuilder guard is not unit-tested for any of the three affected extensions, either before or after this change. The refactoring is a good opportunity to track this as a follow-up: a single ReportProviderRegistrationTests class could cover the guard once (passing a mock ITestApplicationBuilder that is not a TestApplicationBuilder) rather than requiring three separate tests. Consider filing a tracking issue if desired.


Overall: The abstraction is well-designed, the generic constraint where TGenerator : class, IDataConsumer, ITestSessionLifetimeHandler is exactly right, and the InternalsVisibleTo grants that make TestApplicationBuilder accessible in all three consumer assemblies are already in place. No changes required.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 713 AIC · ⌖ 13 AIC ·

Comment threadsrc/Platform/SharedExtensionHelpers/ReportProviderRegistration.cs Outdated
…guard
Convert AddReportProvider's commandLine parameter to a Func factory so the
*ReportGeneratorCommandLine instance is only created after the
TestApplicationBuilder guard passes, restoring the original short-circuit
behavior. Also replace internal-type cref docs with prose.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink) merged commit 216ac8c into mainJun 15, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/dedup-report-extensions-9131 branch June 15, 2026 14:14
Sign up for freeto 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.

[duplicate-code] Duplicate Code: Report extension registration boilerplate repeated across Ctrf/JUnit/Html with inconsistent guard

3 participants

@Evangelink@JanKrivanek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Extract shared report-provider registration helper (#9131) by Evangelink · Pull Request #9152 · microsoft/testfx · GitHub
Skip to content

Extract shared report-provider registration helper (#9131) - #9152

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131
Jun 15, 2026
Merged

Extract shared report-provider registration helper (#9131)#9152
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9131.

Follow-up to #9134, which fixed the missing TestApplicationBuilder guard in CtrfReportExtensions but left the structural duplication (recommendation #2) unaddressed.

This extracts the repeated CompositeExtensionFactory construction, the TestApplicationBuilder guard, and the data-consumer / test-session-lifetime / command-line registration block — shared by the CTRF, JUnit, and HTML report extensions — into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers.

Each Add*ReportProvider method is now a single expression-bodied call that supplies only the generator factory and the per-extension resource strings. Behavior is unchanged (same guard, same registration order).

Validation

  • dotnet build of all three report extension projects: 0 warnings, 0 errors
  • Microsoft.Testing.Extensions.UnitTests (net8.0): all passed

Factor the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and data-consumer/session-lifetime/command-line registration shared by the CTRF, JUnit, and HTML report extensions into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers. Each Add*ReportProvider method is now a single expression-bodied call supplying only the generator factory and resource strings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 15, 2026 12:53

CopilotAI 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.

Pull request overview

This PR refactors the CTRF/JUnit/HTML report extensions in Microsoft.Testing.Platform to remove duplicated registration boilerplate by extracting the shared “guard + composite factory + registrations” logic into a single helper in SharedExtensionHelpers.

Changes:

  • Introduces ReportProviderRegistration.AddReportProvider<TGenerator> to centralize the TestApplicationBuilder guard and the standard registration sequence.
  • Updates CTRF/JUnit/HTML Add*ReportProvider extension methods to delegate to the shared helper.
  • Links the new shared helper into each of the three report extension projects.
Show a summary per file
FileDescription
src/Platform/SharedExtensionHelpers/ReportProviderRegistration.csAdds the shared helper that performs the guard and common registrations.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/Microsoft.Testing.Extensions.CtrfReport.csprojLinks the new shared helper into the CTRF extension project.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportExtensions.csRefactors AddCtrfReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/Microsoft.Testing.Extensions.JUnitReport.csprojLinks the new shared helper into the JUnit extension project.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportExtensions.csRefactors AddJUnitReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/Microsoft.Testing.Extensions.HtmlReport.csprojLinks the new shared helper into the HTML extension project.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/HtmlReportExtensions.csRefactors AddHtmlReportProvider to delegate to the shared helper.

Copilot's findings

  • Files reviewed: 7/7 changed files
  • Comments generated: 1

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary — PR #9152: Extract shared report-provider registration helper

Verdict: ✅ No blocking or major issues found. LGTM with one minor observation.

This is a clean, well-scoped refactoring that faithfully extracts the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and three-call registration block from the CTRF, JUnit, and HTML extension methods into a single generic helper. Behavior is identical to the code it replaces.


22-Dimension Analysis

#DimensionStatusNotes
1Algorithmic CorrectnessGuard, factory, and registration order are all preserved exactly
2Threading & ConcurrencyN/AConfiguration-time only; no concurrency
3Memory & Resource ManagementNo new disposables or allocations beyond the pre-existing lambda capture
4Public API SurfaceReportProviderRegistration is internal; all three public extension-method signatures are unchanged
5Backward CompatibilityNo public API changes
6Security & Trust BoundariesN/ANo security-sensitive paths
7Performance & AllocationcommandLine is still instantiated eagerly at the call site and captured in a single lambda — semantically identical to the old local-variable pattern
8Error Handling & ResilienceInvalidOperationException guard is preserved; consistent with codebase style of not adding null-argument guards for internal helpers
9Naming & ReadabilityReportProviderRegistration, AddReportProvider<TGenerator>, and invalidBuilderTypeErrorMessage are clear
10Code DuplicationThis is exactly what the PR addresses — three near-identical blocks collapsed into one
11Test Coveragei️No new unit tests for ReportProviderRegistration itself (guard path). This was a pre-existing gap; no regression introduced. The three Add*ReportProvider calls in the unit-test Program.cs exercise the happy path.
12Documentation & XML Commentsi️See inline comment — <see cref="TestApplicationBuilder"/> references an internal type. Works at compile time, minor cosmetic concern only.
13Cross-TFM CorrectnessNo TFM-conditional code; targets netstandard2.0 + net frameworks consistently
14Localization & Resource StringsNo new strings; existing ExtensionResources.InvalidTestApplicationBuilderType is passed through unchanged
15MSBuild / Project File Quality<Compile Include> is inserted at the same relative position in all three .csproj files, consistent with the existing Link="Helpers\*.cs" pattern
16Dependency ManagementNo new package references
17Pattern ConsistencyTrxReport and AzureDevOps have extra registrations (tools, OS-conditional branches, multiple composites) that correctly remain unreduced
18Scope DisciplineStrictly limited to the structural deduplication described in the PR
19TODO PolicyNo TODOs introduced
20Assertion Quality (tests)N/ANo new tests
21License HeadersNew file has correct Microsoft copyright header
22File OrganizationReportProviderRegistration.cs placed alongside sibling helpers in SharedExtensionHelpers/

One Pre-existing Gap (not introduced by this PR)

The builder is not TestApplicationBuilder guard is not unit-tested for any of the three affected extensions, either before or after this change. The refactoring is a good opportunity to track this as a follow-up: a single ReportProviderRegistrationTests class could cover the guard once (passing a mock ITestApplicationBuilder that is not a TestApplicationBuilder) rather than requiring three separate tests. Consider filing a tracking issue if desired.


Overall: The abstraction is well-designed, the generic constraint where TGenerator : class, IDataConsumer, ITestSessionLifetimeHandler is exactly right, and the InternalsVisibleTo grants that make TestApplicationBuilder accessible in all three consumer assemblies are already in place. No changes required.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 713 AIC · ⌖ 13 AIC ·

Comment threadsrc/Platform/SharedExtensionHelpers/ReportProviderRegistration.cs Outdated
…guard
Convert AddReportProvider's commandLine parameter to a Func factory so the
*ReportGeneratorCommandLine instance is only created after the
TestApplicationBuilder guard passes, restoring the original short-circuit
behavior. Also replace internal-type cref docs with prose.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink) merged commit 216ac8c into mainJun 15, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/dedup-report-extensions-9131 branch June 15, 2026 14:14
Sign up for freeto 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.

[duplicate-code] Duplicate Code: Report extension registration boilerplate repeated across Ctrf/JUnit/Html with inconsistent guard

3 participants

@Evangelink@JanKrivanek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Extract shared report-provider registration helper (#9131) by Evangelink · Pull Request #9152 · microsoft/testfx · GitHub
Skip to content

Extract shared report-provider registration helper (#9131) - #9152

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131
Jun 15, 2026
Merged

Extract shared report-provider registration helper (#9131)#9152
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/dedup-report-extensions-9131

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9131.

Follow-up to #9134, which fixed the missing TestApplicationBuilder guard in CtrfReportExtensions but left the structural duplication (recommendation #2) unaddressed.

This extracts the repeated CompositeExtensionFactory construction, the TestApplicationBuilder guard, and the data-consumer / test-session-lifetime / command-line registration block — shared by the CTRF, JUnit, and HTML report extensions — into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers.

Each Add*ReportProvider method is now a single expression-bodied call that supplies only the generator factory and the per-extension resource strings. Behavior is unchanged (same guard, same registration order).

Validation

  • dotnet build of all three report extension projects: 0 warnings, 0 errors
  • Microsoft.Testing.Extensions.UnitTests (net8.0): all passed

Factor the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and data-consumer/session-lifetime/command-line registration shared by the CTRF, JUnit, and HTML report extensions into a new ReportProviderRegistration.AddReportProvider helper in SharedExtensionHelpers. Each Add*ReportProvider method is now a single expression-bodied call supplying only the generator factory and resource strings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 15, 2026 12:53

CopilotAI 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.

Pull request overview

This PR refactors the CTRF/JUnit/HTML report extensions in Microsoft.Testing.Platform to remove duplicated registration boilerplate by extracting the shared “guard + composite factory + registrations” logic into a single helper in SharedExtensionHelpers.

Changes:

  • Introduces ReportProviderRegistration.AddReportProvider<TGenerator> to centralize the TestApplicationBuilder guard and the standard registration sequence.
  • Updates CTRF/JUnit/HTML Add*ReportProvider extension methods to delegate to the shared helper.
  • Links the new shared helper into each of the three report extension projects.
Show a summary per file
FileDescription
src/Platform/SharedExtensionHelpers/ReportProviderRegistration.csAdds the shared helper that performs the guard and common registrations.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/Microsoft.Testing.Extensions.CtrfReport.csprojLinks the new shared helper into the CTRF extension project.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportExtensions.csRefactors AddCtrfReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/Microsoft.Testing.Extensions.JUnitReport.csprojLinks the new shared helper into the JUnit extension project.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportExtensions.csRefactors AddJUnitReportProvider to delegate to the shared helper.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/Microsoft.Testing.Extensions.HtmlReport.csprojLinks the new shared helper into the HTML extension project.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/HtmlReportExtensions.csRefactors AddHtmlReportProvider to delegate to the shared helper.

Copilot's findings

  • Files reviewed: 7/7 changed files
  • Comments generated: 1

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary — PR #9152: Extract shared report-provider registration helper

Verdict: ✅ No blocking or major issues found. LGTM with one minor observation.

This is a clean, well-scoped refactoring that faithfully extracts the repeated CompositeExtensionFactory construction, TestApplicationBuilder guard, and three-call registration block from the CTRF, JUnit, and HTML extension methods into a single generic helper. Behavior is identical to the code it replaces.


22-Dimension Analysis

#DimensionStatusNotes
1Algorithmic CorrectnessGuard, factory, and registration order are all preserved exactly
2Threading & ConcurrencyN/AConfiguration-time only; no concurrency
3Memory & Resource ManagementNo new disposables or allocations beyond the pre-existing lambda capture
4Public API SurfaceReportProviderRegistration is internal; all three public extension-method signatures are unchanged
5Backward CompatibilityNo public API changes
6Security & Trust BoundariesN/ANo security-sensitive paths
7Performance & AllocationcommandLine is still instantiated eagerly at the call site and captured in a single lambda — semantically identical to the old local-variable pattern
8Error Handling & ResilienceInvalidOperationException guard is preserved; consistent with codebase style of not adding null-argument guards for internal helpers
9Naming & ReadabilityReportProviderRegistration, AddReportProvider<TGenerator>, and invalidBuilderTypeErrorMessage are clear
10Code DuplicationThis is exactly what the PR addresses — three near-identical blocks collapsed into one
11Test Coveragei️No new unit tests for ReportProviderRegistration itself (guard path). This was a pre-existing gap; no regression introduced. The three Add*ReportProvider calls in the unit-test Program.cs exercise the happy path.
12Documentation & XML Commentsi️See inline comment — <see cref="TestApplicationBuilder"/> references an internal type. Works at compile time, minor cosmetic concern only.
13Cross-TFM CorrectnessNo TFM-conditional code; targets netstandard2.0 + net frameworks consistently
14Localization & Resource StringsNo new strings; existing ExtensionResources.InvalidTestApplicationBuilderType is passed through unchanged
15MSBuild / Project File Quality<Compile Include> is inserted at the same relative position in all three .csproj files, consistent with the existing Link="Helpers\*.cs" pattern
16Dependency ManagementNo new package references
17Pattern ConsistencyTrxReport and AzureDevOps have extra registrations (tools, OS-conditional branches, multiple composites) that correctly remain unreduced
18Scope DisciplineStrictly limited to the structural deduplication described in the PR
19TODO PolicyNo TODOs introduced
20Assertion Quality (tests)N/ANo new tests
21License HeadersNew file has correct Microsoft copyright header
22File OrganizationReportProviderRegistration.cs placed alongside sibling helpers in SharedExtensionHelpers/

One Pre-existing Gap (not introduced by this PR)

The builder is not TestApplicationBuilder guard is not unit-tested for any of the three affected extensions, either before or after this change. The refactoring is a good opportunity to track this as a follow-up: a single ReportProviderRegistrationTests class could cover the guard once (passing a mock ITestApplicationBuilder that is not a TestApplicationBuilder) rather than requiring three separate tests. Consider filing a tracking issue if desired.


Overall: The abstraction is well-designed, the generic constraint where TGenerator : class, IDataConsumer, ITestSessionLifetimeHandler is exactly right, and the InternalsVisibleTo grants that make TestApplicationBuilder accessible in all three consumer assemblies are already in place. No changes required.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 713 AIC · ⌖ 13 AIC ·

Comment threadsrc/Platform/SharedExtensionHelpers/ReportProviderRegistration.cs Outdated
…guard
Convert AddReportProvider's commandLine parameter to a Func factory so the
*ReportGeneratorCommandLine instance is only created after the
TestApplicationBuilder guard passes, restoring the original short-circuit
behavior. Also replace internal-type cref docs with prose.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink) merged commit 216ac8c into mainJun 15, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/dedup-report-extensions-9131 branch June 15, 2026 14:14
Sign up for freeto 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.

[duplicate-code] Duplicate Code: Report extension registration boilerplate repeated across Ctrf/JUnit/Html with inconsistent guard

3 participants

@Evangelink@JanKrivanek