BrighterCommand / BrighterCommand/Brighter

Documentation examples are unguarded: add a fixture that compiles and runs the fluent-builder snippets

Open
#4,307 3 comments 0 reactions 1 assignee Claimed by @iancooper View on GitHub
1 - Up Next Documentation Maintenance
Dominant language
C#
Stars
2.5k
Forks
296
Avg merge
1d 11h
Merged PRs (30d)
21

Description

Raised out of the review discussion on #4302, which repaired seven `CommandProcessorBuilder.With()` sites across two files. The repairs are done; this issue is about the reason they were needed.

## The gap

`CommandProcessorBuilder` and `DispatchBuilder` are well covered — `StartNew()` is called from 15 files, 12 of them tests, plus `ControlBusReceiverBuilder` and `ControlBusSenderFactory` in `src/`. That coverage is why renaming `INeedPolicy` to `INeedResilience` did not break the build.

It also did not break the **documentation**, which is the only unguarded consumer of the API:

- `docs/guides/paramore_brighter_core_guide.md`
- `src/Paramore.Brighter.MessagingGateway.MsSql/README.md`

Both drifted silently through at least one interface rename. Nothing in CI reads them, so the next rename reopens #4301.

## Why compiling is not enough

#4302 compiled its snippets before proposing them, against both the released packages and `master`. Two lines still shipped a runtime failure, because a compile proves the *type* is satisfied and says nothing about a downcast, a null check, or a default that selects a code path:

- `MessagingGateway.MsSql/ChannelFactory.cs:46` downcasts `Subscription` to `MsSqlSubscription` and throws when the cast fails (repeated at `:65`, `:88`). A `Subscription` compiles and dies at `dispatcher.Receive()`.
- `Subscription` defaults to `MessagePumpType.Proactor` (`Subscription.cs:291`), so `MessageMappers(registry, null, null, null)` reaches `Dispatcher.CreateConsumer`'s Proactor branch and throws.

Both were caught by review, not by the harness. A fixture that only builds would have missed both.

## Proposal

A test in `Paramore.Brighter.Core.Tests` that exercises the shapes the docs print:

1. the full `CommandProcessorBuilder` chain — `Handlers` → `DefaultResilience`/`Resilience` → `ExternalBus`/`NoExternalBus` → `NoInstrumentation` → `RequestContextFactory` → `RequestSchedulerFactory` → `Build` — asserting `Build()` returns non-null. This fails at **compile** time on the next `INeedPolicy`-style rename, which is the cheap half and catches most of the drift.
2. a `DispatchBuilder` case that reaches `Receive()` on an in-memory transport, so the pump actually selects a branch. This is the half that catches the two findings above.

Transport-specific shapes (the MsSql dispatcher) need a database, so they belong beside the existing MsSql integration tests rather than in Core.

The wider version — extracting fenced blocks from the markdown and building them, so a snippet cannot drift without CI noticing — is more work and worth discussing separately. Guarding the chains the docs actually print is most of the value for a fraction of the cost.

Happy to do the work; raising it separately so the fence is designed rather than smuggled into a docs PR.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.