test: execute the netstandard2.0 build on a net48 leg - #83
Conversation
The shipping libraries multi-target `netstandard2.0;net10.0`, but the suite targeted `net10.0` alone: the netstandard2.0 assembly was compiled on every CI run and then never executed. That is the blind spot the inverted `Queue.TryPeek` polyfill (FR-TP-013) shipped through, and SRS NFR-004 / CON-001 ask for a target framework matrix instead of a single leg. The suite now multi-targets `net10.0;net48`. `net48` resolves the netstandard2.0 asset of every project reference, so the same 403 tests run a second time against the assembly a .NET Framework consumer actually gets, pulling in the System.Memory / System.Threading.Channels / Microsoft.Bcl.AsyncInterfaces polyfills that only that build uses. The extra target framework is conditioned on the build *host*, not on the CI job: `net48` has no host to run on without Mono, so on Linux and macOS the property collapses to `net10.0` and `dotnet build` / `dotnet test` behave exactly as before. `-p:CanKitProTestNetFrameworkLeg=true` opts in anywhere, which is how the leg can be compile-checked away from Windows. No test needed an `#if NET` guard. The one .NET 6+ API the suite leans on, `Task.WaitAsync`, is polyfilled for the net48 leg in `System.Threading.Tasks` so that all 43 existing call sites — and any written later — compile unchanged on both legs. coverlet.collector 10.x has no .NET Framework build assets, so coverage stays a net10.0 concern; both legs run the same tests, so no line goes unmeasured. PublicApiSurfaceTests needed nothing either: it renders whichever asset its target framework resolved, so the net48 leg approves the netstandard2.0 surface against the same baselines for free. The two surfaces were verified identical. Closes #49 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR SummaryLow Risk Overview
A new Reviewed by Cursor Bugbot for commit 6058521. Bugbot is set up for automated code reviews on this repo. Configure here. |
`dotnet test` builds and runs a project's target frameworks in parallel, so adding the net48 leg quietly started a second test host alongside the net10.0 one — visible in the first CI run on this branch, where both legs reported "Test run for" in the same second and finished 15s apart rather than back to back. That is the contention xunit.runner.json already disables collection parallelism to avoid: this suite measures ISO-TP N_As/N_Bs/N_Cr, J1939 periodic send and DeadlineScheduler expiry, and a second full test host on a three-core runner turns those measurements into measurements of the runner. TestTfmsInParallel serialises the legs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes #49.
What was wrong
src/Directory.Build.propsmulti-targetsnetstandard2.0;net10.0;tests/Directory.Build.propstargetednet10.0alone. The netstandard2.0 assembly was compiled on every CI run and then thrown away — never loaded, never executed. SRS NFR-004 and CON-001 ask for a target-framework matrix, and the example they cite is exactly a netstandard2.0-only defect: the invertedQueue.TryPeekpolyfill (FR-TP-013). A repeat of that bug was invisible.What changed
The suite multi-targets
net10.0;net48.net48resolves thenetstandard2.0asset of every project reference, so the same 403 tests run a second time against the assembly a .NET Framework consumer actually gets — including the polyfill packages (System.Memory,System.Threading.Channels,Microsoft.Bcl.AsyncInterfaces) that only that build pulls in. Windows CI therefore runs 806 tests, Linux and macOS 403.Keeping Linux and macOS green
The extra target framework is conditioned on the build host, not on the CI job:
net48has no host to run on without Mono, so on Linux and macOS the property collapses tonet10.0anddotnet build/dotnet testbehave exactly as they did before — no change to.github/workflows/ci.ymlat all. Conditioning on the host rather than on a CI variable means a working copy behaves the way its CI leg does.CanKitProTestNetFrameworkLeg=trueis the opt-in for everyone else:dotnet build tests/CanKit.Pro.Tests -f net48 -p:CanKitProTestNetFrameworkLeg=truecompile-checks the leg from Linux or macOS. It exists because overridingTargetFrameworksdirectly does not work — as a global property it flows into the referenced shipping projects and retargets those too.Task.WaitAsyncThe only .NET 6+ API this suite depends on, across 43 call sites in seven files.
tests/CanKit.Pro.Tests/Infrastructure/TaskWaitAsyncPolyfill.cssupplies it for the net48 leg, under#if !NET, deliberately in theSystem.Threading.Tasksnamespace — every call site already has that using, so all 43 compile unchanged and a test written tomorrow in a file that has never heard of the class still builds on both legs. The observable contract matches the framework's:TimeoutExceptionon the timeout overload,OperationCanceledExceptionon the cancellation overload, and the awaited task is left running rather than cancelled.No test needed an
#if NETguard. The whole suite compiles and is expected to run on both legs.Running the two legs one at a time
dotnet testruns a project's target frameworks in parallel by default, which the first CI run on this branch showed plainly: both legs printed "Test run for" in the same second. That is the contentionxunit.runner.jsonalready disables collection parallelism to avoid — and worse, because the second leg is a whole second test host rather than another thread, on a suite that measures ISO-TP N_As/N_Bs/N_Cr, J1939 periodic send andDeadlineSchedulerexpiry.TestTfmsInParallel=falseserialises them; the Windows job now runs net10.0 to completion and then net48, at a cost of about a minute of wall clock and nothing on the other two legs.Two smaller consequences
net10.0only. A missing collector is a warning, not a failure (verified locally), and both legs run the same tests, so no line goes unmeasured.PublicApiSurfaceTestsnow approves the netstandard2.0 surface for free — this is the follow-on noted while doing API approvals: the rendering misses base types, sealed/abstract, defaults, ref kinds and nullability #50.Assembly.Loadresolves whichever asset the target framework was given, so on the net48 leg it renders the netstandard2.0 build against the same approval files. No second baseline was needed: I rendered all nine netstandard2.0 assemblies with PublicApiGenerator and diffed them against the checked-in approvals — byte-identical, all nine. That is the expected outcome (same sources, and PublicApiGenerator renders from metadata via Mono.Cecil rather than from the running framework's reflection), so a future divergence is a finding rather than a baseline that needs splitting per TFM. The<remarks>on the test now says so.Verification
dotnet build CanKit.Pro.sln -c Releaseon macOS: clean, 0 warnings.dotnet test CanKit.Pro.sln -c Releaseon macOS: 403 passed, 0 failed — unchanged frommain.dotnet build tests/CanKit.Pro.Tests -f net48 -c Release -p:CanKitProTestNetFrameworkLeg=trueon macOS: builds clean, 0 warnings. The SDK restores the .NET Framework reference assemblies, so the net48 leg is compile-verified off Windows; binding redirects (CanKit.Pro.Tests.dll.config) are generated.Not verifiable locally (macOS, no Mono): actually executing the net48 leg. Whether the netstandard2.0 code paths pass at runtime, and whether the timing-sensitive tests (ISO-TP N_As/N_Bs/N_Cr, STmin,
DeadlineScheduler) hold up under .NET Framework's coarser timers, is what thewindows-latestCI job decides.CI result
All three required checks pass. The Windows job now reports both legs:
806 tests on Windows, 403 on Linux and macOS. No netstandard2.0 defect was found: the netstandard2.0 build passes all 403 tests on the first run of it in this repository's history. That is a good result rather than a disappointing one — it is now a regression net rather than an unknown.
The
Data collection : Could not find data collector 'XPlat Code Coverage'line on the net48 leg is the expected, non-fatal consequence of coverlet having no .NET Framework assets.One thing this surfaced that is not mine to fix
The first run failed on two tests, both on the net10.0 leg, neither on net48, and both pre-existing:
UdsClientTests.TimedOut_Request_Does_Not_Poison_Next_Same_Service_Transaction— "Expected aUdsTimeoutExceptionto be thrown, but no exception was thrown." This is the same failure the CI run formain@41b99dc(the commit this branches from) hit on macOS, so it is a flake onmaintoday, not something this PR introduced.J1939NodeTests.StartPeriodicSend_SingleFrame_FiresAtConfiguredPeriod— median inter-arrival 197ms against a configured 120ms with a 84–192ms band.Both passed on re-run. They look like genuine runner-contention flakes in timing-sensitive tests, and with these three checks now required on
mainthey will block merges intermittently. Worth its own issue; deliberately not touched here.Notes for review
tests/plus one line ofCONTRIBUTING.md.test:— no release.tests/Directory.Build.propsconflict it would have caused is gone; this branch is straight offmain@41b99dc.test/assertions-that-can-fail) is still open and rewrites assertions inIsoTpChannelIntegrationTests,RawCanSubscriptionTests,J1939TpTestsand others. It should not conflict: this PR edits no test-case file except a doc comment inPublicApiSurfaceTests, which test: make the assertions that could not fail able to fail #81 does not touch. Whichever merges second simply gains a second leg for its assertions.🤖 Generated with Claude Code