[code-simplifier] Extract duplicated localization assert helpers into AcceptanceAssert#9298
Conversation
Move the duplicated NormalizeForComparison and AssertOutputContainsNormalized/ AssertOutputDoesNotContainNormalized private helpers from LocalizationTests and LocalizationFailingTests into AcceptanceAssert as public extension methods. This eliminates the code duplication between the two recently-modified test files and makes the normalization helpers available for any future localization tests. The helpers are renamed to NormalizeForLocalization and made into proper extension methods following the existing AcceptanceAssert pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors duplicated localization-output normalization/assertion helpers out of LocalizationTests / LocalizationFailingTests into the shared AcceptanceAssert test helper to reduce duplication and standardize usage across acceptance tests.
Changes:
- Added
AssertOutputContainsNormalized/AssertOutputDoesNotContainNormalizedextension methods (plus a shared normalization helper) toAcceptanceAssert. - Removed now-redundant private helper methods from
LocalizationTestsandLocalizationFailingTests, updating call sites to use the new extension methods. - Removed redundant
using System.Text;directives from the localization test files.
Show a summary per file
| File | Description |
|---|---|
| test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/Helpers/AcceptanceAssert.cs | Introduces reusable normalized-output assertion helpers for localized string comparisons. |
| test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/LocalizationTests.cs | Switches localization assertions to the centralized AcceptanceAssert helpers and removes duplicated code. |
| test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/LocalizationFailingTests.cs | Switches failing-localization assertions to the centralized AcceptanceAssert helpers and removes duplicated code. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 2
…ed asserts The AssertOutputContainsNormalized / AssertOutputDoesNotContainNormalized helpers accepted caller-info parameters but never used them (IDE0060) and built ad-hoc failure messages. Route them through GenerateFailedAssertionMessage like every other AcceptanceAssert helper so failures include the member/file/line context. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Evangelink
left a comment
There was a problem hiding this comment.
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.
✅ 22/22 dimensions clean — no findings.
Observations (non-blocking)
The PR HEAD commit implements the helpers slightly better than the PR description's diff section shows. The description's diff depicts custom error strings ($"Output does not contain '{value}'..."), while the actual commit uses GenerateFailedAssertionMessage(testHostResult, callerMemberName, callerFilePath, callerLineNumber). The actual implementation is strictly better:
- Caller attributes are now used, not silently discarded. In the original private helpers the three
[CallerMemberName/FilePath/LineNumber]parameters were accepted but never forwarded — the custom message only capturedvalueandtestHostResult.StandardOutput. The refactored code correctly threads those values intoGenerateFailedAssertionMessage, producing a failure message that includes the test method name, source file path, and line number. - Richer output on failure.
GenerateFailedAssertionMessageemits{testHostResult}(fullToString()) rather than justtestHostResult.StandardOutput, so stderr, exit code, and other fields are also visible when a localization assertion fails.
The PR description's "Functionality is preserved" note is therefore a slight understatement — diagnostic quality actually improved. Worth noting so future reviewers don't see the description diff and wonder whether the caller attributes are wired up.
Everything else checks out: the NormalizeForLocalization lambda is byte-for-byte identical to NormalizeForComparison in both originals; the extension-method signatures exactly follow the established AcceptanceAssert pattern; the XML doc comments are accurate; the two removed using System.Text; directives were redundant with the project-level global using; and the [Ignore] annotations are correctly preserved on both test classes.
Code Simplification — 2026-06-20
This PR simplifies recently modified code to improve clarity, consistency, and maintainability while preserving all functionality.
Files Simplified
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/Helpers/AcceptanceAssert.cs— Added two public extension methodsAssertOutputContainsNormalized/AssertOutputDoesNotContainNormalizedand a privateNormalizeForLocalizationhelpertest/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/LocalizationTests.cs— Removed duplicated private helpers; updated call sites to use extension method syntaxtest/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/LocalizationFailingTests.cs— Removed duplicated private helpers; updated call sites to use extension method syntaxImprovements Made
Eliminated Code Duplication
LocalizationTestsandLocalizationFailingTestscontained identical private static helpers:NormalizeForComparison,AssertOutputContainsNormalized, and (inLocalizationTests)AssertOutputDoesNotContainNormalizedAcceptanceAssertas reusable extension methodsApplied Project Conventions
AcceptanceAssert(including[CallerMemberName],[CallerFilePath],[CallerLineNumber]optional parameters for diagnostic messages)testHostResult.AssertOutputContainsNormalized("...")instead ofAssertOutputContainsNormalized(testHostResult, "...")Removed Redundant
usingDirectiveusing System.Text;which is already covered by the project-level global using inDirectory.Build.propsChanges Based On
Recent changes from:
Testing
AcceptanceAssertpatterns[Ignore]d (per Temporarily disabled LocalizationTests/LocalizationFailingTests until OneLocBuild ships proper TerminalResources translations #9295) so they do not run until OneLocBuild ships proper translationsReview Focus
Please verify:
NormalizeForLocalizationlogic is byte-for-byte identical to the originalNormalizeForComparisonin both filesAcceptanceAssertpatternAdd this agentic workflows to your repo
To install this agentic workflow, run