Skip to content

test: render API approvals with PublicApiGenerator - #80

Merged
dborgards merged 1 commit into
mainfrom
test/api-approval-rendering
Sep 10, 2026
Merged

dborgards merged 1 commit into
mainfrom
test/api-approval-rendering

Conversation

@dborgards

Copy link
Copy Markdown
Owner

Closes #50.

Which route, and why

The issue offered two: (1) render the approvals with PublicApiGenerator, or (2) keep the hand-rolled renderer and add EnablePackageValidation against a 1.2.0 baseline. This PR takes route 1.

Route 1 closes all six reported gaps in one move — base types and implemented interfaces, sealed/abstract, parameter defaults, in/ref/out, attributes, nullability — because the generator emits compilable C# declarations rather than a summary of them. Hand-rolling those six onto the existing reflection walk would be more reflection code than a test should own, and each one is a place to get the corner cases subtly wrong.

The dependency fits the repo's conventions: it goes into Directory.Packages.props under the existing Test label (central package management is on), and the PackageReference in tests/Directory.Build.props carries PrivateAssets="all", so it is a test-only tool and never reaches a shipped .nupkg. Nothing about the workflow changes: same nine approval files, same .received.txt drop, same CI Show generated API approvals step and artifact upload.

Route 2 was not taken here, but it is not wrong — the two are complementary, as the issue says. Left as a follow-up because:

  • It needs the 1.2.0 packages from nuget.org at pack time, which makes the pack job network-dependent in a way it currently is not.
  • A pinned baseline has to be re-pinned by hand each release, or it silently stops meaning anything.
  • Its real extra value over route 1 is TFM consistency between netstandard2.0 and net10.0 — genuinely uncovered here, since the test only ever loads the net10.0 build. That is worth its own issue rather than being folded in behind an approval-rendering change.

What the approval diff shows

Large, as expected: +1087/−851 across the nine files, because the rendering is much richer. It is not a surface change. I compared the two renderings mechanically — extracting the set of public type names and member names from the old files and from the new ones, package by package — and the sets are identical apart from three differences, all of which are the new renderer seeing more than the old one:

  1. Operator overloads now appear. J1939Name, IsoTpEndpoint and Pci get operator == / operator !=. The old renderer filtered them out as IsSpecialName.
  2. Protected constructors now appear. IsoTpException, J1939NodeException, J1939TpException and UdsException each have a protected ctor taking a CanKitErrorCode. The old renderer's class doc claimed to cover "public/protected member signatures", but it passed BindingFlags.Public to GetConstructors, so protected ctors were invisible to it.
  3. Record-synthesized members collapse into the record keyword. J1939SpnDefinition loses its printed <Clone>$, Deconstruct, Equals, GetHashCode and ToString, and prints as public sealed record instead. TxConfirmation likewise loses Equals/GetHashCode/ToString. I set TreatRecordsAsClasses = false on purpose for this: the generator defaults to printing a record as a class, and record-ness is part of the promise (value equality, with, deconstruction), so turning one back into a class must fail the approval.

Everything else in the diff is the same members, rendered with the detail the issue asked for. A few examples of what is now visible and was not before:

- class CanKit.Pro.Actor.ProtocolActor
+ public sealed class ProtocolActor : CanKit.Pro.Actor.IProtocolActor, System.IDisposable
+     public ProtocolActor(ActorExecutionMode mode = 0, SynchronizationContext? synchronizationContext = null)

- method System.Boolean Matches(CanFrameView& frame)
+ public bool Matches(in CanKit.Abstractions.API.Can.Definitions.CanFrameView frame) { }

- prop System.Boolean Confirmed {get/set}
+ public bool Confirmed { get; init; }

That last one is worth calling out on its own: TxConfirmation's properties are init, not set. The old renderer reported CanWrite and printed {get/set}, so an init silently relaxed to a set would have passed. Same story for [System.Flags] on CanKit.Pro.CANopen's flags enum, which is now in the approval.

No behaviour change, no release

Test-only. test: maps to no release, and nothing in src/ is touched — the nine .approved.txt files are the record of the surface, not the surface.

