Repository navigation
chore: format the three drifted test files and check formatting in CI - #89
Conversation
`dotnet format --verify-no-changes` has been failing on `main`: import ordering in three test files, plus a trailing comment in UdsTransferTests that the formatter wanted re-indented. Nothing enforced it, so it drifted -- and three separate agents reported it in a row while working on unrelated issues, each having to decide again that it was pre-existing and not theirs. Two of the fixes are the formatter's own output. The third is not: aligning the continuation of a trailing comment pushed it to column 66 and well past the line width, which is worse to read than what it replaced. The comment is moved above the argument it describes instead, which the formatter accepts and which reads better. Nothing in .editorconfig was relaxed to make any of this pass. The new `format` job is deliberately not part of build-test. The three OS legs are the required status checks on `main`, and a misplaced blank line has no business blocking a merge -- this reports rather than gates. If that turns out to be too weak, promoting it to a required check is a ruleset change, not a workflow change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR SummaryLow Risk Overview Windows Three test files are brought in line with the formatter: Reviewed by Cursor Bugbot for commit af57229. Bugbot is set up for automated code reviews on this repo. Configure here. |
Two problems appeared once the net48 leg existed, both visible on every
Windows run:
WARNING: Overwriting results file: artifacts\test-results\test-results.trx
Data collection : Unable to find a datacollector with friendly name
'XPlat Code Coverage'.
The first is a real loss: both legs wrote the same LogFileName, so net48
overwrote net10.0 and the uploaded Windows test-results artifact held only
half the run.
The second is noise, but the kind that reads like a broken build.
coverlet.collector 10.x ships no .NET Framework assets, so it is referenced
for net10.0 only (tests/Directory.Build.props). Passing --collect to a
solution-wide run asked the net48 leg for a collector that cannot be there.
Coverage was never actually lost -- the net10.0 leg collected it as always.
Splitting the invocation gives each leg its own trx and puts --collect only
where a collector exists. TestTfmsInParallel is already false, so the legs
ran back to back before and still do; ordering is unchanged.
Verified: `dotnet test --framework net10.0` passes 421/421 locally, and
`dotnet format --verify-no-changes` is clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex found that `CONTRIBUTING.md`'s central release claim is false, and I had
copied it into CLAUDE.md unverified. Checked against the history rather than
argued about:
$ git log --first-parent --oneline origin/main | head -3
52834b2 Merge pull request #91 from dborgards/claude/sort-open-issues-v9a7kk
bd729e3 Merge pull request #90 from dborgards/test/dispose-hygiene
c2712ba Merge pull request #89 from dborgards/chore/format-and-gate
Every one has two parents. Nothing is squash-merged; every branch commit is
retained, and semantic-release analyses all of them. So the pull-request title
is not "the commit semantic-release reads" -- the branch commits are.
The practical inversions:
* a `docs:`-titled pull request containing one `feat:` commit publishes a minor
release, which the old guidance would have called impossible;
* a `BREAKING CHANGE:` footer belongs in the commit message, where the analyser
reads it. Putting it only in the pull-request body -- which is what I did on
#93, and told the author was necessary -- does nothing. No harm there, since
the commit carried it too, but the reasoning was wrong.
Both documents now describe the observed behaviour. CONTRIBUTING.md is
corrected at the source rather than left contradicting the file derived from
it; that is one file beyond this pull request's nominal scope, and leaving the
original assertion standing would have been worse than the scope creep.
Neither document decides the strategy. If the intent is squash merging, that
belongs in the repository settings, and then both texts want revisiting --
noted in CLAUDE.md for the owner rather than assumed either way.
Also from the same review: the packaging gate used a fixed `/tmp/nupkgs`.
Re-running it leaves earlier artifacts in place, and since `verify-packages.py`
checks whatever `*.nupkg` it finds, a change that stops producing a package
could pass on the stale copy. Now packs into a fresh `mktemp -d`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
Both side findings from #87.
1.
dotnet formatfails onmainReproduced before touching anything: import ordering in
Nfr006ErrorArchitectureTests.cs,UdsClientTests.csandUdsTransferTests.cs, plus one whitespace complaint. Nothing enforced formatting, so it drifted — and three agents in a row reported it while working on unrelated issues, each spending effort deciding it was pre-existing and not theirs.Two fixes are simply the formatter's output. The third is not, and it is worth a look:
dotnet formatwanted the continuation lines aligned under the trailing comment, at column 66 and well past the line width — technically consistent, materially worse to read. Moving the comment above the argument satisfies the formatter and reads better. No.editorconfigrule was relaxed to make anything pass.2. The stale pragma in
ControllableBus.csAlready fixed in #87, which started raising the event that
#pragma warning disable CS0067 // Never raisedclaimed nobody listened to. Nothing left to do.The new
formatjobdotnet format --verify-no-changesnow runs in CI as its own job, deliberately not insidebuild-test. The three OS legs are the required status checks onmain; a misplaced blank line has no business blocking a merge. This reports, it does not gate. If that proves too weak, promoting it is a ruleset change rather than a workflow change.Verified
dotnet format CanKit.Pro.sln --verify-no-changes→ exit 0dotnet test CanKit.Pro.sln -c Release→ 412 passed, 0 failed🤖 Generated with Claude Code