Porting latest changes. by AbhitejJohn · Pull Request #2 · microsoft/testfx

Abhitej John (AbhitejJohn) · 2016-08-30T14:36:27Z

- Fix#1: Replace hardcoded 'Expected all elements...' with AllMatchPredicateFailNew resource - Fix#2: Remove 11 dead resource strings (ContainsFail, StartsWithFail, EndsWithFail, IsMatchFail, IsNotMatchFail, DoesNotEndWithFail, DoesNotStartWithFail, AssertThatFailedFormat, AssertThatMessageFormat, AssertThatDetailsPrefix, CollectionEqualReason) - Fix#3: Replace bare catch with catch (Exception) in TryEvaluateFormatted - Fix#5: Type-check StartsWith/EndsWith/Contains to only match string methods - Fix#6: Add explicit ellipsis sentinel handling in FormatCallSite - Fix#7: Fix grammar 'Both collection contain same elements' -> 'Both collections contain the same elements'

The PR's recent introduction of `MockableReflectionOperations` (which implements the new generic interface methods by filtering results from `mock.Object.GetCustomAttributes(Assembly, typeof(Attribute))`) plus a couple of test-only mistakes left the Windows CI runs red on .NET FX 4.8, net8.0 and net9.0. This commit addresses both the failures and the two remaining unresolved Copilot review comments. Test fixes: - `TestAssemblySettingsProviderTests` (4 tests): the existing setups use `Setup(ro => ro.GetCustomAttributes(It.IsAny<Assembly>(), typeof(ParallelizeAttribute)))` but `MockableReflectionOperations` always passes `typeof(Attribute)` (matching production's `NotCachedReflectionAccessor` for assemblies), so those setups never matched and the tests saw a default `Workers = -1`. Switch the setups to `typeof(Attribute)`. - `ReflectionOperationsTests` (4 `GetTestPropertiesAsTraits*` tests): switched from `Should().Equal(new[] { new Trait(...) })` to `Should().BeEquivalentTo(new[] { new Trait(...) }, options => options.WithStrictOrdering())`. `Trait` does not override `Equals`, so the previous `Equal()` call performed reference comparison and always failed even when values matched. Review comment #2 (`TestableExtendedTestMethod` declared public in the global namespace): the type is already in the file-scoped `MSTestAdapter.PlatformServices.UnitTests.Services` namespace (the reviewer's claim about the global namespace is not accurate with the file-scoped namespace declaration), but it does not need to be public — made it `internal`. Review comment #1 (`AttributeMockingHelper` treating class providers as `TypeInfo` rather than `Type`): investigated and confirmed the premise is incorrect. On all supported runtimes (.NET Framework 4.5+ and .NET Core 1.0+), `RuntimeType` derives from `TypeInfo`, so `typeof(Foo) is TypeInfo` is `true` and the class-level branch in `AttributeMockingHelper.GetCustomAttributesNotCached` is reachable. Verified locally with a small repro on net8.0. No change needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix stale 21-dimension references throughout the doc now that the count is 22 (lines: 'Assess all 21 dimensions...', batching math '4 batches for 21 dimensions, last batch has 3' -> 'last batch has 4', summary table denominator 'N/21' -> 'N/22', example footers '19/21' and '21/21' updated, MSBuild supplemental section now says it is 'not a 23rd dimension'). * Perf rule #7: clarify the cross-TFM-safe fix. `MemoryMarshal.CreateReadOnlySpan(ref ch, 1)` with the `ReadOnlySpan<char>` overload of `Encoding.GetBytes` is not universally available, and `ref ch` cannot be applied to a `foreach` iteration variable. Recommend a single `char[1]` declared outside the loop (works on every TFM), with `stackalloc char[1]` + Span overload as the net6+ alternative. * Split former rule #8 in two: #8 (PowerShell ` += `) and #9 (C# `result = result.Concat(page).ToArray()`). The reviewer correctly noted that `array +=` is PowerShell-specific and lumping both forms under one rule confuses C#-only reviewers. * IPC rule #5: add the explicit bounds checks (`charCount > 0 && charCount < input.Length`) before indexing `input[charCount - 1]`. Without them the rule's own recommended fix would throw on truncate-to-zero inputs. * PowerShell rule #6: stop suggesting `catch { \$failures += '...' }` which reintroduces the O(n^2) accumulation defect called out by rule #2. Recommend a `List[string]` + `.Add()` instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…t globs Root cause of the broken main build: the acceptance source-gen build harness let a multi-project asset's leftover reflection outputs pollute the later source-gen build. TestAssetFixtureBase builds each asset twice in the same directory: first the reflection build (default bin/Release), then the source-gen build (bin/obj redirected to a *SourceGen sub-folder). For a multi-project asset (e.g. AssemblyFixtureProviderAcceptance, whose test project references ProviderLibrary), the reflection build leaves ProviderLibrary/bin/Release/<tfm>/*.dll on disk. When the source-gen build then evaluates the host project, the SDK's default item globs pulled those stray DLLs in as None items, and RAR treated a wrong-TFM copy (a net10.0 MSTest.TestFramework.Extensions.dll, assembly version 9.0.0.0) as a candidate for the net8.0 leg -> MSB3277, which MSBuildTreatWarningsAsErrors promotes to an error, failing the fixture's ClassInitialize. The injected props already re-excluded bin/obj, but only at the asset root (bin/**;obj/**), which does not match the nested ProviderLibrary/bin/**. Broaden the re-exclusion to **/obj/**;**/bin/** so a referenced project's leftover reflection outputs stay out of the source-gen build's globs. The reflection build (#1) is unaffected because at its evaluation time the referenced project's bin is still empty; only the later source-gen build (#2) runs in a directory polluted by #1. This is the proper harness fix; the AssemblyFixtureProvider opt-out is no longer needed, so the asset is restored to source-gen coverage. Verified by running the AssemblyFixtureProvider acceptance tests (reflection + source-gen builds) green across net462/net8.0/net10.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…crosoft#2) - Wrap per-node work in try/catch + LogWarning, mirroring the sibling GitHubActionsSlowTestReporter/GitHubActionsSummaryReporter so a failure while building/emitting an annotation (e.g. a malformed stack-trace path making IFileSystem.ExistFile throw) degrades to "no annotation for this test" instead of propagating into the platform's data-consumer dispatch. - Use FirstOrDefault instead of SingleOrDefault for the test-node state so a malformed node carrying multiple state properties does not throw. - Drop the dead testDisplayName parameter and compute GetTestName once. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Reverts the ##9724 change that allowed the literal value '--minimum-expected-tests 0'. Keeping '0' invalid restores an unambiguous meaning for GetMinimumExpectedTests() == 0 ('option not set'), which avoids the terminal-reporter verdict divergence tracked in ##9744: with '--min 0' the exit code was 0 (success) while the terminal still rendered a failed 'Zero tests ran' verdict, because the shared verdict helpers could not distinguish 'unset' from 'explicit 0' without a cross-repo change to the vendored TerminalTestReporterOptions. Issue ##7457 item #2 ('--min 0 == --ignore-exit-code 8') remains served by the existing '--ignore-exit-code 8'. The load-bearing behavior for the 'dotnet test --test-modules' orchestrator -- an explicit '--minimum-expected-tests N' governs the zero-tests verdict (exit code 9, not 8) -- is unaffected and stays. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Reverts the #9724 change that allowed the literal value '--minimum-expected-tests 0'. Keeping '0' invalid restores an unambiguous meaning for GetMinimumExpectedTests() == 0 ('option not set'), which avoids the terminal-reporter verdict divergence tracked in #9744: with '--min 0' the exit code was 0 (success) while the terminal still rendered a failed 'Zero tests ran' verdict, because the shared verdict helpers could not distinguish 'unset' from 'explicit 0' without a cross-repo change to the vendored TerminalTestReporterOptions. Issue #7457 item #2 ('--min 0 == --ignore-exit-code 8') remains served by the existing '--ignore-exit-code 8'. The load-bearing behavior for the 'dotnet test --test-modules' orchestrator -- an explicit '--minimum-expected-tests N' governs the zero-tests verdict (exit code 9, not 8) -- is unaffected and stays. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Append the product version as an anchor so nuget.org's Release Notes tab deep-links to the entry for that release: MSTest packages use VersionPrefix (e.g. #4.4.0) and Microsoft.Testing.Platform packages use TestingPlatformVersionPrefix (e.g. #2.4.0), which keeps extension packages with their own VersionPrefix pointing at the right platform entry. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6b5e055b-a59c-423f-aa05-3403db796d95

Addresses the remaining three semantic gaps from Amaury's review: - Return types (#3): resolve Task/ValueTask at compilation start and compare by symbol, so Task<T>/ValueTask<T> are rejected; require non-async void. - ClassInitialize vs ClassCleanup params (#4): split the class-fixture kind so ClassInitialize requires exactly one TestContext while ClassCleanup allows zero or one; a parameterless legacy [ClassInitialize] is no longer flagged. - Hiding (#2): make suppression member-kind-aware and signature-based. Instance fixtures are suppressed only by a same-name derived method that is itself a valid fixture; test methods only by a same-signature method; class fixtures accumulate and are never name-suppressed. Overrides still always suppress. Adds C# tests for each case plus two Visual Basic tests confirming language parity. 🤖 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Abhitej John (AbhitejJohn) · 2016-08-30T14:36:27Z

Jayarani garg (@jayaranigarg)codito