Skip to content

test: make SourceLink and TestKit.Concurrency report coverage - #668

Merged
Chris-Wolfgang merged 3 commits into
mainfrom
test/coverage-every-test-assembly
Oct 12, 2026
Merged

Chris-Wolfgang merged 3 commits into
mainfrom
test/coverage-every-test-assembly

Conversation

@Chris-Wolfgang

@Chris-Wolfgang Chris-Wolfgang commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

Why

Stage 1 on the sync PR #667 fails with Wolfgang.Etl.Abstractions.Tests.SourceLink (no coverage data) and Wolfgang.Etl.TestKit.Tests.Concurrency (no coverage data) (run 36947263126). The log shows "Unable to find a datacollector with friendly name 'XPlat Code Coverage'" for both.

Cause and fix, per assembly

Assembly Cause Fix Coverage after (local)
Abstractions.Tests.SourceLink no coverlet.collector add collector 8.0.1 (same as the other net10.0-only test projects); rework the network probe (below) none → 100% (first measured at 75.2%)
TestKit.Tests.Concurrency no coverlet.collector add collector 8.0.1; run the scenarios in the normal sweep (below) none → 100% (first measured at 10.7%)

What the new rows exposed, and the test changes

TestKit.Tests.Concurrency/ConcurrencyTests.cs

  • Every test returned immediately unless COYOTE_REWRITTEN=1, so the PR sweep ran no scenario at all. RunSystematic now always runs Coyote's TestingEngine and sets .WithSystematicFuzzingEnabled(!rewritten). Un-rewritten, Coyote fuzzes timing without the false "Deadlock detected" that schedule control reports. Rewritten (the coyote workflow), it controls the schedule as before.
  • The race scenarios no longer branch on who wins. A straight-line DrainAsync returns (observed, fault) and the scenario asserts with Specification.Assert (fault is null or OperationCanceledException / ObjectDisposedException).
  • Three deterministic facts pin each outcome: never cancelled/disposed, pre-cancelled, pre-disposed.
  • Checked locally both ways:
    • un-rewritten: 5/5 pass;
    • after coyote rewrite rewrite.coyote.json with COYOTE_REWRITTEN=1: 5/5 pass, three runs;
    • baseline: the original file also passes when rewritten.

