Skip to content

Refactor TestClassModelBuilder into focused files (#9910)#9911

Merged
Evangelink merged 5 commits into
mainfrom
dev/amauryleve/glowing-fortnight
Jul 14, 2026
Merged

Refactor TestClassModelBuilder into focused files (#9910)#9911
Evangelink merged 5 commits into
mainfrom
dev/amauryleve/glowing-fortnight

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes #9910.

Summary

Splits the 933-line src/Analyzers/MSTest.SourceGeneration/Generators/TestClassModelBuilder.cs into smaller, single-responsibility files. All new types remain internal static in the same ...SourceGeneration.Generators namespace, so there is no change to the internal or public API surface and behavior is preserved.

New files (all under 300 lines)

File Lines Responsibility
TestClassModelBuilder.cs (slimmed) 207 Top-level orchestration: walks the inheritance chain, assembles the TestClassModel, delegates to helpers
DynamicDataSourceBuilder.cs 288 Resolving/validating [DynamicData] sources (incl. the DynamicDataSourceType enum)
AttributeMaterializationHelper.cs 245 Attribute inheritance collection, trimming/AOT materializability, AttributeData → model conversion
TestMemberValidationHelper.cs 131 Member eligibility, unsupported-shape diagnostics (AOTSG0004/0005), method signature keys
SymbolReferenceabilityHelper.cs 81 Reusable accessibility/referenceability predicates over Roslyn symbols
DataRowBuilder.cs 74 [DataRow] parsing
SymbolDisplayFormats.cs 21 Shared fully-qualified SymbolDisplayFormat used across the builders

The public BuildAttributes entry point moved to AttributeMaterializationHelper; its only external caller (MSTestReflectionMetadataGenerator) was updated accordingly.

Notes

  • The existing shared MSTest.Analyzers.Shared.SymbolAccessibilityHelper (linked from ../Shared/) was not merged into, since it is a separate, cross-project concern. The builder's private referenceability predicates went into a new SymbolReferenceabilityHelper to avoid a name/namespace clash.
  • Namespace remains Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.SourceGeneration.Generators.

Validation

  • .\build.cmdBuild succeeded, 0 warnings, 0 errors.
  • MSTest.SourceGeneration.UnitTests (net8.0) — 113/113 passed.

Split the 933-line TestClassModelBuilder.cs into smaller, single-responsibility files within the same Generators namespace:

- DynamicDataSourceBuilder: [DynamicData] source resolution
- AttributeMaterializationHelper: attribute collection/materialization + TypedConstant conversion
- DataRowBuilder: [DataRow] parsing
- SymbolReferenceabilityHelper: reusable accessibility/referenceability predicates
- TestMemberValidationHelper: member eligibility, unsupported-shape reporting, signature keys
- SymbolDisplayFormats: shared fully-qualified SymbolDisplayFormat

TestClassModelBuilder now only holds top-level orchestration. Behavior is unchanged; all files are under 300 lines. Updated the BuildAttributes call site in MSTestReflectionMetadataGenerator.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2fc1a0b3-faab-4c48-a40a-23eae6fa9386
Copilot AI review requested due to automatic review settings July 13, 2026 19:46

Copilot AI 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

Refactors the source-generation model builder into focused helpers while preserving behavior and API visibility.

Changes:

  • Extracts dynamic data, data row, attribute, validation, and symbol logic.
  • Centralizes symbol display formatting.
  • Updates orchestration and assembly metadata call sites.
Show a summary per file
File Description
TestClassModelBuilder.cs Retains top-level model orchestration.
DynamicDataSourceBuilder.cs Handles dynamic-data resolution.
AttributeMaterializationHelper.cs Handles attribute inheritance and conversion.
TestMemberValidationHelper.cs Handles member validation and signatures.
SymbolReferenceabilityHelper.cs Centralizes referenceability checks.
DataRowBuilder.cs Handles data-row parsing.
SymbolDisplayFormats.cs Shares symbol display formatting.
MSTestReflectionMetadataGenerator.cs Uses the extracted attribute helper.

Review details

  • Files reviewed: 8/8 changed files
  • Comments generated: 0
  • Review effort level: Medium

@Evangelink Evangelink added the state/needs-review Awaiting review from the team. label Jul 13, 2026
@Evangelink Evangelink enabled auto-merge (squash) July 13, 2026 19:51
…ach filters

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2fc1a0b3-faab-4c48-a40a-23eae6fa9386
Copilot AI review requested due to automatic review settings July 13, 2026 20:12

Copilot AI 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.

Review details

  • Files reviewed: 8/8 changed files
  • Comments generated: 6
  • Review effort level: Medium

Comment thread src/Analyzers/MSTest.SourceGeneration/Generators/DataRowBuilder.cs
Regenerated via UpdateXlf. The last localized check-in added translations for
'GlobalTestFixtureShouldBeValidClassLayout' (fr) and 'SlowTestStillRunning' (ru)
that XliffTasks rejects (placeholder mismatch), breaking the build with
"xlf is out-of-date with resx". UpdateXlf resets those two units to state="new"
so the build passes; they will be re-translated on the next localization pass.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4350f662-0e5d-4e05-b156-1ef619da3b9e
Copilot AI review requested due to automatic review settings July 14, 2026 08:01

Copilot AI 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.

Review details

  • Files reviewed: 10/10 changed files
  • Comments generated: 2
  • Review effort level: Medium

Comment thread src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf
Copilot AI review requested due to automatic review settings July 14, 2026 09:06

Copilot AI 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.

Review details

  • Files reviewed: 10/10 changed files
  • Comments generated: 5
  • Review effort level: Low

Comment thread src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf
Comment thread src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf
Copilot AI review requested due to automatic review settings July 14, 2026 09:47

Copilot AI 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.

Review details

  • Files reviewed: 10/10 changed files
  • Comments generated: 2
  • Review effort level: Medium

Comment thread src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Build Failure Analysis

Root Cause: Duplicate <target> elements in XLF localization files

The build fails because two XLF files contain duplicate <target> elements within a single <trans-unit>, which violates the XLIFF 1.2 schema (exactly one <target> per <trans-unit> is required). MSBuild's XlfTransformer task rejects the malformed XML.


Error Group 1: src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf

Lines 566–595: The GlobalTestFixtureShouldBeValidDescription trans-unit has two <target> elements:

  1. Lines 566–580: Contains a partial French translation (state="new")
  2. Lines 581–595: Duplicate English-only fallback (state="new")

Fix: Remove the duplicate second <target> block (lines 581–595). Keep the first one which contains the actual French translation.


Error Group 2: src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.ru.xlf

Lines 77–78: The SlowTestStillRunning trans-unit has two <target> elements:

  1. Line 77: Correct Russian translation (state="translated")
  2. Line 78: Duplicate English-only fallback (state="new")

Fix: Remove line 78 (the duplicate <target state="new"> element).


Summary

File Issue Fix
src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf Duplicate <target> in GlobalTestFixtureShouldBeValidDescription Remove lines 581–595
src/Platform/.../GitHubActionsResources.ru.xlf Duplicate <target> in SlowTestStillRunning Remove line 78

Both issues appear to be from a OneLocBuild merge that inserted a translated <target> without removing the pre-existing state="new" placeholder.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🤖 Automated content by GitHub Copilot. Generated by the Build Failure Analysis workflow. · 360.1 AIC · ⌖ 5.2 AIC · ⊞ 6K · [◷]( · )

@github-actions github-actions Bot 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.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🤖 Automated content by GitHub Copilot. Generated by the Build Failure Analysis workflow. · 360.1 AIC · ⌖ 5.2 AIC · ⊞ 6K ·

Comments that could not be inline-anchored

src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.ru.xlf:78

Duplicate &lt;target&gt; element — XLF schema allows only one per &lt;trans-unit&gt;. The Russian translation on the line above is correct; remove this English fallback.

src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf:595

Duplicate &lt;target&gt; element — XLF allows only one per &lt;trans-unit&gt;. The French translation is in the target above (lines 566-580). Remove this duplicate English-only block.

@Evangelink Evangelink disabled auto-merge July 14, 2026 11:01
@Evangelink Evangelink merged commit 0d96d48 into main Jul 14, 2026
31 of 38 checks passed
@Evangelink Evangelink deleted the dev/amauryleve/glowing-fortnight branch July 14, 2026 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-review Awaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[file-diet] Refactor TestClassModelBuilder.cs (933 lines) into focused, smaller files

3 participants