Verification

  • dotnet build CanKit.Pro.sln -c Release — clean, 0 warnings.
  • dotnet test CanKit.Pro.sln -c Release — 403 passed, 0 failed.
  • dotnet format --verify-no-changes reports nothing on the touched file (the pre-existing violations in UdsTransferTests.cs, UdsClientTests.cs and Nfr006ErrorArchitectureTests.cs are untouched and predate this branch).

Two things the richer rendering exposed, neither fixed here

Both look like genuine API observations rather than rendering artefacts, and both feel like separate issues:

  1. ICanBusService.FindOverlappingFilterSubscriptions() returns a bare tuple pair. It renders as IReadOnlyList<ValueTuple<ISubscription, ISubscription>> plus a [return: TupleElementNames({"First", "Second"})] attribute — the source declares (ISubscription First, ISubscription Second). "First"/"Second" carry no meaning for an overlapping pair, the attribute is noise in the approval, and renaming tuple element names is a source-breaking change for callers who destructure by name. A small named readonly record struct would read better and be safe to evolve.

  2. Only the net10.0 build is approved. The test does Assembly.Load, so it sees whichever TFM the test project resolved — never netstandard2.0. The packages ship both. A netstandard2.0-only difference (a polyfilled overload, a #if that guards a member) would not be caught by this test at all. This is exactly the hole route 2's EnablePackageValidation fills, and is the strongest argument for doing it as a follow-up.

🤖 Generated with Claude Code

The hand-rolled renderer in PublicApiSurfaceTests printed a type's kind, its
name and its member signatures, and nothing else. Six kinds of breaking change
therefore passed the approval test unchanged: sealing a type or dropping
`abstract`, removing a base type or an implemented interface, changing or
deleting a parameter default, flipping an `in`/`ref`/`out` modifier, adding or
removing an attribute, and tightening a nullable annotation. All six break
consumers, and the approval is the only thing in the repository watching for
them.

Hand-rolling the missing six is more reflection than a test should own, so the
rendering now comes from PublicApiGenerator, which emits compilable C#
declarations and covers all of them at once. It is a test-only PackageReference
carrying PrivateAssets="all", so nothing reaches a shipped package. Two things
are excluded deliberately, both documented at the options object: assembly-level
attributes, which carry the version this build was handed from outside and would
fail the approval on every CI run, and the compiler's nullable bookkeeping
attributes, which the generator already renders as `?` on the signatures
themselves.

The approvals are regenerated. The diff is large because the rendering is much
richer, not because the surface moved: the set of public types and member names
is identical package by package, with three deliberate gains — operator
overloads, which the old renderer skipped as SpecialName; protected
constructors, which it never asked reflection for; and record-synthesized
members, which collapse into the `record` keyword now that records print as
records.

Closes #50

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Test and approval-baseline changes only; no production code paths or runtime behavior are modified.

Overview
Public API approval tests now use PublicApiGenerator instead of a custom reflection renderer, so baselines capture full C# surface detail (bases, interfaces, sealed/record, defaults, in/ref/out, attributes, nullability).

PublicApiSurfaceTests calls assembly.GeneratePublicApi with tuned options (no assembly version attributes, records not flattened to classes, compiler nullable attributes excluded). The nine ApiApprovals/*.approved.txt files are re-baselined to compilable declaration text—the diff is formatting and newly visible members (e.g. operators, protected exception ctors), not new shipped API in src/.

PublicApiGenerator 11.5.4 is added as a test-only central package (Directory.Packages.props, tests/Directory.Build.props with PrivateAssets="all"). The ApiApprovals README documents what the files represent.

Reviewed by Cursor Bugbot for commit 8f0e3d4. Bugbot is set up for automated code reviews on this repo. Configure here.

@dborgards
dborgards merged commit 41b99dc into main Sep 10, 2026
11 checks passed
@dborgards
dborgards deleted the test/api-approval-rendering branch September 10, 2026 19:49
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.

API approvals: the rendering misses base types, sealed/abstract, defaults, ref kinds and nullability

1 participant