Abstractions.Tests.SourceLink/SourceLinkPdbTests.cs

  • Which lines ran depended on what raw.githubusercontent.com answered. The retry and classification logic is now ProbeAsync (injectable status source, TimeSpan pause), Classify and IsAcceptable. New deterministic tests cover 200/403/429/503/400, a persistent 404 (3 attempts), a 404 that clears on retry, HttpRequestException and TaskCanceledException, plus the acceptability table. Behaviour is the same as before:
    • pass on success or infra failures;
    • fail on any other 4xx;
    • fail a persistent 404 only in CI.
  • A probe URL can always be built: SourceLink substitutes the commit SHA on every build, local ones included (local PDB: https://raw.githubusercontent.com/Chris-Wolfgang/ETL-Abstractions/bdc6c85.../*). So "no mappings" and "no probe URL" are now assertion failures instead of early returns. The urlPrefix.Contains('*') branch could never be taken and was removed. BuildProbeUrl and ReadSourceLinkPayload are now LINQ queries.

Exclusions removed

None existed in these two assemblies. Tests.Unit still carries class-level [ExcludeFromCodeCoverage] on test doubles. That is out of scope here, because that assembly already reports a row (99.1%).

Verified locally

  • dotnet build -c Release: 0 warnings, 0 errors.
  • Ran Stage 1's per-project loop (dotnet test per test project per net5.0–net10.0 TFM with --collect:"XPlat Code Coverage" --settings coverlet.runsettings, then dotnet reportgenerator → Summary.txt). Every test assembly that ran has a row, and all 34 project/TFM runs pass. Test rows:
    • DocExamples 100%
    • Fuzz 99.1%
    • SourceLink 100%
    • Abstractions.Tests.Unit 99.1%
    • ErrorPolicies.Tests.Unit 100%
    • TestKit.Tests.Concurrency 100%
    • TestKit.Tests.DocExamples 99%
    • TestKit.Tests.Fuzz 100%
    • TestKit.Tests.Unit 99.7%
    • TestKit.Xunit.Tests.Unit 100%

Unblocks #667; part of #658.

Round 2 (2596884): every test assembly at 100% with no test-code exclusions

Fleet policy: every test assembly at 100%, and [ExcludeFromCodeCoverage] in tests only on a dedicated infrastructure-check method. This repo has no such method.

Exclusions removed / added

  • Removed: all 71 [ExcludeFromCodeCoverage] attributes in tests/. They sat on test doubles and helpers in Abstractions.Tests.Unit (BaseClassTests/, ConfigureAwaitContextTests, DisposedGuardTests, FinalizationSuppressionTests, ItemErrorHandlingTests, Performance/AllocationFreeTests, PipelineTests/), ErrorPolicies.Tests.Unit, TestKit.Tests.Unit (mutation tests, CountingProgressTimer, CultureSwapper) and TestKit.Xunit.Tests.Unit. The commit message says 70; that is a miscount, the diff removes 71. grep -rn ExcludeFromCodeCoverage tests now returns nothing.
  • Added: none. src/ is untouched, including its existing exclusions.

Test changes (for review)

All of these add tests or make mechanical changes. No existing assertion changes.

  • New BaseClassTests/StageRunner.cs. Runs a stage once through its progress overload and returns the items and reports. New *_when_run_produces_no_items_and_a_final_report_of_zero tests use it to run each double whose own tests only construct, configure or dispose it: GuardParity, OptionsConstructor, ConvenienceBaseOptionsConstructor, DisposedGuard, TimerSeam, BaseCoverage DisposeRecording, ItemErrorHandling, FinalizationSuppression. The runs reach the doubles' worker and CreateProgressReport.
  • New timer tests. ExtractorBaseTests, LoaderBaseTests and TransformerBaseTests each get one: the internal timer-injection constructor, with a ManualProgressTimer fired mid-run, gives reports [2, 3]. The loader test also reads ListLoader.LoadedItems, which nothing read before.
  • New positive-control tests:
    • an undisposed Finalizable* double is finalized (the control for the SuppressFinalize tests);
    • CountingSynchronizationContext counts and runs Post and Send (the control for the "posted nothing" assertions);
    • DualDisposableLoader.Dispose sets its flag when called directly.
  • New guard tests. OverloadDoubleCoverageTests calls the never-routed CancelOnly* guards directly and pins the WrongOverloadCalledException message.
  • Doubles and helpers pinned by new tests:
    • recording timers' Start / Dispose (TestKit mutation tests, *BaseCoverageTests; new CountingProgressTimerTests);
    • RecordingLogger.BeginScope (ErrorPolicies);
    • the helper iterators: EmptyIgnoring*, CancellingSkipSourceAsync, CancelOnFirst, CancelAt, CancellingSequence through its non-generic enumerator;
    • DisposeTrackingEnumerable non-generic Current / Reset;
    • RetryingExtractor on the progress path;
    • ParsingExtractor / CollectingLoader progress reports;
    • ConfigurableLoader / ConfigurableTransformer Abort rethrow;
    • CounterHarness.ExtractAsync, which throws NotSupportedException.
  • Fuzz: a new property, LoaderBase_progress_run_reports_every_loaded_item_in_its_final_report.
  • Mechanical changes:
    • GuardParityTests: the never-enumerated EmptyAsync() argument becomes AsyncEnumerable.Empty<int>(), and the helper is removed.
    • FaultyExtractorMutationTests: the local iterator in ExtractAsync_cancelling_during_the_skip_phase_throws_promptly moves to the static CancellingSkipSource(cts). The items and cancel point are the same, and a new test drains it.
    • TestKit.Tests.DocExamples: the per-example rot check moves into CheckExample(...). New tests cover a clean snippet and a snippet that is rot in every context (int value = "text"; → CS0029). That second test reaches failures.Add and best ?? ..., which no test reached before.
    • DelayingExtractorCancellationContractTests: both overrides now share one DrainAsync(items, onItem). It counts each item before the callback and sets canceled on OperationCanceledException, the same semantics as the two loops it replaces. A new test drives a run where cancelAfter is never reached, which covers normal completion.
  • Stale "cannot be covered" / "never called" comments on the doubles are updated.

Coverage before -> after (local, Linux-equivalent Stage 1 loop)

BEFORE = this branch at 7ac9d97 with its current coverlet.runsettings, net8.0+net10.0. AFTER = 2596884 with the #667 runsettings (repo-template#702), net5.0–net10.0. A config-only run (7ac9d97 + new runsettings) gave the same numbers as BEFORE, because this repo has no TestModels/*Fixture classes. Removing the exclusions before adding tests took Abstractions.Tests.Unit to 96.8%, ErrorPolicies.Tests.Unit to 96.3%, TestKit.Tests.Unit to 98.7% and TestKit.Xunit.Tests.Unit to 99.3%.

Assembly Before After
Abstractions.Tests.DocExamples 100% 100%
Abstractions.Tests.Fuzz 99.1% 100%
Abstractions.Tests.SourceLink 100% 100%
Abstractions.Tests.Unit 99.1% 100%
ErrorPolicies.Tests.Unit 100% 100%
TestKit.Tests.Concurrency 100% 100%
TestKit.Tests.DocExamples 99% 100%
TestKit.Tests.Fuzz 100% 100%
TestKit.Tests.Unit 99.7% 100%
TestKit.Xunit.Tests.Unit 100% 100%
src: Wolfgang.Etl.Abstractions 97.4% 97.4%
src: Wolfgang.Etl.ErrorPolicies 100% 100%
src: Wolfgang.Etl.TestKit 98.4% 98.4%
src: Wolfgang.Etl.TestKit.Xunit 98.3% 98.3%

Wolfgang.Etl.Abstractions.Tests.Concurrency (Coyote CLI, no Test SDK) and AotSmoke are not xunit test projects. Stage 1 does not run them, and they are not measured.

Verified locally

  • dotnet build -c Release: 0 warnings, 0 errors (all TFMs).
  • All 30 project/TFM runs in the net5.0–net10.0 coverage loop pass.
  • The four touched multi-TFM projects also pass on net462, net472, net48, net481 and netcoreapp3.1.
  • The finalizer and synchronization-context tests passed 10 of 10 repeated runs (net10.0 and net48).

Merge order

Merge this PR (#668) first, then the config PR #667.

🤖 Generated with Claude Code

Both assemblies ran tests in Stage 1 without coverlet.collector, so the
sync PR's (#667) gate failed them as "no coverage data". Add the
collector (8.0.1, as the other net10.0-only test projects).

That exposed what they would measure:

- TestKit.Tests.Concurrency (10.7%): every test returned early unless
  the assembly had been Coyote-rewritten, so the normal sweep ran
  nothing. Run the TestingEngine in both modes: systematic fuzzing when
  un-rewritten (no false deadlocks), schedule control when rewritten.
  The race scenarios hand their outcome to a straight-line DrainAsync and
  assert on it, and three deterministic facts pin each outcome the races
  accept. Verified both un-rewritten and after `coyote rewrite` with
  COYOTE_REWRITTEN=1.
- Abstractions.Tests.SourceLink (75.2%): the probe's branches depended on
  what GitHub answered. The retry/classification logic is now
  ProbeAsync + Classify + IsAcceptable with deterministic tests for every
  status, and the network test asserts the outcome. A probe URL is
  always buildable (SourceLink substitutes the SHA on every build), so a
  missing one now fails instead of returning.

Part of #658

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 2, 2026 02:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No blocking issues were identified in the collector additions or test refactors.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Adds coverage collection for SourceLink and TestKit concurrency tests, addressing the missing coverage rows exposed by #667 and supporting #658.

Changes:

  • Adds coverlet.collector to both test projects.
  • Runs concurrency scenarios without rewriting and adds deterministic outcome tests.
  • Refactors SourceLink probing for deterministic retry and status tests.
File Description
tests/​Wolfgang.Etl.TestKit.Tests.Concurrency/​Wolfgang.Etl.TestKit.Tests.Concurrency.csproj Enables coverage collection.
tests/​Wolfgang.Etl.TestKit.Tests.Concurrency/​ConcurrencyTests.cs Enables ordinary-sweep scenarios and tests deterministic outcomes.
tests/​Wolfgang.Etl.Abstractions.Tests.SourceLink/​Wolfgang.Etl.Abstractions.Tests.SourceLink.csproj Enables coverage collection.
tests/​Wolfgang.Etl.Abstractions.Tests.SourceLink/​SourceLinkPdbTests.cs Separates probe logic and adds deterministic tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/Wolfgang.Etl.TestKit.Tests.Concurrency/ConcurrencyTests.cs
…n tests

Remove all 70 [ExcludeFromCodeCoverage] attributes from test code (test
doubles and helpers in Abstractions.Tests.Unit, ErrorPolicies.Tests.Unit,
TestKit.Tests.Unit and TestKit.Xunit.Tests.Unit). The fleet policy allows the
attribute only on an infrastructure check; this repo has none. Cover what the
removal exposed, plus the pre-existing gaps, with new tests and mechanical
changes only. No existing assertion changes.

- New BaseClassTests/StageRunner: runs a stage once through its progress
  overload and returns the items and reports. New tests use it to run each
  double whose own tests only construct, configure or dispose it (worker and
  CreateProgressReport).
- Timer-injection constructors on SequenceExtractor / ListLoader /
  IdentityTransformer: one test each fires a ManualProgressTimer mid-run.
- Positive controls: the finalizer runs when a Finalizable* double is not
  disposed; CountingSynchronizationContext counts and runs Post and Send;
  DualDisposableLoader.Dispose sets its flag when called directly.
- Never-routed guards on the CancelOnly* doubles are called directly and pin
  the WrongOverloadCalledException message.
- Recording timers' Start/Dispose, RecordingLogger.BeginScope and the
  helper iterators (cancelling / empty sources) are pinned by new tests.
- Mechanical: GuardParityTests' never-enumerated EmptyAsync() becomes
  AsyncEnumerable.Empty<int>(); FaultyExtractorMutationTests' local iterator
  becomes the static CancellingSkipSource (same items, same cancel point);
  TestKit DocExamples' per-example check moves into CheckExample;
  DelayingExtractorCancellationContractTests' two drain loops share one
  DrainAsync(items, onItem) with the same counting and cancel semantics.
- Fuzz: new property that a LoaderBase progress run reports every loaded item.
- Stale "cannot be covered" comments updated.

Local, Stage 1 TFMs (net5.0-net10.0) with the repo-template#702 runsettings:
every test assembly 100%; Release build 0 warnings / 0 errors; the touched
projects also pass on net462/net472/net48/net481/netcoreapp3.1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Chris-Wolfgang added a commit that referenced this pull request Oct 12, 2026
#701)

#668 makes the TestKit ConcurrencyTests always
run: un-rewritten in systematic fuzzing mode in the ordinary test sweep, and
under controlled scheduling in this workflow with COYOTE_REWRITTEN=1. The
header and the systematic-run step comments still said the tests only run
when rewritten and that RunSystematic early-returns without the flag.

Comments only; no behaviour change.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants