Skip to content

[code-simplifier] Extract duplicated localization assert helpers into AcceptanceAssert#9298

Open
Evangelink wants to merge 2 commits into
mainfrom
simplify/extract-localization-assert-helpers-a02824efc0eb3db9
Open

[code-simplifier] Extract duplicated localization assert helpers into AcceptanceAssert#9298
Evangelink wants to merge 2 commits into
mainfrom
simplify/extract-localization-assert-helpers-a02824efc0eb3db9

Conversation

@Evangelink

Copy link
Copy Markdown
Member

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 methods AssertOutputContainsNormalized / AssertOutputDoesNotContainNormalized and a private NormalizeForLocalization helper
  • test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/LocalizationTests.cs — Removed duplicated private helpers; updated call sites to use extension method syntax
  • test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/LocalizationFailingTests.cs — Removed duplicated private helpers; updated call sites to use extension method syntax

Improvements Made

  1. Eliminated Code Duplication

    • Both LocalizationTests and LocalizationFailingTests contained identical private static helpers: NormalizeForComparison, AssertOutputContainsNormalized, and (in LocalizationTests) AssertOutputDoesNotContainNormalized
    • These 3 helpers (28 lines) are now consolidated into AcceptanceAssert as reusable extension methods
  2. Applied Project Conventions

    • The new extension methods follow the exact signature pattern of all other helpers in AcceptanceAssert (including [CallerMemberName], [CallerFilePath], [CallerLineNumber] optional parameters for diagnostic messages)
    • Call sites now use fluent extension method syntax consistent with the rest of the test suite: testHostResult.AssertOutputContainsNormalized("...") instead of AssertOutputContainsNormalized(testHostResult, "...")
  3. Removed Redundant using Directive

    • Both files had using System.Text; which is already covered by the project-level global using in Directory.Build.props

Changes Based On

Recent changes from:

Testing

Review Focus

Please verify:

  • Functionality is preserved — the NormalizeForLocalization logic is byte-for-byte identical to the original NormalizeForComparison in both files
  • The extension method signatures follow the established AcceptanceAssert pattern
  • No unintended side effects from the consolidation

🤖 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 Code Simplifier workflow. · 2.1K AIC · ⌖ 25.8 AIC · ⊞ 44.7K · [◷]( · )

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/code-simplifier.md@main
  • expires on Jun 21, 2026, 1:57 PM UTC

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>
Copilot AI review requested due to automatic review settings June 20, 2026 13:57
@Evangelink Evangelink added type/automation Created or maintained by an agentic workflow. type/tech-debt Code health, refactoring, simplification. labels Jun 20, 2026

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

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 / AssertOutputDoesNotContainNormalized extension methods (plus a shared normalization helper) to AcceptanceAssert.
  • Removed now-redundant private helper methods from LocalizationTests and LocalizationFailingTests, 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

@Evangelink Evangelink marked this pull request as ready for review June 20, 2026 14:11
…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 Evangelink left a comment

Copy link
Copy Markdown
Member Author

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.

✅ 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:

  1. 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 captured value and testHostResult.StandardOutput. The refactored code correctly threads those values into GenerateFailedAssertionMessage, producing a failure message that includes the test method name, source file path, and line number.
  2. Richer output on failure. GenerateFailedAssertionMessage emits {testHostResult} (full ToString()) rather than just testHostResult.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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/automation Created or maintained by an agentic workflow. type/tech-debt Code health, refactoring, simplification.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants