Clean up the test harness - #1369
Merged
Merged
Conversation
This was referenced Sep 1, 2026
Closed
Closed
This was referenced Sep 24, 2026
Open
Open
Open
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Analyzer
DoNotIgnoreReturnValueAnalyzer) now declares a single descriptor.ReturnValueRuleandOutParameterRuleshared theMA0060id, which forced every test of that rule to useMarkupOptions.UseFirstDescriptor. They are merged into oneRulewithmessageFormat: "{0}"(the pattern already used byOptimizeStringBuilderUsageAnalyzerand others), and the message is composed at the report site. The reported messages are unchanged, suffix included:The return value of 'X' should be used(+: <Message>from[DoNotIgnore])The out parameter 'x' of 'M' should not be discarded(+: <Message>)Tests
CodeFixTestBehaviors.FixOne | CodeFixTestBehaviors.SkipFixAllCheckpairs. The two flags are orthogonal and the pairing is valid, but only 7 of the 13 sites needed a flag at all: 4 tests inAvoidUnusedInternalTypesAnalyzerTestskeepSkipFixAllCheck(the fix removes the file's only type, so the harness cannot compare a state with no document) and 3 inOptimizeStringBuilderUsageAnalyzerTestskeepFixOne(the fix reveals another diagnostic). Each removal was verified by re-running the affected classes.DoNotIgnoreReturnValueAnalyzerTestsdropsUseFirstDescriptorand, now that the message is assertable, pins both message shapes with{|#0:…|}+WithMessage(...).ReferenceAssemblieswith an optional version —AddSerilog(),AddSqlite(),AddEntityFrameworkCore(),AddNewtonsoftJson(),AddMoq(), … — instead of inlineAddPackages([new PackageIdentity(...)]). Default versions are the latest stable ones (MSTest 4.3.3, NUnit 4.6.1, xUnit v3 4.0.0, EF Core 10.0.11, Serilog 4.4.0, YamlDotNet 18.1.0, …); a test that needs a specific version passes it.AddXUnitApi→AddXunit(now xUnit v3),AddMSTestApi→AddMSTest,AddNUnitApi→AddNUnit. AddedAddXunitV2for the test that asserts the pre-Xunit.TestContextbehavior. RemovedAddSystemTextJsonandAddSystemCollectionsImmutable, which only existed because a few tests compiled against an older target framework.DoNotUseBlockingCallInAsyncContextAnalyzer_AsyncContextTestsno longer pinsnetstandard2.1for the whole class (it dated back to needingValueTaskin 2020). OnlyProcessWaitForExit_NET5depended on it and now pinsNet50explicitly, matching its name and itsProcessWaitForExit_NET6counterpart.UseStringComparerAnalyzerTestsimmutable-collection tests no longer targetnet48+ aSystem.Collections.Immutablepackage; nothing in them is framework specific.Build / CI
paths-ignoreof thepushtrigger now includestests/**, so a push tomaintouching only tests does not run the workflow and does not publish a package. Pull requests still run the full matrix.Meziantou.NET.Sdk,.Testand.Webto1.0.161.Microsoft.Bcl.AsyncInterfacesmust stay: the suppressor testsAssembly.LoadFromthe IDE code-style analyzers, which need it in the output folder.Notes for reviewers
TforAreEqual(null, true), and NUnit 4 movedAssert.AreEqualtoClassicAssert(which MA0003 does not exempt), so the snippet is nowAssert.That(true, Is.True)— still a boolean literal onNUnit.Framework.Assert, so it exercises the same exemption.Assert.That(null, Is.Null)was rejected because it is ambiguous under Roslyn 4.8.DbContext_Addmoved offNet60because EF Core 10 does not supportnet6.0.global.jsonwas updated despite theAGENTS.mdrule, at the maintainer's request.Validation
Full test suite passes on every supported Roslyn version: 4.8 (3748), 4.14 (3775), 5.0 (3803), 5.6 (3827), 5.9 (3860) — 0 failures.
dotnet buildis clean anddotnet run --project src/DocumentationGeneratorreports no documentation change.