Skip to content

Backport genuinely-missing main commits to rel/4.3#9692

Merged
Evangelink merged 8 commits into
rel/4.3from
dev/amauryleve/backport-main-commits-to-rel-4-3
Jul 7, 2026
Merged

Backport genuinely-missing main commits to rel/4.3#9692
Evangelink merged 8 commits into
rel/4.3from
dev/amauryleve/backport-main-commits-to-rel-4-3

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Backports the genuinely-missing (non-vstest-removal) commits from main to rel/4.3. Every commit was cherry-picked with -x; the full solution builds clean (build.cmd -c Debug: 0 warnings, 0 errors).

Backported (8)

Perf

Tests

Docs / infra

Intentionally excluded

Evangelink and others added 8 commits July 7, 2026 09:06
…9588)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 0c3fc2b)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit fe518a5)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 8c167ed)
… captured (#9636)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 50adfdd)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit af913e0)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit b3c2614)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 621b8ff)
…9671)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 2e7c221)
@Evangelink Evangelink added the state/needs-review Awaiting review from the team. label Jul 7, 2026
@Evangelink Evangelink enabled auto-merge (squash) July 7, 2026 07:21
@Evangelink Evangelink disabled auto-merge July 7, 2026 07:22
@Evangelink Evangelink enabled auto-merge (squash) July 7, 2026 07:22
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9692

GradeTestNotes
B (80–89) new DuplicateTestMethodAttributeAnalyzerTests.
WhenDerivedAttributeIsFirstAndTestMethodIsSecond_
CodeFix_
KeepsDerivedAttribute
Body is ~52 lines due to embedded code strings; consider extracting the custom attribute boilerplate into a shared helper to reduce verbosity.
B (80–89) new PreferConstructorOverTestInitializeAnalyzerTests.
WhenTestClassHasOnlyParameterizedCtorAndTestInitialize_
CodeFix_
MergesIntoParameterizedCtor
Complex merge scenario is well-explained by the comment; body over 30 lines — trimming the embedded field declarations slightly would bring this to A.
B (80–89) new PreferConstructorOverTestInitializeAnalyzerTests.
WhenTestInitializeMethodInNonTestClass_
Diagnostic
Good edge case with informative comment explaining both diagnostic and fix; body just over 30 lines — a minor trim would reach A.
B (80–89) new PreferDisposeOverTestCleanupAnalyzerTests.
WhenTestCleanupMethodInNonTestClass_
Diagnostic
IDisposable conversion fix well-covered with comment; body is 32 lines — trimming the embedded code slightly would suffice.
B (80–89) new UseCooperativeCancellationForTimeoutAnalyzerTests.
WhenTimeoutAttributeOnAsyncTestMethod_
Diagnostic
Async-method edge case clearly verified with both diagnostic location and fix output; body 36 lines — shortening the test method stub would improve conciseness.
B (80–89) new UseCooperativeCancellationForTimeoutAnalyzerTests.
WhenTimeoutAttributeWithNamedArgument_
Diagnostic
Named-argument form well-covered; body just over 30 lines — a minor trim would be enough to reach A.
A (90–100) new DuplicateTestMethodAttributeAnalyzerTests.
WhenTestMethodHasDuplicateAttributesOutsideTestClass_
Diagnostic
Well-commented edge case; diagnostic location precisely marked via [|...|], clear single-behavior focus in ~20 lines.
A (90–100) new GlobalTestFixtureShouldBeValidAnalyzerTests.
WhenGlobalTestCleanupWithDerivedTestClassAttribute_
NoDiagnostic
Negative test verifying attribute inheritance is respected; comment references the analyzer's Inherits(testClassAttributeSymbol) guard directly.
A (90–100) new GlobalTestFixtureShouldBeValidAnalyzerTests.
WhenGlobalTestInitializeInGenericClass_
Diagnostic
Comment links the test to the allowGenericType: false parameter; concise, precisely scoped edge case in ~18 lines.
A (90–100) mod LocalizationFailingTests.
Execution_
WithFailingTest_
OutputContainsTranslatedFailureSummary
Multiple assertions: exit code + normalized French string checks; data-driven per TFM. Re-enabled from class-level ignore.
A (90–100) mod LocalizationTests.
Execution_
WithFrenchLocale_
OutputContainsTranslatedSummary
Thorough: exit code + 5 positive + 2 negative assertions covering all translated count labels. Re-enabled from class-level ignore.
A (90–100) mod LocalizationTests.
Execution_
WithSpanishLocale_
OutputContainsTranslatedSummary
Good parallel structure with the French test; direct AssertOutputContains (no normalization) is appropriate for Spanish. Re-enabled from class-level ignore.
A (90–100) mod LocalizationTests.
Execution_
WithTestingPlatformUILanguage_
TakesPrecedenceOverDotnetCLI
Clean precedence test with comment explaining expected outcome; exit code + positive and negative assertions. Re-enabled from class-level ignore.
A (90–100) new UseCooperativeCancellationForTimeoutAnalyzerTests.
WhenTimeoutAttributeOnMethodInNonTestClass_
Diagnostic
Non-test-class edge case cleanly covered; both diagnostic and fix verified in ~28 lines — the most concise of the three new timeout tests.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.

🤖 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 Grade Tests on PR (on open / sync) workflow. · 149.6 AIC · ⌖ 9.3 AIC · ⊞ 9.5K · [◷]( · )

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

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.

Expert Review — PR #9692

Dimension Severity Verdict Notes
1. Algorithmic Correctness MAJOR GetInvokeResultWithParametersAsync correctly threads the cached ParameterInfo[] through all call sites. ConstructGenericMethod signature and internal usage updated consistently.
2. Threading & Concurrency BLOCKING TestMethodInfo.ParameterTypes uses benign lazy-init (field ??=); worst case is double-compute of an equivalent array. No shared-mutable-state concerns.
3. Security & IPC Contract Safety BLOCKING No new deserialization or IPC changes. Workflow uses contents: read at top level with write scoped to the update job only.
4. Public API & Binary Compatibility BLOCKING All new/changed methods are internal. No public surface changes.
5. Performance & Allocations MAJOR Core goal of the PR — GetParameters() allocation eliminated from the hot path by caching via ParameterTypes property. Clean split between hot-path (GetInvokeResultWithParametersAsync) and non-hot (GetInvokeResultAsync wrapper).
6. Cross-TFM Compatibility MAJOR Changes compile across all TFMs (net462/netstandard2.0/net8.0/net9.0). No TFM-specific APIs introduced.
7. Test Coverage & Quality MAJOR New analyzer edge-case tests follow existing patterns (VerifyAnalyzerAsync/VerifyCodeFixAsync). Localization tests re-enabled.
8. Error Handling & Diagnostics MAJOR RuntimeError raised with context in the Python script. C# error paths unchanged (same TestFailedException messages).
9. Naming & Style MINOR arguments == nullarguments is null ✓. methodParameters?.Length ?? 0methodParameters.Length (now non-nullable) ✓.
10. Documentation & Comments MINOR Good inline comments explaining the thin-wrapper pattern in MethodInfoExtensions. README updated with 8-day PAT policy and fast-unblock instructions.
11. Resource & Lifecycle Management MAJOR N/A — no IDisposable or CancellationToken flow changes.
12. Localization MAJOR N/A — no new user-facing strings.
13. Build & Project File Quality MAJOR N/A — no .csproj / .props / .targets changes.
14. Backward / Forward Compatibility MAJOR N/A — no wire-format changes.
15. Dependency Hygiene MAJOR N/A — no new package references.
16. Changelog / Release Notes MINOR #9641 entry added. Old testanywhere links corrected to testfx.
17. Agentic Workflow Quality MAJOR N/A — no agentic workflow .md sources changed.
18. TODO Comment Policy MINOR No bare TODO comments introduced.
19. CI/CD & Workflow Correctness MAJOR Actions pinned to full SHAs (actions/checkout@34e11..., actions/setup-python@a26af..., peter-evans/create-pull-request@67ccf...). Least-privilege permissions. Concurrency control prevents racing updates.
20. Inline Test Infrastructure MAJOR N/A — no acceptance test duration assertions.
21. MSBuild / SDK Quality MAJOR N/A — no props/targets changes.
22. Python / Shell Script Quality MAJOR ⚠️ Minor: _stable_tuple(current) can return None causing TypeError if the props file ever holds a prerelease string. Low practical risk. See inline comment.

Overall Verdict: LGTM

Well-crafted backport. The perf optimization in MethodInfoExtensions is the highest-impact change — it correctly eliminates per-invocation GetParameters() allocations on the data-driven test hot path by threading a lazily-cached ParameterInfo[] from TestMethodInfo.ParameterTypes. The new Playwright bump workflow follows all CI security best practices. Test additions are thorough edge-case coverage for existing analyzers.

Comment thread .github/scripts/update_bundled_playwright.py
@Evangelink Evangelink merged commit b7d1d85 into rel/4.3 Jul 7, 2026
41 checks passed
@Evangelink Evangelink deleted the dev/amauryleve/backport-main-commits-to-rel-4-3 branch July 7, 2026 08:37
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.

2 participants