Remove repo-wide analyzer relaxation and honor test cancellation#418
Merged
Merged
Conversation
CODESTYLE "Analyzer Suppressions" forbids relaxing a batch of rules repo-wide, and names dotnet_analyzer_diagnostic.severity as the exact anti-pattern. Removing it surfaced one rule, xUnit1051, across 29 call sites in the test project; the library and Sandbox were already clean under AnalysisLevel=latest-all with no findings behind the relaxation. Pass TestContext.Current.CancellationToken at each site so xUnit v3 can cancel a stalled test. This matters most in the network-bound Download tests, where a hung request previously had no cancellation path. Every remaining suppression was re-verified as load-bearing by removing it and rebuilding: CA1711 (Ex suffix), CA1707 and CA1515 (xUnit naming and visibility), and the IL3058 NoWarn (Polly and Serilog are not annotated AOT-compatible, and IL3058 has no source location to scope). IDE0055 stays because CSharpier owns formatting; it now carries that rationale as a comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #418 +/- ##
========================================
Coverage 66.89% 66.89%
========================================
Files 13 13
Lines 1160 1160
Branches 108 108
========================================
Hits 776 776
Misses 338 338
Partials 46 46 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens analyzer suppression hygiene by removing a repo-wide analyzer relaxation from the root .editorconfig, and updates async tests to pass TestContext.Current.CancellationToken so xUnit v3 can cancel stalled tests.
Changes:
- Remove
dotnet_analyzer_diagnostic.severityrelaxation from the root.editorconfig(keepIDE0055suppression with rationale). - Plumb
TestContext.Current.CancellationTokeninto async calls acrossUtilitiesTests(Download/FileEx/HttpClient/StringCompression/Extensions tests).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| UtilitiesTests/StringCompressionAsyncTests.cs | Passes xUnit cancellation token into async compression/decompression calls. |
| UtilitiesTests/HttpClientFactoryResilienceTests.cs | Passes xUnit cancellation token into HttpClient.GetAsync calls. |
| UtilitiesTests/FileExAsyncTests.cs | Passes xUnit cancellation token into FileEx and File.*Async calls (but contains one argument-order compile issue). |
| UtilitiesTests/ExtensionsTests.cs | Passes xUnit cancellation token into async string extension calls. |
| UtilitiesTests/DownloadAsyncTests.cs | Passes xUnit cancellation token into Download.*Async calls. |
| .editorconfig | Removes repo-wide analyzer relaxation and documents why IDE0055 is suppressed. |
Merged
ptr727
added a commit
that referenced
this pull request
Jul 18, 2026
Promotion of the current `develop` integration state to `main`. ## Contents | Area | Change | | --- | --- | | Analyzer conformance | #418 -- removed the repo-wide `dotnet_analyzer_diagnostic` relaxation, fixed the 29 `xUnit1051` sites it hid | | Docs | #419 -- Codecov coverage shield in the README build status block | | Repo config | #417 -- repo-config carry refresh and self-audit carry | ## Conformance issues resolved Closes #380 Closes #387 #380 (xUnit v3 + AwesomeAssertions, `ILoggerFactory` seam) was already satisfied in the tree -- verified rather than assumed: 0 `Assert.*` remaining against 245 `.Should()` calls, `xunit.v3` 3.2.2 pinned with no v2 package, zero Serilog references in the library, and `Utilities/LogOptions.cs` matching the LanguageTags reference seam. #387's four findings were likewise already addressed except finding 1 (analyzer suppression hygiene), which #418 closes. Every remaining suppression was independently probed by removing it and rebuilding, and all proved load-bearing (`CA1711`, `CA1707`, `CA1515`, `NoWarn IL3058`). ## Release impact: no auto-publish This promotion carries **no shipped input**. Per [WORKFLOW.md](../blob/main/WORKFLOW.md) the inclusion list is `Utilities/**`, `version.json`, `Directory.Build.props`, `Directory.Packages.props`; this diff is confined to tests, docs, CI, and repo config: ``` .editorconfig .github/workflows/merge-bot-pull-request.yml AUDIT.md README.md UtilitiesTests/*.cs repo-config/* spec/secrets.json ``` `Utilities/**` is byte-identical between `main` and `develop`, so the published package is already current and merging this will correctly **not** trigger a stable release.
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.
Follow-up to the conformance audit for #387 (finding 1, analyzer suppression hygiene).
What prompted this
CODESTYLE.md "Analyzer Suppressions" names
dotnet_analyzer_diagnostic.severityas the exact anti-pattern it forbids -- relaxing a batch of rules repo-wide -- yet the root.editorconfigcarried it.What removing it surfaced
Exactly one rule, and none of it in shipping code:
UtilitiesandSandboxproduced zero findings underAnalysisLevel=latest-all+AnalysisMode=All. I re-ran with-p:EnforceCodeStyleInBuild=trueto confirm IDE analyzers were not being silently skipped -- same result. The relaxation was hiding nothing in library code.xUnit1051 is a true positive, not a suppression candidate: it asks that async calls in tests pass
TestContext.Current.CancellationTokenso xUnit v3 can cancel a stalled test. The clearest case isDownloadAsyncTests, which makes real network calls and previously had no cancellation path if a request hung.Audit of every other suppression
Each was removed and rebuilt independently to prove it is load-bearing. All of them are -- kept, no changes:
CA1711Exsuffix conventionCA1707Method_Scenario_ExpectednamingCA1515NoWarn IL3058PublishAot=true)IDE0055The naming-rule severities were also raised to
warningas a probe and produced nothing.Verification
dotnet build -c ReleasewithTreatWarningsAsErrorsintact: 0 warnings, 0 errorsdotnet test -c Release: 183/183 passeddotnet format style --verify-no-changes: cleandotnet formatHusky hooks passed on commitNotes
No
Closes #387here -- per AGENTS.md "Branching Model", closing keywords belong in thedevelop -> mainpromotion PR. #387's other three findings (inline pragmas,GlobalUsings.cs, stale package source) were already resolved in earlier work and need no change.A fleet-wide issue is being filed against ProjectTemplate, since LanguageTags carries the identical
dotnet_analyzer_diagnosticline and the other repos have not been probed.