From 33f9c3828afefa5217899165e9960f109abf99f1 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Fri, 31 Jul 2026 15:39:25 -0400 Subject: [PATCH] feat(nsubstitute): milestone 5 phase 2 - test suites and verification Adds Compono.NSubstitute.Tests (IsSubstitutable/NSubstituteProvider/ UseNSubstitute coverage, shared-substitute reuse via CompositionRow, and a public-API-surface approval test) and a real packaged end-to-end run through Compono.XunitV3.SampleTests proving the plan's own Goal scenario compiles and passes under a real xUnit v3 runner. Writing this phase's own regression coverage caught two real bugs before they shipped further: - NSubstituteProvider.IsSubstitutable misclassified System.Delegate/ System.MulticastDelegate themselves as substitutable abstract classes - ADR-0025 Amendment 1's stated intent was only half-enforced. - Compono.Generators' root-level CMP0003 check still hard-rejected an interface/abstract-class/delegate type used as a composed root, a stale pre-Milestone-5 assumption that broke this plan's own Goal snippet when actually compiled. Loosened per ADR-0024 Amendment 2, with generator regression coverage proving concrete-type roots are unaffected. Co-Authored-By: Claude Sonnet 5 --- Compono.slnx | 1 + Directory.Packages.props | 4 + ...024-public-provider-extensibility-model.md | 61 +++++++ docs/mvp.md | 16 +- ...005-milestone-5-nsubstitute-integration.md | 82 ++++++++- docs/public-api.md | 9 +- .../Discovery/LeafTypeClassifier.cs | 54 +++--- .../NSubstituteProvider.cs | 12 +- .../CompositionPlanVerifyTests.cs | 169 +++++++++++------- ...nostic_AfterRootProviderCheck.verified.txt | 17 -- ...bstractType_ReportsDiagnostic.verified.txt | 17 -- ...OnInterface_ReportsDiagnostic.verified.txt | 17 -- ...nostic_AfterRootProviderCheck.verified.txt | 17 ++ ...elegateType_ReportsDiagnostic.verified.txt | 17 -- .../Compono.NSubstitute.Tests.csproj | 23 +++ .../CompositionBuilderExtensionsTests.cs | 54 ++++++ .../EndToEndCompositionTests.cs | 52 ++++++ .../IsSubstitutableTests.cs | 75 ++++++++ .../NSubstituteProviderTests.cs | 107 +++++++++++ .../PublicApiSurfaceTests.cs | 34 ++++ .../xunit.runner.json | 3 + .../Compono.XunitV3.SampleTests.csproj | 5 +- .../NSubstituteTests.cs | 49 +++++ .../pack-to-local-feed.sh | 14 +- 24 files changed, 729 insertions(+), 180 deletions(-) delete mode 100644 test/Compono.Generators.Tests/Snapshots/CompositionPlanVerifyTests.AbstractRootType_StillReportsDiagnostic_AfterRootProviderCheck.verified.txt delete mode 100644 test/Compono.Generators.Tests/Snapshots/CompositionPlanVerifyTests.AbstractType_ReportsDiagnostic.verified.txt delete mode 100644 test/Compono.Generators.Tests/Snapshots/CompositionPlanVerifyTests.ComposableAttributeOnInterface_ReportsDiagnostic.verified.txt create mode 100644 test/Compono.Generators.Tests/Snapshots/CompositionPlanVerifyTests.ConcreteRootTypeWithNoAccessibleConstructor_StillReportsDiagnostic_AfterRootProviderCheck.verified.txt delete mode 100644 test/Compono.Generators.Tests/Snapshots/CompositionPlanVerifyTests.DelegateType_ReportsDiagnostic.verified.txt create mode 100644 test/Compono.NSubstitute.Tests/Compono.NSubstitute.Tests.csproj create mode 100644 test/Compono.NSubstitute.Tests/CompositionBuilderExtensionsTests.cs create mode 100644 test/Compono.NSubstitute.Tests/EndToEndCompositionTests.cs create mode 100644 test/Compono.NSubstitute.Tests/IsSubstitutableTests.cs create mode 100644 test/Compono.NSubstitute.Tests/NSubstituteProviderTests.cs create mode 100644 test/Compono.NSubstitute.Tests/PublicApiSurfaceTests.cs create mode 100644 test/Compono.NSubstitute.Tests/xunit.runner.json create mode 100644 test/Compono.XunitV3.SampleTests/NSubstituteTests.cs diff --git a/Compono.slnx b/Compono.slnx index 5ec5559..81177bd 100644 --- a/Compono.slnx +++ b/Compono.slnx @@ -9,6 +9,7 @@ + diff --git a/Directory.Packages.props b/Directory.Packages.props index bc1e30e..1a3f913 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -20,6 +20,10 @@ feed populated by dotnet pack (PLAN-0004 Phase 3), never a ProjectReference. The version here must match the -p:Version passed to that project's PackToLocalFeed pre-restore target. --> + + + true + true + + + + + + + + + + + diff --git a/test/Compono.NSubstitute.Tests/CompositionBuilderExtensionsTests.cs b/test/Compono.NSubstitute.Tests/CompositionBuilderExtensionsTests.cs new file mode 100644 index 0000000..e536620 --- /dev/null +++ b/test/Compono.NSubstitute.Tests/CompositionBuilderExtensionsTests.cs @@ -0,0 +1,54 @@ +using NSubstitute.Core; + +namespace Compono.NSubstitute.Tests; + +/// +/// 's UseNSubstitute()/UseNSubstitute(Action{NSubstituteOptions}) +/// wiring a working into a real - PLAN-0005 +/// Phase 2. +/// +public sealed class CompositionBuilderExtensionsTests +{ + [Fact] + public void UseNSubstitute_WithNoConfiguration_ComposesAnInterfaceAsARealSubstitute() + { + var composer = Composer.Create(builder => builder.UseNSubstitute()); + + var value = composer.Create(); + + value.Should().BeAssignableTo(); + } + + [Fact] + public void UseNSubstitute_WithNoConfiguration_ComposesAnUnsealedAbstractClassAsARealSubstitute() + { + // Default NSubstituteOptions.SubstituteAbstractClasses is true. + var composer = Composer.Create(builder => builder.UseNSubstitute()); + + var value = composer.Create(); + + value.Should().BeAssignableTo(); + } + + [Fact] + public void UseNSubstitute_Configured_AppliesTheSuppliedOptions() + { + var composer = Composer.Create(builder => builder.UseNSubstitute(options => options.SubstituteAbstractClasses = false)); + + var act = () => composer.Create(); + + act.Should().Throw().WithMessage("*SampleAbstractService*"); + } + + [Fact] + public void UseNSubstitute_Configure_ThrowsForANullDelegate() + { + var act = () => Composer.Create(builder => builder.UseNSubstitute(null!)); + + act.Should().Throw(); + } + + public interface ISampleService; + + public abstract class SampleAbstractService; +} diff --git a/test/Compono.NSubstitute.Tests/EndToEndCompositionTests.cs b/test/Compono.NSubstitute.Tests/EndToEndCompositionTests.cs new file mode 100644 index 0000000..3a6ccb7 --- /dev/null +++ b/test/Compono.NSubstitute.Tests/EndToEndCompositionTests.cs @@ -0,0 +1,52 @@ +using NSubstitute.Core; + +namespace Compono.NSubstitute.Tests; + +/// +/// End-to-end composition through a real /, +/// proving UseNSubstitute()'s registration against the whole +/// pipeline rather than in isolation - PLAN-0005 Phase 2. The interface/delegate/abstract-class +/// positive and negative shapes are already covered by and +/// ; this file covers the one scenario those don't: +/// this plan's own Goal-section shared-substitute-reuse shape, run against the real +/// rather than a hand-written fake (Phase 0's +/// CompositionValueProviderTests.SharedRequest_SatisfiedByAPublicProvider_IsReusedByALaterOrdinaryRequest +/// already covers the general mechanism with a fake). +/// +public sealed class EndToEndCompositionTests +{ + [Fact] + public void SharedSubstitute_EstablishedOnARow_IsReusedByALaterOrdinaryRequestForTheSameType() + { + var composer = Composer.Create(builder => builder.UseNSubstitute()); + var row = composer.CreateRow(typeof(EndToEndCompositionTests)); + + // The [Shared] test-method-parameter request - what Compono.XunitV3's [Shared] attribute + // itself compiles down to. + var sharedDescriptor = new CompositionRequestDescriptor( + CompositionRequestKind.TestParameter, ordinal: 0, name: "repository", declaringType: typeof(EndToEndCompositionTests), Nullability.NotNullable); + var repositoryFromSharedRequest = row.ResolveShared(sharedDescriptor); + + // An ordinary, unmarked nested constructor-parameter request for the same type - what a SUT's + // own generated plan compiles down to when it needs an IRepository - reuses the shared value + // transparently (ADR-0021), with no [Shared] marker of its own. + var nestedDescriptor = new CompositionRequestDescriptor( + CompositionRequestKind.ConstructorParameter, ordinal: 0, name: "repository", declaringType: typeof(Handler), Nullability.NotNullable); + var repositoryFromNestedRequest = row.Resolve(nestedDescriptor); + + repositoryFromNestedRequest.Should().BeSameAs(repositoryFromSharedRequest); + repositoryFromSharedRequest.Should().BeAssignableTo(); + } + + public interface IRepository; + + public sealed class Handler + { + public Handler(IRepository repository) + { + Repository = repository; + } + + public IRepository Repository { get; } + } +} diff --git a/test/Compono.NSubstitute.Tests/IsSubstitutableTests.cs b/test/Compono.NSubstitute.Tests/IsSubstitutableTests.cs new file mode 100644 index 0000000..7277b44 --- /dev/null +++ b/test/Compono.NSubstitute.Tests/IsSubstitutableTests.cs @@ -0,0 +1,75 @@ +namespace Compono.NSubstitute.Tests; + +/// +/// unit coverage - PLAN-0005 Phase 2. See +/// docs/adr/0025-compono-nsubstitute-package-design.md (Amendment 1 for the +/// / distinction this file's negative cases +/// exist to lock). +/// +public sealed class IsSubstitutableTests +{ + [Theory] + [InlineData(typeof(ISampleInterface))] + [InlineData(typeof(SampleDelegate))] + [InlineData(typeof(SampleAbstractClass))] + public void IsSubstitutable_ReturnsTrue_ForASubstitutableShape_WithAbstractClassesAllowed(Type requestedType) + { + var result = NSubstituteProvider.IsSubstitutable(requestedType, substituteAbstractClasses: true); + + result.Should().BeTrue(); + } + + [Theory] + [InlineData(typeof(ISampleInterface))] + [InlineData(typeof(SampleDelegate))] + public void IsSubstitutable_ReturnsTrue_ForAnInterfaceOrDelegate_RegardlessOfTheAbstractClassOption(Type requestedType) + { + var result = NSubstituteProvider.IsSubstitutable(requestedType, substituteAbstractClasses: false); + + result.Should().BeTrue(); + } + + [Fact] + public void IsSubstitutable_ReturnsFalse_ForAnUnsealedAbstractClass_WhenTheOptionDisallowsIt() + { + var result = NSubstituteProvider.IsSubstitutable(typeof(SampleAbstractClass), substituteAbstractClasses: false); + + result.Should().BeFalse(); + } + + [Theory] + [InlineData(typeof(SampleSealedClass))] + [InlineData(typeof(SampleStruct))] + [InlineData(typeof(string))] + public void IsSubstitutable_ReturnsFalse_ForANonSubstitutableShape(Type requestedType) + { + var resultWithAbstractClassesAllowed = NSubstituteProvider.IsSubstitutable(requestedType, substituteAbstractClasses: true); + var resultWithAbstractClassesDisallowed = NSubstituteProvider.IsSubstitutable(requestedType, substituteAbstractClasses: false); + + resultWithAbstractClassesAllowed.Should().BeFalse(); + resultWithAbstractClassesDisallowed.Should().BeFalse(); + } + + // ADR-0025 Amendment 1 regression: IsSubclassOf(typeof(MulticastDelegate)) must not treat the + // framework base types themselves as a substitutable "delegate type" - only a real, concrete + // delegate type (SampleDelegate above) qualifies. + [Theory] + [InlineData(typeof(Delegate))] + [InlineData(typeof(MulticastDelegate))] + public void IsSubstitutable_ReturnsFalse_ForTheDelegateFrameworkBaseTypesThemselves(Type requestedType) + { + var result = NSubstituteProvider.IsSubstitutable(requestedType, substituteAbstractClasses: true); + + result.Should().BeFalse(); + } + + private interface ISampleInterface; + + private delegate void SampleDelegate(); + + private abstract class SampleAbstractClass; + + private sealed class SampleSealedClass; + + private struct SampleStruct; +} diff --git a/test/Compono.NSubstitute.Tests/NSubstituteProviderTests.cs b/test/Compono.NSubstitute.Tests/NSubstituteProviderTests.cs new file mode 100644 index 0000000..281c4d6 --- /dev/null +++ b/test/Compono.NSubstitute.Tests/NSubstituteProviderTests.cs @@ -0,0 +1,107 @@ +using NSubstitute.Core; + +namespace Compono.NSubstitute.Tests; + +/// +/// unit coverage, exercised through a real +/// ('s Value/IsHandled are +/// internal to Compono, so a provider's own outcome is only observable from outside through the +/// pipeline it feeds) - PLAN-0005 Phase 2. See +/// docs/adr/0025-compono-nsubstitute-package-design.md. +/// +public sealed class NSubstituteProviderTests +{ + [Fact] + public void TryProvide_ProducesARealNSubstituteSubstitute_ForAnInterface() + { + var composer = Composer.Create(builder => builder.AddTestDoubleProvider(new NSubstituteProvider(new NSubstituteOptions()))); + + var value = composer.Create(); + + AssertIsARealSubstitute(value); + } + + [Fact] + public void TryProvide_ProducesARealNSubstituteSubstitute_ForADelegate() + { + var composer = Composer.Create(builder => builder.AddTestDoubleProvider(new NSubstituteProvider(new NSubstituteOptions()))); + + var value = composer.Create(); + + AssertIsARealSubstitute(value); + } + + [Fact] + public void TryProvide_ProducesARealNSubstituteSubstitute_ForAnUnsealedAbstractClass_WhenTheOptionAllowsIt() + { + var composer = Composer.Create(builder => builder.AddTestDoubleProvider(new NSubstituteProvider(new NSubstituteOptions { SubstituteAbstractClasses = true }))); + + var value = composer.Create(); + + AssertIsARealSubstitute(value); + } + + [Fact] + public void TryProvide_DoesNotHandle_AnUnsealedAbstractClass_WhenTheOptionDisallowsIt() + { + var composer = Composer.Create(builder => builder.AddTestDoubleProvider(new NSubstituteProvider(new NSubstituteOptions { SubstituteAbstractClasses = false }))); + + var act = () => composer.Create(); + + act.Should().Throw().WithMessage("*SampleAbstractService*"); + } + + [Fact] + public void TryProvide_DoesNotHandle_ASealedClass() + { + var composer = Composer.Create(builder => builder.AddTestDoubleProvider(new NSubstituteProvider(new NSubstituteOptions()))); + + var act = () => composer.Create(); + + act.Should().Throw(); + } + + // string isn't re-verified here: Compono's built-in PrimitiveValueProvider (stage 7) always + // satisfies a bare string request regardless of what NSubstituteProvider does with it, so a + // composer.Create() call can't observe NSubstituteProvider's own NotHandled outcome the + // way SampleSealedService's above can. IsSubstitutableTests.IsSubstitutable_ReturnsFalse_ForANonSubstitutableShape + // already covers string directly against the static check this provider's TryProvide delegates to. + + [Fact] + public void Constructor_SnapshotsOptions_SoMutatingTheOriginalInstanceAfterwardHasNoEffect() + { + // ADR-0025 Amendment 1's mutation regression: the provider must snapshot + // NSubstituteOptions.SubstituteAbstractClasses at construction, not hold the NSubstituteOptions + // reference itself - proving the snapshot, not just that it compiles. + var options = new NSubstituteOptions { SubstituteAbstractClasses = true }; + var provider = new NSubstituteProvider(options); + var composer = Composer.Create(builder => builder.AddTestDoubleProvider(provider)); + + options.SubstituteAbstractClasses = false; + + var value = composer.Create(); + + AssertIsARealSubstitute(value); + } + + // NSubstitute's own "is this a substitute" check, not a hand-rolled heuristic (PLAN-0005 Phase 2's + // own task wording). `is ICallRouterProvider` only holds for an interface/abstract-class proxy - a + // delegate substitute is a plain delegate instance whose target isn't the proxy itself, so + // GetCallRouterFor (the same lookup NSubstitute's own Received()/Returns() extensions use + // internally) is the one check that works uniformly across every substitutable shape this provider + // supports. + private static void AssertIsARealSubstitute(object? value) + { + var act = () => SubstitutionContext.Current.GetCallRouterFor(value!); + + act.Should().NotThrow(); + } + + public interface ISampleService; + + public delegate void SampleDelegate(); + + public abstract class SampleAbstractService; + + public sealed class SampleSealedService; +} diff --git a/test/Compono.NSubstitute.Tests/PublicApiSurfaceTests.cs b/test/Compono.NSubstitute.Tests/PublicApiSurfaceTests.cs new file mode 100644 index 0000000..0dfb5fe --- /dev/null +++ b/test/Compono.NSubstitute.Tests/PublicApiSurfaceTests.cs @@ -0,0 +1,34 @@ +namespace Compono.NSubstitute.Tests; + +// Cheap insurance against accidental public-API drift (PLAN-0005 Phase 2's plan task) - locks the +// exact set of public types Compono.NSubstitute exposes, matching Compono.XunitV3.Tests' +// PublicApiSurfaceTests pattern (PLAN-0004 Phase 3). +public sealed class PublicApiSurfaceTests +{ + [Fact] + public void Assembly_ExposesExactlyTheDocumentedPublicTypes() + { + // IsPublic alone misses an accidentally-added nested public type (PR #26 review, fourth + // round): a nested type is never IsPublic (only a top-level type can be), it's IsNestedPublic + // instead - reflection's own naming split, not a synonym. + // + // The Name.Contains('<') exclusion is new here versus Compono.XunitV3.Tests' version of this + // test: CompositionBuilderExtensions uses C# 14 extension-block syntax + // (coding-standards.md), which the compiler lowers into its own nested marker + // type(s) - one of which reflection reports as IsNestedPublic=true despite carrying no + // CompilerGeneratedAttribute to filter on, since it's part of the extension block's own + // metadata encoding rather than an ordinary compiler-synthesized closure. '<'/'>' are + // otherwise illegal in a C# identifier, so this exclusion can never accidentally hide a real, + // hand-declared public type. + var publicTypeNames = typeof(NSubstituteProvider).Assembly.GetTypes() + .Where(static type => (type.IsPublic || type.IsNestedPublic) && !type.Name.Contains('<')) + .Select(static type => type.FullName); + + publicTypeNames.Should().BeEquivalentTo( + [ + "Compono.NSubstituteProvider", + "Compono.NSubstituteOptions", + "Compono.CompositionBuilderExtensions", + ]); + } +} diff --git a/test/Compono.NSubstitute.Tests/xunit.runner.json b/test/Compono.NSubstitute.Tests/xunit.runner.json new file mode 100644 index 0000000..86c7ea0 --- /dev/null +++ b/test/Compono.NSubstitute.Tests/xunit.runner.json @@ -0,0 +1,3 @@ +{ + "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json" +} diff --git a/test/Compono.XunitV3.SampleTests/Compono.XunitV3.SampleTests.csproj b/test/Compono.XunitV3.SampleTests/Compono.XunitV3.SampleTests.csproj index c1471a0..942c16c 100644 --- a/test/Compono.XunitV3.SampleTests/Compono.XunitV3.SampleTests.csproj +++ b/test/Compono.XunitV3.SampleTests/Compono.XunitV3.SampleTests.csproj @@ -52,6 +52,9 @@ + + - + diff --git a/test/Compono.XunitV3.SampleTests/NSubstituteTests.cs b/test/Compono.XunitV3.SampleTests/NSubstituteTests.cs new file mode 100644 index 0000000..72199b7 --- /dev/null +++ b/test/Compono.XunitV3.SampleTests/NSubstituteTests.cs @@ -0,0 +1,49 @@ +namespace Compono.XunitV3.SampleTests; + +// PLAN-0005 Phase 2's real-runner proof: composing through the actual packaged +// Compono.NSubstitute -> Compono dependency chain under a real xUnit v3 runner, not just +// Compono.NSubstitute.Tests' own direct Composer calls - matching PLAN-0004 Phase 3's real-packaged- +// consumer testing strategy (see this project's pack-to-local-feed.sh). +public interface IOrderRepository +{ + Task SaveAsync(Order order, CancellationToken cancellationToken); +} + +public sealed record Order; + +public sealed record PlaceOrder(string CustomerName, int Quantity); + +public sealed class CreateOrderHandler +{ + public CreateOrderHandler(IOrderRepository repository) + { + Repository = repository; + } + + public IOrderRepository Repository { get; } + + public Task Handle(PlaceOrder command) => Repository.SaveAsync(new Order(), CancellationToken.None); +} + +// Applies UseNSubstitute() to this row's own CompositionBuilder, exactly like an application's +// Program.cs would (coding-standards.md's application-level-wiring rule) - reached only through +// NSubstituteTests.Saves_order's own [Compose] theory parameter. +public sealed class NSubstituteTestProfile : ICompositionProfile +{ + public void Configure(CompositionBuilder builder) => builder.UseNSubstitute(); +} + +public sealed class NSubstituteTests +{ + // This plan's own Goal-section example, run for real: repository is a real NSubstitute + // substitute, reused as the exact same instance inside handler's own composed IOrderRepository + // constructor parameter, with no manual Substitute.For() call anywhere in this test. + [Theory] + [Compose] + public async Task Saves_order([Shared] IOrderRepository repository, CreateOrderHandler handler, PlaceOrder command) + { + await handler.Handle(command); + + await repository.Received(1).SaveAsync(Arg.Any(), Arg.Any()); + } +} diff --git a/test/Compono.XunitV3.SampleTests/pack-to-local-feed.sh b/test/Compono.XunitV3.SampleTests/pack-to-local-feed.sh index 3bf6f8b..8a075ee 100755 --- a/test/Compono.XunitV3.SampleTests/pack-to-local-feed.sh +++ b/test/Compono.XunitV3.SampleTests/pack-to-local-feed.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash -# Packs Compono and Compono.XunitV3 into the local NuGet feed this project restores against -# (test/Compono.XunitV3.SampleTests/nuget.config), serialized behind a cross-process lock, and clears -# this restore's own isolated packages path before every pack. +# Packs Compono, Compono.XunitV3, and Compono.NSubstitute into the local NuGet feed this project +# restores against (test/Compono.XunitV3.SampleTests/nuget.config), serialized behind a cross-process +# lock, and clears this restore's own isolated packages path before every pack. # # Why the lock: this script is invoked from a PackToLocalFeed MSBuild target # (BeforeTargets="Restore") on every restore of this project. CI (and RealRunnerTests, which shells @@ -32,9 +32,10 @@ set -euo pipefail compono_csproj="$1" xunitv3_csproj="$2" -feed_dir="$3" -configuration="$4" -restore_packages_path="$5" +nsubstitute_csproj="$3" +feed_dir="$4" +configuration="$5" +restore_packages_path="$6" lock_dir="$feed_dir/.pack.lock" @@ -64,3 +65,4 @@ rm -rf "$restore_packages_path" dotnet pack "$compono_csproj" -c "$configuration" -o "$feed_dir" -p:Version=1.0.0 --nologo dotnet pack "$xunitv3_csproj" -c "$configuration" -o "$feed_dir" -p:Version=1.0.0 --nologo +dotnet pack "$nsubstitute_csproj" -c "$configuration" -o "$feed_dir" -p:Version=1.0.0 --nologo