diff --git a/.github/workflows/build-package.yaml b/.github/workflows/build-package.yaml index 2616dfd..6d0bbeb 100644 --- a/.github/workflows/build-package.yaml +++ b/.github/workflows/build-package.yaml @@ -31,9 +31,15 @@ jobs: with: fetch-depth: 0 + # Both SDKs are needed. global.json selects the 10.0 SDK to build with, and it can compile + # every target here — but running the net8.0 test projects needs the 8.0 runtime, which the + # 10.0 SDK does not carry. Roll-forward stops at the major version boundary by default, so a + # net8.0 test host will not start on 10.0 alone. - uses: actions/setup-dotnet@v4 with: - dotnet-version: 8.0.x + dotnet-version: | + 8.0.x + 10.0.x - run: dotnet restore DependencyModules.sln @@ -94,7 +100,9 @@ jobs: - uses: actions/setup-dotnet@v4 with: - dotnet-version: 8.0.x + dotnet-version: | + 8.0.x + 10.0.x source-url: https://nuget.pkg.github.com/ipjohnson/index.json env: NUGET_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 46f8e2b..8ecd5e7 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -27,9 +27,12 @@ jobs: with: fetch-depth: 0 + # See build-package.yaml: the 10.0 SDK builds, the 8.0 runtime runs the net8.0 tests. - uses: actions/setup-dotnet@v4 with: - dotnet-version: 8.0.x + dotnet-version: | + 8.0.x + 10.0.x - name: Resolve version id: version diff --git a/CHANGELOG.md b/CHANGELOG.md index 2269067..555d5fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,117 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +- **xUnit v3 updated from 1.0.0 to 3.2.2.** `[ModuleTest]` builds on xUnit's extensibility surface — + a custom test case and discoverer — and that surface moved across the two major versions. Module + tests now pick up the conditional-skip family the way `[Fact]` and `[Theory]` do: `SkipExceptions` + on the test case, and per-row `SkipType`/`SkipUnless`/`SkipWhen`/`Label` on a data row, each of + which previously had nowhere to go. + + `DependencyModules.xUnit` also now references `xunit.v3.extensibility.core` rather than + `xunit.v3`. It ships `[ModuleTest]` for other people's test projects and is not a test project + itself, which is exactly what that package is for. It replaces three defensive settings that + existed only to stop xunit.v3's build targets forcing this library to be an executable. + + **Breaking for anyone constructing `ModuleTestCase` directly:** the constructor gained a + `skipExceptions` parameter in fifth position, so positional callers past the fourth argument + need updating. Using `[ModuleTest]` is unaffected. + +### Fixed + +- **A module test now reports where it is declared.** `[ModuleTest]` captured no source location and + the discoverer forwarded none, so a test explorer had nowhere to navigate to and results carried + no file or line. Both halves are fixed, and a test asserts the location survives the whole way + onto the discovered test case. + + One limitation, deliberate and pinned by its own test: naming *two or more* modules — + `[ModuleTest(typeof(A), typeof(B))]` — still reports no location. C# does not allow the + caller-info parameters that capture it to follow a `params` array, so the multi-module overload + cannot take them. Naming one module or none captures the location as expected. + +### Added + +- **.NET 10 support.** Every shipping package now multi-targets `net8.0` and `net10.0`. A .NET 10 + project already worked — a `net8.0` assembly loads fine on it — but the package brought its + `Microsoft.Extensions.*` 8.x dependency along, and on .NET 10 those live in the shared framework, + so an older assembly landed in the output in place of the one the framework already supplies. Each + target framework now carries its own baseline version, so consumers roll forward from it rather + than being dragged back to it. + + Nothing is dropped: `net8.0` remains a target until it leaves support in November 2026, and the + generators stay on `netstandard2.0`, which is what Roslyn analyzers must target. The test suites + and the package verification script run against both frameworks. + +- **A test can ask for a `Mock` directly.** With `[MoqSupport]`, a parameter typed + `Mock` hands over the mock itself, so `Mock.Get` is no longer the only way to + reach it. It works exactly as `[Mock]` does: the service is replaced in the container before + anything resolves, so the service under test is built against the same mock. No attribute is needed + — the type says what it is — but `[Mock]` on such a parameter is accepted and simply redundant. + + The two spellings agree. `[Mock] IFoo` and `Mock` on one test give one mock seen two ways, + and two parameters naming the same `Mock` are one mock. A `[TestExport]` naming a real + implementation still overrides both, as it already did for `[Mock]`. +- **Environment conditions on decorators.** `[IfEnvironment]` and the rest of the family now take + effect on a `[Decorator]`, so a decorator can exist only where it is wanted — request logging in + development, a circuit breaker only in production. Where the condition does not hold the decorator + is never applied, so the service resolves undecorated rather than being wrapped by something that + re-tests the environment on every call. A condition changes whether a decorator applies, never + where it sits in the nesting. +- **Environment conditions on conventions**, as `IfEnvironment(…)`, `IfNotEnvironment(…)`, + `IfEnvironmentValue(key)`, `IfEnvironmentValue(key, value)` and `IfNotEnvironmentValue(…)`. A whole + rule can be gated without repeating the attribute on every class it matches. Named after the + attributes so the two ways of saying the same thing read the same. + + A condition on a convention combines with **and** against any condition on a matched class, so + neither declaration can silently discard the other. Two conventions matching one class under + different conditions keep their own guards rather than sharing the stricter one. + +### Fixed + +- **A condition on a `[Decorator]` was silently ignored.** The attribute compiled, read as + deliberate, and did nothing: decoration never looked at conditions, so a decorator marked + `[IfEnvironment("Development")]` wrapped the service in production too. + +### Changed + +- **The test extensibility hooks moved to `DependencyModules.Testing`** and no longer mention xUnit. + `ITestParameterValueProvider` and `IServiceProviderBuilderAttribute` moved across as they were but + now take an `ITestMethodContext` — a `MethodInfo` and the attributes already in scope — in place of + `IXunitTestMethod`. `ITestStartupAttribute` split along the seam it always had: registering services + is `ITestServiceSetupAttribute`, running against the built container stays `ITestStartupAttribute`, + and an attribute that only registers no longer carries a no-op `StartupAsync`. + + This finishes what creating `DependencyModules.Testing` started. Only the pieces that already had + no xUnit reference moved then, which left a mocking package unable to register anything without + taking a dependency on a test framework it does not use — which is how `Mock` support is + implemented without `DependencyModules.Moq` referencing xUnit at all. + + Implementations need a namespace change and the new parameter type; the bodies rarely change, since + nothing in this repository read more than `.Method` off the xUnit model. An attribute that does need + the full model can downcast the context to `IXunitTestMethodContext`. +- **An environment caches what it reads from the process**, misses included, for the life of the + instance. `IModuleEnvironment` is injectable and the instance `AddModules` registers is held for + the application's lifetime, so a service reading a value per request was paying a process lookup + and a fresh string allocation every call — and a miss is the common case, since an optional + variable that is not set is exactly what a default exists for. Values supplied at the call site are + unaffected, and the cache is kept separate from them so enumerating an environment still yields + only what was supplied. The cost is that an instance no longer sees a variable changed + mid-process. +- **`ModuleEnvironment.Default` is now `ModuleEnvironment.CreateDefault()`**, returning a new + instance per call rather than one shared by the process. A shared instance that caches would let + the first read of a variable fix it for every application in the process with no way to opt out — + the same reasoning that keeps `None` a type of its own. Because each call builds a fresh one, + asking again is how a current view of the process is obtained. +- **`DecoratorRegistration.RegistryFunc` is now an `EnvironmentRegistryFunc`**, taking the + environment alongside the collection, so a decorator's condition can be evaluated where it is + applied. Constructing one is unaffected — the `RegistryFunc` overload remains and adapts — but code + reading the property and invoking it with a single argument needs the extra parameter. This is + module plumbing reached through `IDependencyModule.InternalGetDecorators`; hand-written modules + that decorate directly are unaffected. + ## [1.0.0-rc9210] - 2026-08-09 Everything since `1.0.0-rc9200`. Still a release candidate: convention registration is new and diff --git a/Directory.Build.props b/Directory.Build.props index 0991cae..4743727 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -11,6 +11,16 @@ 1.0.0.0 + + + net8.0;net10.0 + + Ian Johnson Ian Johnson diff --git a/README.md b/README.md index 9e8cdd6..94a2e4c 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,9 @@ dotnet add package DependencyModules.Runtime dotnet add package DependencyModules.SourceGenerator ``` -Requires .NET 8.0 or later. See [CHANGELOG.md](CHANGELOG.md) for release notes. +Requires .NET 8.0 or later. The packages ship both `net8.0` and `net10.0` assemblies, so a project on +either LTS release gets one built against its own framework. See [CHANGELOG.md](CHANGELOG.md) for +release notes. ## Service Attributes diff --git a/benchmarks/DependencyModules.Benchmarks/DependencyModules.Benchmarks.csproj b/benchmarks/DependencyModules.Benchmarks/DependencyModules.Benchmarks.csproj index cbf20c0..f32a6ff 100644 --- a/benchmarks/DependencyModules.Benchmarks/DependencyModules.Benchmarks.csproj +++ b/benchmarks/DependencyModules.Benchmarks/DependencyModules.Benchmarks.csproj @@ -2,7 +2,7 @@ Exe - net8.0 + $(LibraryTargetFrameworks) disable enable false @@ -12,9 +12,17 @@ + + + + + + + + diff --git a/global.json b/global.json new file mode 100644 index 0000000..d99e6aa --- /dev/null +++ b/global.json @@ -0,0 +1,7 @@ +{ + "sdk": { + "version": "10.0.302", + "rollForward": "latestFeature", + "allowPrerelease": false + } +} diff --git a/integ-tests/ConsoleTestProject/ConsoleTestProject.csproj b/integ-tests/ConsoleTestProject/ConsoleTestProject.csproj index 352a233..bf5b959 100644 --- a/integ-tests/ConsoleTestProject/ConsoleTestProject.csproj +++ b/integ-tests/ConsoleTestProject/ConsoleTestProject.csproj @@ -2,7 +2,7 @@ Exe - net8.0 + $(LibraryTargetFrameworks) enable enable @@ -13,7 +13,11 @@ - + + + + + diff --git a/integ-tests/SecondarySutProject/SecondarySutProject.csproj b/integ-tests/SecondarySutProject/SecondarySutProject.csproj index 797104c..00eb7b0 100644 --- a/integ-tests/SecondarySutProject/SecondarySutProject.csproj +++ b/integ-tests/SecondarySutProject/SecondarySutProject.csproj @@ -1,7 +1,7 @@  - net8.0 + $(LibraryTargetFrameworks) enable enable False diff --git a/integ-tests/SutProject.Tests/Customization/CustomServiceProviderAttribute.cs b/integ-tests/SutProject.Tests/Customization/CustomServiceProviderAttribute.cs index 5dd8e91..7566d7f 100644 --- a/integ-tests/SutProject.Tests/Customization/CustomServiceProviderAttribute.cs +++ b/integ-tests/SutProject.Tests/Customization/CustomServiceProviderAttribute.cs @@ -1,13 +1,13 @@ -using DependencyModules.xUnit.Attributes.Interfaces; +using DependencyModules.Testing.Attributes.Interfaces; using Microsoft.Extensions.DependencyInjection; -using Xunit.v3; namespace SutProject.Tests.Customization; public class CustomServiceProviderAttribute : Attribute, IServiceProviderBuilderAttribute { - - public IServiceProvider BuildServiceProvider(IXunitTestMethod testCaseContext, IServiceCollection serviceCollection) { + + public IServiceProvider BuildServiceProvider( + ITestMethodContext testMethod, IServiceCollection serviceCollection) { serviceCollection.AddSingleton(); return serviceCollection.BuildServiceProvider(); } -} \ No newline at end of file +} diff --git a/integ-tests/SutProject.Tests/EnvironmentTests/EnvironmentConfigurationTests.cs b/integ-tests/SutProject.Tests/EnvironmentTests/EnvironmentConfigurationTests.cs index d21e084..f4ca0ff 100644 --- a/integ-tests/SutProject.Tests/EnvironmentTests/EnvironmentConfigurationTests.cs +++ b/integ-tests/SutProject.Tests/EnvironmentTests/EnvironmentConfigurationTests.cs @@ -79,7 +79,7 @@ public void ProcessEnvironment_WhenNotRegistered() { var serviceProvider = serviceCollection.BuildServiceProvider(); var dependency = serviceProvider.GetRequiredService(); - Assert.Equal(ModuleEnvironment.Default.EnvironmentName, dependency.EnvironmentName); + Assert.Equal(ModuleEnvironment.CreateDefault().EnvironmentName, dependency.EnvironmentName); } /// @@ -155,9 +155,19 @@ public void NullEnvironmentParameter_RegistersTheProcessDefault() { serviceCollection.AddModules((IModuleEnvironment?)null, new EnvironmentAwareModule()); + // The registered instance is the one that decided the registrations. CreateDefault builds a + // fresh environment per call, so the invariant is that these are the same object — not that + // either matches something asked for later. + var registered = Assert.Single( + serviceCollection, descriptor => descriptor.ServiceType == typeof(IModuleEnvironment)); + var serviceProvider = serviceCollection.BuildServiceProvider(); - Assert.Same(ModuleEnvironment.Default, serviceProvider.GetRequiredService()); + Assert.Same( + registered.ImplementationInstance, serviceProvider.GetRequiredService()); + Assert.Equal( + ModuleEnvironment.CreateDefault().EnvironmentName, + serviceProvider.GetRequiredService().EnvironmentName); } /// diff --git a/integ-tests/SutProject.Tests/Moq/MoqAttributeTests.cs b/integ-tests/SutProject.Tests/Moq/MoqAttributeTests.cs index 4b84f56..7778ded 100644 --- a/integ-tests/SutProject.Tests/Moq/MoqAttributeTests.cs +++ b/integ-tests/SutProject.Tests/Moq/MoqAttributeTests.cs @@ -7,8 +7,8 @@ namespace SutProject.Tests.Moq; /// /// The same scenario as the NSubstitute and FakeItEasy tests, so the three can be read against each -/// other. Moq is the one that separates the mock from the object, and this is what that costs at the -/// call site: Mock.Get to reach the setup. +/// other, plus the cases that only arise for Moq. Moq is the one that separates the mock from the +/// object, so a test can name either — and the two have to agree about which mock they mean. /// [MoqSupport] public class MoqAttributeTests { @@ -32,4 +32,115 @@ public void MockTest([Mock] IDependencyOne dependencyOne, public void UnconfiguredMembersAreLoose([Mock] IDependencyOne dependencyOne) { Assert.Null(dependencyOne.ScopedService); } + + /// + /// A Mock<T> parameter needs no attribute — the type already says what it is — and + /// naming it replaces the service for the whole test, not just for the parameter holding it. + /// + /// + /// The second assertion is the one that matters. Asking only whether the mock can be configured + /// would pass even with the registration removed, because an unregistered Mock<T> + /// parameter is constructed by the container as an ordinary concrete type and behaves like a mock + /// nothing else can see. + /// + [ModuleTest] + [SutModule] + public void MockOfTIsInjectedDirectly( + Mock mock, ISingletonService singletonService) { + mock.Setup(x => x.GetName()).Returns("mocked"); + + Assert.Same(mock.Object, singletonService); + Assert.Equal("mocked", singletonService.GetName()); + } + + /// + /// [Mock] on a Mock<T> is redundant rather than wrong, so a test written either way + /// behaves the same. Without the unwrap in ProvideMock this asks Moq to mock a Mock. + /// + [ModuleTest] + [SutModule] + public void MockOfTIsInjectedWithTheAttributeToo( + [Mock] Mock mock, ISingletonService singletonService) { + mock.Setup(x => x.GetName()).Returns("mocked"); + + Assert.Same(mock.Object, singletonService); + Assert.Equal("mocked", singletonService.GetName()); + } + + /// + /// The point of the whole thing: naming the mock replaces the service in the container, so the + /// real DependencyOne is constructed against it. Registering only the Mock<T> would leave + /// this holding a mock nothing else can see. + /// + [ModuleTest] + [SutModule] + public void ServiceUnderTestIsBuiltAgainstTheMock( + IDependencyOne dependencyOne, Mock singletonService) { + singletonService.Setup(x => x.GetName()).Returns("mocked"); + + Assert.Same(singletonService.Object, dependencyOne.SingletonService); + Assert.Equal("mocked", dependencyOne.SingletonService.GetName()); + } + + /// + /// Asking for both spellings of one service is asking for two views of a single mock. Two + /// mechanisms register this type — [Mock] and the Mock<T> scan — and if they disagreed the + /// test would configure one mock while the container handed out another. + /// + [ModuleTest] + [SutModule] + public void TheMockAndTheInstanceAreOnePair( + [Mock] ISingletonService instance, Mock mock) { + mock.Setup(x => x.GetName()).Returns("mocked"); + + Assert.Same(mock.Object, instance); + Assert.Equal("mocked", instance.GetName()); + } + + /// + /// Two parameters naming one service are one mock, so a setup made through either is visible + /// through the other. + /// + [ModuleTest] + [SutModule] + public void RepeatedMockParametersShareOneMock( + Mock first, Mock second) { + first.Setup(x => x.GetName()).Returns("mocked"); + + Assert.Same(first, second); + Assert.Equal("mocked", second.Object.GetName()); + } + + /// + /// Distinct services stay distinct — the scan keys on the mocked type, not on being a + /// Mock<T> — and both land in the graph, so one mocked dependency does not crowd out + /// another. + /// + [ModuleTest] + [SutModule] + public void DifferentServicesGetDifferentMocks( + Mock singletonService, + Mock scopedService, + IDependencyOne dependencyOne) { + Assert.Same(singletonService.Object, dependencyOne.SingletonService); + Assert.Same(scopedService.Object, dependencyOne.ScopedService); + } + + /// + /// Naming a real implementation beats mocking it. Mock support registers first within the setup + /// pass precisely so this holds, and holds wherever [MoqSupport] is applied — here it is on the + /// class and [TestExport] is on the method, but the outcome does not depend on that. + /// + [ModuleTest] + [SutModule] + [TestExport(typeof(ISingletonService), Implementation = typeof(ExportedSingletonService))] + public void TestExportStillWinsOverAMock( + ISingletonService instance, Mock mock) { + Assert.IsType(instance); + Assert.NotSame(mock.Object, instance); + } + + public class ExportedSingletonService : ISingletonService { + public string GetName() => "exported"; + } } diff --git a/integ-tests/SutProject.Tests/SutProject.Tests.csproj b/integ-tests/SutProject.Tests/SutProject.Tests.csproj index 6ea0dc8..dc96e18 100644 --- a/integ-tests/SutProject.Tests/SutProject.Tests.csproj +++ b/integ-tests/SutProject.Tests/SutProject.Tests.csproj @@ -1,7 +1,7 @@  - net8.0 + $(LibraryTargetFrameworks) enable enable False @@ -25,11 +25,11 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + all runtime; build; native; contentfiles; analyzers; buildtransitive - + diff --git a/integ-tests/SutProject/SutProject.csproj b/integ-tests/SutProject/SutProject.csproj index 5728aa5..66f953f 100644 --- a/integ-tests/SutProject/SutProject.csproj +++ b/integ-tests/SutProject/SutProject.csproj @@ -1,7 +1,7 @@  - net8.0 + $(LibraryTargetFrameworks) enable enable False diff --git a/integ-tests/web/WebApiApp.Tests/WebApiApp.Tests.csproj b/integ-tests/web/WebApiApp.Tests/WebApiApp.Tests.csproj index 90e213d..fa0c3bb 100644 --- a/integ-tests/web/WebApiApp.Tests/WebApiApp.Tests.csproj +++ b/integ-tests/web/WebApiApp.Tests/WebApiApp.Tests.csproj @@ -1,7 +1,7 @@  - net8.0 + $(LibraryTargetFrameworks) enable enable @@ -20,10 +20,10 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + all runtime; build; native; contentfiles; analyzers; buildtransitive - + diff --git a/integ-tests/web/WebApiApp/Program.cs b/integ-tests/web/WebApiApp/Program.cs index e20f610..6f7abf7 100644 --- a/integ-tests/web/WebApiApp/Program.cs +++ b/integ-tests/web/WebApiApp/Program.cs @@ -3,25 +3,16 @@ var builder = WebApplication.CreateBuilder(args); -// Add services to the container. -// Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle -builder.Services.AddEndpointsApiExplorer(); -builder.Services.AddSwaggerGen(); +// The point of this project: the generated module registers the services, and the endpoint below +// resolves one out of the container the same way any ASP.NET Core app would. builder.Services.AddModule(); var app = builder.Build(); -// Configure the HTTP request pipeline. -if (app.Environment.IsDevelopment()) { - app.UseSwagger(); - app.UseSwaggerUI(); -} - app.UseHttpsRedirection(); app.MapGet("/weatherforecast", (Weather weather) => weather.GetWeatherForecast()) - .WithName("GetWeatherForecast") - .WithOpenApi(); + .WithName("GetWeatherForecast"); app.Run(); diff --git a/integ-tests/web/WebApiApp/WebApiApp.csproj b/integ-tests/web/WebApiApp/WebApiApp.csproj index 3f08190..103186a 100644 --- a/integ-tests/web/WebApiApp/WebApiApp.csproj +++ b/integ-tests/web/WebApiApp/WebApiApp.csproj @@ -1,15 +1,17 @@ - net8.0 + $(LibraryTargetFrameworks) enable enable - - - - + diff --git a/scripts/verify-packages.sh b/scripts/verify-packages.sh index 66439f9..28f96f4 100755 --- a/scripts/verify-packages.sh +++ b/scripts/verify-packages.sh @@ -18,7 +18,11 @@ VERSION="${1:-1.0.0-verify}" REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" WORK_DIR="$(mktemp -d)" FEED="${WORK_DIR}/feed" -APP="${WORK_DIR}/ConsumerApp" + +# Every target framework the shipping libraries claim. Each one gets its own consumer project +# below, because "it packs" and "a consumer on that TFM can actually resolve and run it" are +# different claims, and only the second one is what ships. +TFMS=(net8.0 net10.0) cleanup() { rm -rf "${WORK_DIR}"; } trap cleanup EXIT @@ -68,6 +72,44 @@ for id in DependencyModules.SourceGenerator DependencyModules.Conventions; do pass "${id} analyzer assembly is at analyzers/dotnet/cs/ with no lib/" done +# The shipping libraries multi-target. A missing lib/ folder means one TFM quietly stopped being +# produced, and consumers on it would resolve no assembly at all. +LIB_PACKAGES=( + DependencyModules.Runtime + DependencyModules.Testing + DependencyModules.xUnit + DependencyModules.NSubstitute + DependencyModules.Moq + DependencyModules.FakeItEasy +) +for id in "${LIB_PACKAGES[@]}"; do + entries="$(unzip -Z1 "${FEED}/${id}.${VERSION}.nupkg")" + for tfm in "${TFMS[@]}"; do + grep -qx "lib/${tfm}/${id}.dll" <<<"${entries}" \ + || fail "${id}: no lib/${tfm}/${id}.dll in package, got:\n${entries}" + done + pass "${id} ships lib/ for every target framework" +done + +# Framework-matched dependency groups. This is the whole reason the libraries multi-target: a +# net10.0 group pinning an 8.x Microsoft.Extensions package puts that older assembly into a .NET 10 +# consumer's output in place of the one its shared framework already supplies. +for id in "${LIB_PACKAGES[@]}"; do + unzip -p "${FEED}/${id}.${VERSION}.nupkg" "${id}.nuspec" | python3 -c ' +import re, sys +nuspec = sys.stdin.read() +for tfm, body in re.findall(r"(.*?)", nuspec, re.S): + match = re.match(r"net(\d+)\.", tfm) + if not match: + continue + want = match.group(1) + for dep, ver in re.findall(r"id=\"(Microsoft\.Extensions\.[^\"]+)\" version=\"([^\"]+)\"", body): + if ver.split(".")[0] != want: + sys.exit(f" {tfm} group depends on {dep} {ver}, expected {want}.x") +' || fail "${id}: dependency groups are not framework-matched" + pass "${id} dependency groups are framework-matched" +done + # Placeholder metadata must never ship. for pkg in "${FEED}"/*.nupkg; do id="$(basename "${pkg}" ".${VERSION}.nupkg")" @@ -88,7 +130,21 @@ for id in DependencyModules.SourceGenerator DependencyModules.Conventions; do pass "${id} does not leak compiler dependencies" done -echo "==> Building a consumer project against the packed feed" +for TFM in "${TFMS[@]}"; do + +echo "==> Building a ${TFM} consumer project against the packed feed" + +APP="${WORK_DIR}/ConsumerApp-${TFM}" + +# Match the DI implementation package to the consumer's framework. Referencing 8.0.1 from a net10.0 +# app would mask the very regression this script exists to catch, by pinning the old assembly from +# the consumer side rather than letting the package's own dependency group decide. +case "${TFM}" in + net8.0) DI_VERSION="8.0.1" ;; + net10.0) DI_VERSION="10.0.0" ;; + *) fail "no Microsoft.Extensions.DependencyInjection version mapped for ${TFM}" ;; +esac + mkdir -p "${APP}" cat >"${APP}/nuget.config" < @@ -98,6 +154,15 @@ cat >"${APP}/nuget.config" < + + + + EOF @@ -105,7 +170,7 @@ cat >"${APP}/ConsumerApp.csproj" < Exe - net8.0 + ${TFM} enable enable - + EOF @@ -192,8 +257,14 @@ public static class Program { EOF dotnet build "${APP}/ConsumerApp.csproj" -c Release --nologo -v quiet \ - || fail "consumer project failed to build against the packed feed" -pass "consumer project builds" + || fail "${TFM} consumer project failed to build against the packed feed" +pass "${TFM} consumer project builds" + +# The package has a lib/ folder per TFM, and NuGet picks one. Assert it picked this consumer's own +# rather than falling back to an older compatible one, which would still build and still run. +grep -qF "lib/${TFM}/DependencyModules.Runtime.dll" "${APP}/obj/project.assets.json" \ + || fail "${TFM} consumer did not resolve lib/${TFM}/ of DependencyModules.Runtime" +pass "${TFM} consumer resolved lib/${TFM}/" # Prove the generator actually ran, rather than the build merely succeeding. generated="$(find "${APP}/generated" -name '*.g.cs' 2>/dev/null || true)" @@ -215,11 +286,13 @@ if grep -rq 'ExcludeFromCodeCoverage' "${APP}/generated"; then fi pass "MSBuild properties reach the generator through build/*.targets" -echo "==> Running the consumer app" +echo "==> Running the ${TFM} consumer app" output="$(dotnet run --project "${APP}/ConsumerApp.csproj" -c Release --no-build --nologo)" [ "${output}" = "hello from a packaged generator" ] \ - || fail "unexpected consumer output: ${output}" -pass "resolved a generated registration at run time" + || fail "unexpected ${TFM} consumer output: ${output}" +pass "${TFM} resolved a generated registration at run time" + +done echo -echo "All package verification checks passed." +echo "All package verification checks passed for: ${TFMS[*]}" diff --git a/src/DependencyModules.Conventions/ConventionContractSource.cs b/src/DependencyModules.Conventions/ConventionContractSource.cs index 432c48c..aa83c34 100644 --- a/src/DependencyModules.Conventions/ConventionContractSource.cs +++ b/src/DependencyModules.Conventions/ConventionContractSource.cs @@ -339,6 +339,66 @@ IConventionRegistration WithoutAttribute() /// Name patterns to exclude. IConventionRegistration WithoutName(params string[] patterns); + /// + /// Registers the matches only when the environment name is one of + /// . + /// + /// + /// + /// The test runs when the modules are applied, not while the build runs, so this + /// changes what is registered rather than what the convention matched. Every match + /// is still emitted, behind the same guard. + /// + /// + /// A condition here combines with and against any + /// [IfEnvironment] on a matched class, so neither can silently override the + /// other. Conditions of different kinds also combine with and; alternatives go + /// inside one call. + /// + /// + /// + /// Accepted names, compared case-insensitively to match + /// IHostEnvironment.IsDevelopment(). + /// + IConventionRegistration IfEnvironment(params string[] environmentNames); + + /// + /// Registers the matches only when the environment name is none of + /// . + /// + /// Names to exclude, compared case-insensitively. + IConventionRegistration IfNotEnvironment(params string[] environmentNames); + + /// + /// Registers the matches only when the environment carries a value for + /// . + /// + /// The key that must be present. + IConventionRegistration IfEnvironmentValue(string key); + + /// + /// Registers the matches only when the environment's value for + /// equals . + /// + /// The key to read. + /// The value it must equal, compared ordinally. + IConventionRegistration IfEnvironmentValue(string key, string value); + + /// + /// Registers the matches only when the environment carries no value for + /// . + /// + /// The key that must be absent. + IConventionRegistration IfNotEnvironmentValue(string key); + + /// + /// Registers the matches only when the environment's value for + /// does not equal . + /// + /// The key to read. + /// The value it must not equal, compared ordinally. + IConventionRegistration IfNotEnvironmentValue(string key, string value); + /// /// Registers every match as , whatever it matched /// through. diff --git a/src/DependencyModules.Conventions/Models/ConventionModel.cs b/src/DependencyModules.Conventions/Models/ConventionModel.cs index e2a5d66..0a0a0ef 100644 --- a/src/DependencyModules.Conventions/Models/ConventionModel.cs +++ b/src/DependencyModules.Conventions/Models/ConventionModel.cs @@ -174,7 +174,8 @@ public record ConventionModel( IReadOnlyList? AttributeFilters = null, IReadOnlyList? NameFilters = null, ITypeDefinition? ExplicitServiceType = null, - string? AssemblyName = null) { + string? AssemblyName = null, + IReadOnlyList? Conditions = null) { /// /// Whether the attributes a candidate carries pass the filters. @@ -249,7 +250,8 @@ other is not null && ModelEquality.ListEquals(AttributeFilters, other.AttributeFilters) && ModelEquality.ListEquals(NameFilters, other.NameFilters) && Equals(ExplicitServiceType, other.ExplicitServiceType) && - AssemblyName == other.AssemblyName; + AssemblyName == other.AssemblyName && + ModelEquality.ListEquals(Conditions, other.Conditions); public override int GetHashCode() { unchecked { @@ -267,6 +269,7 @@ public override int GetHashCode() { hash = hash * 31 + ModelEquality.ListHashCode(NameFilters); hash = hash * 31 + (ExplicitServiceType?.GetHashCode() ?? 0); hash = hash * 31 + (AssemblyName?.GetHashCode() ?? 0); + hash = hash * 31 + ModelEquality.ListHashCode(Conditions); return hash; } } diff --git a/src/DependencyModules.Conventions/Utilities/ConventionMatcher.cs b/src/DependencyModules.Conventions/Utilities/ConventionMatcher.cs index 81e5c52..3c3c635 100644 --- a/src/DependencyModules.Conventions/Utilities/ConventionMatcher.cs +++ b/src/DependencyModules.Conventions/Utilities/ConventionMatcher.cs @@ -618,16 +618,21 @@ private static IReadOnlyList BuildServiceModels( // ServiceModelUtility builds for the attribute path. Two models with the same // ImplementationType would duplicate the per-implementation state the writer reads: // constructor, conditions, cross-wire. - var byImplementation = new Dictionary>(); - var order = new List(); + // Keyed by implementation *and* the conditions in force, rather than implementation alone. + // The writer emits one guard around everything a ServiceModel registers, so two conventions + // matching the same class under different conditions have to stay apart — merged into one + // model, whichever condition set won would silently gate the other's registrations too. + var byGroup = new Dictionary>(); + var order = new List(); foreach (var entry in usable) { - var implementation = entry.Match.Candidate.ImplementationType; + var conditions = MergeConditions(entry.Match); + var key = GroupKey(entry.Match.Candidate.ImplementationType, conditions); - if (!byImplementation.TryGetValue(implementation, out var list)) { + if (!byGroup.TryGetValue(key, out var list)) { list = new List(); - byImplementation[implementation] = list; - order.Add(entry.Match); + byGroup[key] = list; + order.Add(new ConventionModelGroup(entry.Match, conditions, key)); } list.Add(entry.Registration); @@ -635,28 +640,80 @@ private static IReadOnlyList BuildServiceModels( var models = new List(order.Count); - foreach (var match in order) { - var registrations = byImplementation[match.Candidate.ImplementationType]; + foreach (var group in order) { + var registrations = byGroup[group.Key]; logger.Info( - $" {match.Candidate.ImplementationType.Name} -> " + + $" {group.Match.Candidate.ImplementationType.Name} -> " + $"{string.Join(", ", registrations.Select(r => r.ServiceType.Name))} " + "(by convention)"); models.Add(new ServiceModel( - match.Candidate.ImplementationType, - match.Candidate.Constructor, + group.Match.Candidate.ImplementationType, + group.Match.Candidate.Constructor, null, null, registrations, RegistrationFeature.None, - // Shared across every match for this type, so the first one carries them all. - match.Candidate.Conditions)); + group.Conditions)); } return models; } + /// + /// One ServiceModel's worth of matches: the same implementation under the same conditions. + /// + private record ConventionModelGroup( + ConventionRegistrationMatch Match, + IReadOnlyList? Conditions, + string Key); + + /// + /// The conditions in force for one match: the convention's and the class's, combined. + /// + /// + /// Combined with and, matching how conditions of different kinds already combine on a + /// class. Letting either side win would mean one declaration silently discarding a condition + /// written in the other, which is the kind of thing nobody finds until production. + /// + private static IReadOnlyList? MergeConditions( + ConventionRegistrationMatch match) { + + var fromConvention = match.Convention.Conditions; + var fromCandidate = match.Candidate.Conditions; + + if ((fromConvention?.Count ?? 0) == 0) { + return fromCandidate; + } + + if ((fromCandidate?.Count ?? 0) == 0) { + return fromConvention; + } + + var merged = new List(fromConvention!.Count + fromCandidate!.Count); + merged.AddRange(fromConvention); + merged.AddRange(fromCandidate); + + return merged; + } + + /// + /// A stable key for grouping, so equal condition sets share a model and different ones do not. + /// + private static string GroupKey( + ITypeDefinition implementation, IReadOnlyList? conditions) { + + if ((conditions?.Count ?? 0) == 0) { + return implementation.ToString(); + } + + var parts = conditions!.Select(condition => + $"{condition.Kind}|{condition.Negate}|{condition.Key}|{string.Join(",", condition.Values)}"); + + return implementation + "::" + string.Join(";", parts); + } + /// /// The registrations one match produces, according to what the convention registers matches as. /// diff --git a/src/DependencyModules.Conventions/Utilities/ConventionModelUtility.cs b/src/DependencyModules.Conventions/Utilities/ConventionModelUtility.cs index 592441f..683ed81 100644 --- a/src/DependencyModules.Conventions/Utilities/ConventionModelUtility.cs +++ b/src/DependencyModules.Conventions/Utilities/ConventionModelUtility.cs @@ -40,6 +40,20 @@ public static class ConventionModelUtility { private const string InAssemblyOfCall = "InAssemblyOf"; private const string WithKeyCall = "WithKey"; + /// + /// The environment condition calls, and what each produces. + /// + /// + /// Named after the attributes rather than something fluent-sounding like OnlyIn, so the + /// two ways of saying the same thing read the same and one is discoverable from the other. + /// + private static readonly Dictionary ConditionCalls = new() { + ["IfEnvironment"] = (EnvironmentConditionKind.Name, false), + ["IfNotEnvironment"] = (EnvironmentConditionKind.Name, true), + ["IfEnvironmentValue"] = (EnvironmentConditionKind.Value, false), + ["IfNotEnvironmentValue"] = (EnvironmentConditionKind.Value, true), + }; + private static readonly Dictionary LifetimeCalls = new() { ["AsSingleton"] = ServiceLifestyle.Singleton, ["AsScoped"] = ServiceLifestyle.Scoped, @@ -253,6 +267,7 @@ private static void ReadStatement( IReadOnlyList? keyNamespaces = null; List? attributeFilters = null; List? nameFilters = null; + List? conditions = null; ITypeDefinition? explicitServiceType = null; string? assemblyName = null; @@ -376,6 +391,44 @@ private static void ReadStatement( continue; } + if (ConditionCalls.TryGetValue(name, out var conditionCall)) { + // Read as literals for the same reason every other filter is: the declaration is + // parsed, never executed, so anything the build cannot see is refused rather than + // quietly dropped. + var arguments = ReadPatterns(context, call); + + if (arguments.Count == 0) { + reason = conditionCall.Kind == EnvironmentConditionKind.Name + ? $"'{name}' needs at least one environment name it can read at compile time" + : $"'{name}' needs an environment key it can read at compile time"; + + return null; + } + + conditions ??= new List(); + + if (conditionCall.Kind == EnvironmentConditionKind.Name) { + conditions.Add(new EnvironmentConditionModel( + EnvironmentConditionKind.Name, conditionCall.Negate, null, arguments)); + } else { + // (key) tests presence, (key, value) tests equality. More than two would be a + // call that does not exist on the interface. + if (arguments.Count > 2) { + reason = $"'{name}' takes a key and an optional value"; + + return null; + } + + conditions.Add(new EnvironmentConditionModel( + EnvironmentConditionKind.Value, + conditionCall.Negate, + arguments[0], + arguments.Count > 1 ? new[] { arguments[1] } : Array.Empty())); + } + + continue; + } + if (name is WithAttributeCall or WithoutAttributeCall) { var attributeType = SingleTypeArgumentOf(context, call); @@ -452,7 +505,8 @@ private static void ReadStatement( attributeFilters, nameFilters, explicitServiceType, - assemblyName); + assemblyName, + conditions); } /// diff --git a/src/DependencyModules.FakeItEasy/DependencyModules.FakeItEasy.csproj b/src/DependencyModules.FakeItEasy/DependencyModules.FakeItEasy.csproj index 962c562..80b4d1b 100644 --- a/src/DependencyModules.FakeItEasy/DependencyModules.FakeItEasy.csproj +++ b/src/DependencyModules.FakeItEasy/DependencyModules.FakeItEasy.csproj @@ -1,7 +1,7 @@  - net8.0 + $(LibraryTargetFrameworks) enable enable True diff --git a/src/DependencyModules.Moq/DependencyModules.Moq.csproj b/src/DependencyModules.Moq/DependencyModules.Moq.csproj index ffa4967..dd3b116 100644 --- a/src/DependencyModules.Moq/DependencyModules.Moq.csproj +++ b/src/DependencyModules.Moq/DependencyModules.Moq.csproj @@ -1,12 +1,12 @@  - net8.0 + $(LibraryTargetFrameworks) enable enable True DependencyModules.Moq - Moq mocking support for DependencyModules test integrations. Add [MoqSupport] to resolve any unregistered dependency as a Moq mock, and use [Mock] on a test parameter to inject it. Reach the underlying Mock<T> with Mock.Get(instance). Pair with a test framework integration such as DependencyModules.xUnit. + Moq mocking support for DependencyModules test integrations. Add [MoqSupport], then take a Mock<T> test parameter to get the mock itself, or mark a parameter [Mock] to get the mocked instance. Either way the service under test is built against the same mock. Pair with a test framework integration such as DependencyModules.xUnit. true diff --git a/src/DependencyModules.Moq/MoqSupportAttribute.cs b/src/DependencyModules.Moq/MoqSupportAttribute.cs index 35fefd1..d20ba8d 100644 --- a/src/DependencyModules.Moq/MoqSupportAttribute.cs +++ b/src/DependencyModules.Moq/MoqSupportAttribute.cs @@ -1,19 +1,22 @@ +using System.Diagnostics.CodeAnalysis; using DependencyModules.Testing.Attributes.Interfaces; +using Microsoft.Extensions.DependencyInjection; using MoqLib = Moq; namespace DependencyModules.Moq; /// -/// Resolves any dependency that is not registered as a Moq mock. +/// Supplies a test's mocks with Moq. /// /// /// Applies to a method, a class, or a whole assembly, so a test project can switch mocks on once in /// an AssemblyInfo rather than per test. /// -/// Moq separates the mock from the object it produces, and the container needs the object, so that -/// is what is injected. Reach the Mock<T> to configure or verify it with -/// Mock.Get(instance). This is the one place Moq reads differently from NSubstitute and -/// FakeItEasy, where the injected instance is itself the thing you configure. +/// A test asks for a mock in either of two ways, and both work. A parameter typed +/// Mock<T> hands over the mock itself — the thing you configure and verify on — while +/// the container is separately given its Object, so the service under test is built against +/// that same mock. A parameter marked [Mock] and typed as the service hands over the object +/// instead, and Mock.Get(instance) reaches the mock behind it. /// /// Mocks are loose, matching Moq's own default: an unconfigured member returns default rather than /// throwing. @@ -22,28 +25,89 @@ namespace DependencyModules.Moq; /// /// [ModuleTest] /// [MoqSupport] -/// public void SendsTheMail(IEmailSender sender, [Mock] IAuditLog log) { -/// Mock.Get(log).Verify(x => x.Write(It.IsAny<string>())); +/// public void SendsTheMail(IEmailSender sender, Mock<IAuditLog> log) { +/// log.Verify(x => x.Write(It.IsAny<string>())); /// } /// /// [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class | AttributeTargets.Assembly)] -public class MoqSupportAttribute : Attribute, IMockSupportAttribute { +public class MoqSupportAttribute : Attribute, IMockSupportAttribute, ITestServiceSetupAttribute { /// - /// Provides a mocked instance of the specified type. + /// Registers a mock for every Mock<T> the test asked for, alongside the object that + /// mock produces. + /// + /// + /// Registering both is what lines the two halves up. The test resolves the Mock<T> + /// and configures it; everything the container builds — the service under test included — + /// resolves the T and gets that same mock's object. Neither has to know about the other, + /// and the test wires nothing together itself. + /// + /// Runs after the value providers behind [Mock] and before any other setup attribute, and + /// registrations are last-one-wins. So this settles any disagreement with [Mock] — which + /// is what lets [Mock] IFoo and Mock<IFoo> sit on one test and resolve to a + /// matched pair rather than to two unrelated mocks — while a [TestExport] naming a real + /// implementation of the same service still overrides both. + /// + /// The test the container is being built for. + /// The collection backing the test's container. + public void SetupServiceCollection(ITestMethodContext testMethod, IServiceCollection serviceCollection) { + var mocked = new HashSet(); + + foreach (var parameter in testMethod.Method.GetParameters()) { + // Add returns false for a type already handled: two parameters naming the same + // Mock are one mock, so configuring either is configuring what the test was given. + if (!TryGetMockedType(parameter.ParameterType, out var mockedType) || + !mocked.Add(mockedType)) { + continue; + } + + var mock = CreateMock(mockedType); + + serviceCollection.AddSingleton(parameter.ParameterType, _ => mock); + serviceCollection.AddSingleton(mockedType, _ => mock.Object); + } + } + + /// + /// Provides a mocked instance of the specified type, for a parameter marked [Mock]. /// /// /// Constructed through the closed generic because Mock<T> is the only form Moq /// offers, and the type is not known until the test asks for it. + /// + /// A [Mock] Mock<IFoo> parameter arrives here as the Mock<IFoo>, which + /// is unwrapped rather than mocked — asking Moq to mock a Mock is never what was meant. + /// owns that parameter and runs afterwards, replacing what + /// is returned here, so the two spellings converge on the one mock. /// - /// The type to mock. + /// The type to mock, or a Mock<T> naming it. /// - /// The mocked instance — Mock<T>.Object, not the Mock<T> itself. + /// The mocked instance — Mock<T>.Object — or a Mock<T> when that is + /// what was asked for. /// public object ProvideMock(Type type) { - var mock = (MoqLib.Mock)Activator.CreateInstance(typeof(MoqLib.Mock<>).MakeGenericType(type))!; + if (TryGetMockedType(type, out var mockedType)) { + return CreateMock(mockedType); + } + + return CreateMock(type).Object; + } + + private static MoqLib.Mock CreateMock(Type type) => + (MoqLib.Mock)Activator.CreateInstance(typeof(MoqLib.Mock<>).MakeGenericType(type))!; + + /// + /// Reads IFoo out of a Mock<IFoo>, and reports anything else as not ours. + /// + private static bool TryGetMockedType(Type parameterType, [NotNullWhen(true)] out Type? mockedType) { + if (parameterType.IsGenericType && + parameterType.GetGenericTypeDefinition() == typeof(MoqLib.Mock<>)) { + mockedType = parameterType.GetGenericArguments()[0]; + return true; + } - return mock.Object; + mockedType = null; + return false; } } diff --git a/src/DependencyModules.NSubstitute/DependencyModules.NSubstitute.csproj b/src/DependencyModules.NSubstitute/DependencyModules.NSubstitute.csproj index 3bd80d5..b26a745 100644 --- a/src/DependencyModules.NSubstitute/DependencyModules.NSubstitute.csproj +++ b/src/DependencyModules.NSubstitute/DependencyModules.NSubstitute.csproj @@ -1,7 +1,7 @@  - net8.0 + $(LibraryTargetFrameworks) enable enable True diff --git a/src/DependencyModules.Runtime/DependencyModules.Runtime.csproj b/src/DependencyModules.Runtime/DependencyModules.Runtime.csproj index 5f20cf8..a51742f 100644 --- a/src/DependencyModules.Runtime/DependencyModules.Runtime.csproj +++ b/src/DependencyModules.Runtime/DependencyModules.Runtime.csproj @@ -1,7 +1,7 @@  - net8.0 + $(LibraryTargetFrameworks) enable enable True @@ -10,8 +10,19 @@ true - + + + + + + diff --git a/src/DependencyModules.Runtime/Helpers/DecoratorRegistration.cs b/src/DependencyModules.Runtime/Helpers/DecoratorRegistration.cs index 48b7754..41d7503 100644 --- a/src/DependencyModules.Runtime/Helpers/DecoratorRegistration.cs +++ b/src/DependencyModules.Runtime/Helpers/DecoratorRegistration.cs @@ -1,3 +1,5 @@ +using DependencyModules.Runtime.Interfaces; + namespace DependencyModules.Runtime.Helpers; /// @@ -13,8 +15,22 @@ namespace DependencyModules.Runtime.Helpers; /// them. By convention framework packages use 0-999 and application code uses 1000 and above, so an /// application's decorators wrap those contributed by the libraries it consumes. /// -/// The function that rewrites registrations in the collection. -public readonly struct DecoratorRegistration(int order, RegistryFunc registryFunc) { +/// +/// The function that rewrites registrations in the collection, receiving the environment any +/// condition on the decorator is evaluated against. +/// +public readonly struct DecoratorRegistration(int order, EnvironmentRegistryFunc registryFunc) { + /// + /// A decorator with no environment condition. + /// + /// + /// Adapted to the environment-taking form rather than stored separately, so conditional and + /// unconditional decorators sort against each other on alone. Keeping them + /// apart would have let a condition change where a decorator sits in the nesting. + /// + public DecoratorRegistration(int order, RegistryFunc registryFunc) + : this(order, (serviceCollection, _) => registryFunc(serviceCollection)) { } + /// /// Order in which this decorator is applied relative to all others. /// @@ -23,5 +39,5 @@ public readonly struct DecoratorRegistration(int order, RegistryFunc registryFun /// /// The function that performs the decoration. /// - public RegistryFunc RegistryFunc { get; } = registryFunc; + public EnvironmentRegistryFunc RegistryFunc { get; } = registryFunc; } diff --git a/src/DependencyModules.Runtime/Helpers/DependencyRegistry.cs b/src/DependencyModules.Runtime/Helpers/DependencyRegistry.cs index bc1d22e..16620d3 100644 --- a/src/DependencyModules.Runtime/Helpers/DependencyRegistry.cs +++ b/src/DependencyModules.Runtime/Helpers/DependencyRegistry.cs @@ -130,6 +130,25 @@ public static int AddDecorator(RegistryFunc registryFunc, int order = 0) { return 1; } + /// + /// Add decorator func whose application may depend on the environment + /// + /// + /// Generated code uses this form only when a decorator declares an environment condition. + /// A decorator that does not apply is simply never invoked, so the service resolves undecorated + /// rather than being wrapped by something that checks the environment on every call. + /// + /// Function that decorates registrations already in the collection. + /// See the other overload; ordering is unaffected by the condition. + /// + public static int AddDecorator(EnvironmentRegistryFunc registryFunc, int order = 0) { + lock (SyncLock) { + Decorators.Add(new DecoratorRegistration(order, registryFunc)); + } + + return 1; + } + /// /// Add module /// @@ -163,7 +182,7 @@ public static void LoadModules(IServiceCollection serviceCollection, params IDep /// /// public static void ApplyServices(IServiceCollection serviceCollection) { - ApplyServices(serviceCollection, ModuleEnvironment.Default); + ApplyServices(serviceCollection, FindOrCreateEnvironment(serviceCollection)); } /// @@ -188,10 +207,60 @@ public static void ApplyServices(IServiceCollection serviceCollection, IModuleEn /// /// public static void ApplyDecorators(IServiceCollection serviceCollection) { + ApplyDecorators(serviceCollection, FindOrCreateEnvironment(serviceCollection)); + } + + /// + /// The environment already in the collection, or a fresh default. + /// + /// + /// Deliberately does not register what it creates. These overloads apply registrations to a + /// collection the caller owns, and adding a descriptor nobody asked for would change what they + /// hand back. An environment the caller did supply is found and shared, which is the case + /// where two calls disagreeing would actually matter — two process defaults read the same + /// variables and give the same answers. + /// + private static IModuleEnvironment FindOrCreateEnvironment(IServiceCollection serviceCollection) { + RefuseUnusableEnvironment(serviceCollection); + + return FindModuleEnvironment(serviceCollection) ?? ModuleEnvironment.CreateDefault(); + } + + /// + /// The environment already in the collection, or a new default registered into it. + /// + /// + /// Registered, not just used. Otherwise conditions would be decided by an environment that + /// GetRequiredService<IModuleEnvironment>() then throws for. Only when nothing + /// supplied one, so an application's own environment is never displaced — and registering it is + /// what lets decoration find the same instance the registrations were decided against, which + /// matters now that CreateDefault builds a fresh one per call. + /// + private static IModuleEnvironment ResolveEnvironment(IServiceCollection serviceCollection) { + var environment = FindModuleEnvironment(serviceCollection); + + if (environment != null) { + return environment; + } + + RefuseUnusableEnvironment(serviceCollection); + + environment = ModuleEnvironment.CreateDefault(); + serviceCollection.AddSingleton(environment); + + return environment; + } + + /// + /// Apply all decorators, evaluating any environment conditions against the supplied environment + /// + /// + /// + public static void ApplyDecorators(IServiceCollection serviceCollection, IModuleEnvironment environment) { // OrderBy is a stable sort, so decorators sharing an order keep their registration order // rather than nesting arbitrarily. foreach (var decorator in GetDecorators().OrderBy(decorator => decorator.Order)) { - decorator.RegistryFunc(serviceCollection); + decorator.RegistryFunc(serviceCollection, environment); } } @@ -243,8 +312,16 @@ private static void ApplyDecorators(IServiceCollection serviceCollection, IReadO } if (decorators.Count > 0) { + // The same environment the registrations were decided against. ApplyServices runs first + // and registers one when nothing supplied it, so this finds that instance rather than + // building a second answer to "what environment is this" — a decorator gated on + // Development must not apply next to a service that decided it was in Production. + // CreateDefault returns a fresh instance per call, so falling back to it here rather + // than to the registered one would be exactly that divergence. + var environment = ResolveEnvironment(serviceCollection); + foreach (var decorator in decorators.OrderBy(decorator => decorator.Order)) { - decorator.RegistryFunc(serviceCollection); + decorator.RegistryFunc(serviceCollection, environment); } } @@ -279,19 +356,11 @@ private static void ApplyServices(IServiceCollection serviceCollection, IReadOnl return; } - var environment = FindModuleEnvironment(serviceCollection); - - if (environment == null) { - RefuseUnusableEnvironment(serviceCollection); - - environment = ModuleEnvironment.Default; - - // Registered, not just used. Otherwise conditions would be decided by an environment - // that GetRequiredService() then throws for, which is the same - // inconsistency one layer out. Only when nothing supplied one, so an application's own - // environment is never displaced. - serviceCollection.AddSingleton(environment); - } + // Registered, not just used. Otherwise conditions would be decided by an environment that + // GetRequiredService() then throws for, which is the same inconsistency + // one layer out. Only when nothing supplied one, so an application's own environment is + // never displaced — and registering it is what lets ApplyDecorators find the same instance. + var environment = ResolveEnvironment(serviceCollection); for (var i = 0; i < modules.Count; i++) { var module = modules[i]; diff --git a/src/DependencyModules.Runtime/ModuleEnvironment.cs b/src/DependencyModules.Runtime/ModuleEnvironment.cs index ceaab26..4a38f48 100644 --- a/src/DependencyModules.Runtime/ModuleEnvironment.cs +++ b/src/DependencyModules.Runtime/ModuleEnvironment.cs @@ -1,4 +1,5 @@ using System.Collections; +using System.Collections.Concurrent; using DependencyModules.Runtime.Interfaces; namespace DependencyModules.Runtime; @@ -26,6 +27,10 @@ public class ModuleEnvironment : IModuleEnvironment, IEnumerable _values; private readonly bool _fallBackToEnvironmentVariables; + // Null values are cached too — an unset optional variable is the case a default exists for, and + // not caching it would leave the common path paying a process read every call. + private readonly ConcurrentDictionary _processValues = new(); + /// /// Creates an environment with a fixed name and an optional set of values, falling back to /// environment variables for anything not supplied here. @@ -80,15 +85,27 @@ public ModuleEnvironment( /// hide an environment variable of the same name. Only a key that was never mentioned falls /// through to the process. /// - /// The variable is read on each call rather than captured, matching - /// . Registration reads these once at startup, so nothing repeats the cost. + /// A variable read from the process is cached for the life of this instance, misses included. + /// This is injectable, so a service reading a value on every request would otherwise pay a + /// process lookup and a fresh string allocation each time — and a miss is the common case, since + /// an optional value that is not set is exactly what a default exists for. The cost of that is + /// no longer seeing a variable changed mid-process, which nothing should be relying on; ask + /// for a fresh view if you need one. /// - public string? Value(string name) => - _values.TryGetValue(name, out var value) - ? value - : _fallBackToEnvironmentVariables - ? Environment.GetEnvironmentVariable(name) - : null; + public string? Value(string name) { + if (_values.TryGetValue(name, out var value)) { + return value; + } + + if (!_fallBackToEnvironmentVariables) { + return null; + } + + // Separate from _values rather than written back into it. That dictionary is what the caller + // supplied, and GetEnumerator says so — folding process reads into it would have this + // environment report values nobody gave it. + return _processValues.GetOrAdd(name, static key => Environment.GetEnvironmentVariable(key)); + } /// /// Adds a value, replacing any already present for . @@ -131,17 +148,23 @@ public ModuleEnvironment( /// IHostEnvironment, and means a service gated on a non-production environment stays /// unregistered unless something says otherwise. /// - /// Values are read on each call rather than captured, so a variable set after startup is still - /// seen. Registration happens once, so the cost does not repeat. + /// A new instance each call, rather than one shared by the process. The instance caches what it + /// reads, and a cache shared by every application in the process would let the first read of a + /// variable fix it for all of them, with no way to opt out — the same reasoning that keeps + /// a type of its own. + /// + /// Because each call builds a fresh one, asking again is how you get a current view of the + /// process. The instance AddModules registers is held for the application's lifetime, so + /// a service injecting reads through a warm cache. /// - public static IModuleEnvironment Default { get; } = new ProcessModuleEnvironment(); + public static IModuleEnvironment CreateDefault() => new ProcessModuleEnvironment(); /// /// An environment with no name and no values, so every condition evaluates false. /// /// /// Pass this to AddModules to state that this application has no environment, rather - /// than leaving it unset and picking up . + /// than leaving it unset and picking up . /// /// Its own type rather than an empty : this instance is shared by /// every application in the process, and would let a cast reach in and give @@ -150,12 +173,17 @@ public ModuleEnvironment( public static IModuleEnvironment None { get; } = new EmptyModuleEnvironment(); private sealed class ProcessModuleEnvironment : IModuleEnvironment { + private readonly ConcurrentDictionary _values = new(); + + // Not cached. It is read once per AddModules call rather than per service, and a fresh + // instance is what CreateDefault hands out anyway. public string EnvironmentName => Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT") ?? "Production"; - public string? Value(string name) => Environment.GetEnvironmentVariable(name); + public string? Value(string name) => + _values.GetOrAdd(name, static key => Environment.GetEnvironmentVariable(key)); } private sealed class EmptyModuleEnvironment : IModuleEnvironment { diff --git a/src/DependencyModules.SourceGenerator.Impl/DecoratorFileWriter.cs b/src/DependencyModules.SourceGenerator.Impl/DecoratorFileWriter.cs index 9ff145a..a3d8da3 100644 --- a/src/DependencyModules.SourceGenerator.Impl/DecoratorFileWriter.cs +++ b/src/DependencyModules.SourceGenerator.Impl/DecoratorFileWriter.cs @@ -60,15 +60,35 @@ private static void WriteDecorator( var services = method.AddParameter( KnownTypes.Microsoft.DependencyInjection.IServiceCollection, "services"); - method.AddIndentedStatement( - new StaticInvokeStatement( - KnownTypes.DependencyModules.Helpers.DecoratorHelper, - "Decorate", - new List { - CodeOutputComponent.Get(services.Name), - TypeOf(decorator.ServiceType), - TypeOf(decorator.DecoratorType) - })); + // The parameter only appears when something tests it, so an unconditional decorator keeps + // the RegistryFunc shape and the AddDecorator overload it always used. + var hasConditions = decorator.Conditions is { Count: > 0 }; + + var environment = hasConditions + ? method.AddParameter(KnownTypes.DependencyModules.Interfaces.IModuleEnvironment, "environment") + : null; + + var decorate = new StaticInvokeStatement( + KnownTypes.DependencyModules.Helpers.DecoratorHelper, + "Decorate", + new List { + CodeOutputComponent.Get(services.Name), + TypeOf(decorator.ServiceType), + TypeOf(decorator.DecoratorType) + }); + + if (environment != null) { + // Guarding the call rather than the registration: a decorator that does not apply is + // simply not run, so the service resolves undecorated instead of being wrapped by + // something that re-tests the environment on every call. + var block = method.If( + CodeOutputComponent.Get( + EnvironmentConditionWriter.BuildCondition(decorator.Conditions!, environment.Name))); + + block.AddIndentedStatement(decorate); + } else { + method.AddIndentedStatement(decorate); + } // A field initializer registers the method, matching how service registrations are hooked up. // DynamicDependency keeps the trimmer from removing a method only referenced this way. diff --git a/src/DependencyModules.SourceGenerator.Impl/DependencyFileWriter.cs b/src/DependencyModules.SourceGenerator.Impl/DependencyFileWriter.cs index 7b12a24..461645d 100644 --- a/src/DependencyModules.SourceGenerator.Impl/DependencyFileWriter.cs +++ b/src/DependencyModules.SourceGenerator.Impl/DependencyFileWriter.cs @@ -225,46 +225,11 @@ private string GenerateDependencyMethod(ModuleEntryPointModel entryPointModel, return method.Name; } - /// - /// The guard for one service's conditions, as it is written into the generated method. - /// - /// - /// Composed as text rather than through CSharpAuthor because the library has no combinators for - /// && or !, and nesting an if per condition to avoid them would emit a - /// staircase for something that reads as one line. The calls themselves are static, so the - /// generated file needs no using. - /// + // Shared with DecoratorFileWriter through EnvironmentConditionWriter, so a service and a + // decorator carrying the same attributes cannot end up testing them differently. private static string BuildCondition( - IReadOnlyList conditions, string environmentParameter) { - - var parts = new List(conditions.Count); - - foreach (var condition in conditions) { - // An empty condition tests nothing; it is reported as DM0012 and left out rather than - // emitted as a call that is constant either way. - if (EnvironmentConditionUtility.IsEmpty(condition)) { - continue; - } - - var call = condition.Kind == EnvironmentConditionKind.Name - ? $"{ConditionsType}.NameIs({environmentParameter}, {QuoteAll(condition.Values)})" - : condition.Values.Count > 0 - ? $"{ConditionsType}.ValueIs({environmentParameter}, {QuoteString(condition.Key!)}, {QuoteString(condition.Values[0])})" - : $"{ConditionsType}.HasValue({environmentParameter}, {QuoteString(condition.Key!)})"; - - parts.Add(condition.Negate ? "!" + call : call); - } - - // Every condition was empty, so there is nothing left to test and the registration is - // unconditional. The diagnostic has already said so. - return parts.Count == 0 ? "true" : string.Join(" && ", parts); - } - - private static string QuoteAll(IReadOnlyList values) => - string.Join(", ", values.Select(QuoteString)); - - private const string ConditionsType = - "global::" + KnownTypes.DependencyModules.Helpers.Namespace + ".EnvironmentConditions"; + IReadOnlyList conditions, string environmentParameter) => + EnvironmentConditionWriter.BuildCondition(conditions, environmentParameter); private void CrossWireRegisterImplementation( DependencyModuleConfigurationModel configurationModel, diff --git a/src/DependencyModules.SourceGenerator.Impl/Models/DecoratorModel.cs b/src/DependencyModules.SourceGenerator.Impl/Models/DecoratorModel.cs index 069ae29..4f8bb5c 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Models/DecoratorModel.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Models/DecoratorModel.cs @@ -13,11 +13,17 @@ namespace DependencyModules.SourceGenerator.Impl.Models; /// module, not only within the declaring one. /// /// Restricts the decorator to one module, matching the service Realm property. +/// +/// Environment conditions read from the decorator class, combining with and exactly as they do +/// on a service. A decorator that does not apply is never invoked, so the service resolves +/// undecorated rather than being wrapped by something that re-tests the environment per call. +/// public record DecoratorModel( ITypeDefinition ServiceType, ITypeDefinition DecoratorType, int Order, - ITypeDefinition? Realm) { + ITypeDefinition? Realm, + IReadOnlyList? Conditions = null) { /// /// Sentinel for a syntax node that carried the attribute but produced no usable model, matching @@ -50,15 +56,24 @@ public bool Equals(DecoratorModel? x, DecoratorModel? y) { return x.Order == y.Order && x.ServiceType.Equals(y.ServiceType) && x.DecoratorType.Equals(y.DecoratorType) && - Equals(x.Realm, y.Realm); + Equals(x.Realm, y.Realm) && + ConditionsEqual(x.Conditions, y.Conditions); } + // Structural rather than by reference: two runs build separate lists, so comparing references + // would miss the cache on every keystroke and re-emit every decorator. + private static bool ConditionsEqual( + IReadOnlyList? x, + IReadOnlyList? y) => + (x?.Count ?? 0) == 0 && (y?.Count ?? 0) == 0 || ModelEquality.ListEquals(x, y); + public int GetHashCode(DecoratorModel obj) { unchecked { var hash = obj.ServiceType.GetHashCode(); hash = hash * 31 + obj.DecoratorType.GetHashCode(); hash = hash * 31 + obj.Order; hash = hash * 31 + (obj.Realm?.GetHashCode() ?? 0); + hash = hash * 31 + ModelEquality.ListHashCode(obj.Conditions); return hash; } diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/DecoratorModelUtility.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/DecoratorModelUtility.cs index 39a7317..219318a 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Utilities/DecoratorModelUtility.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/DecoratorModelUtility.cs @@ -65,7 +65,12 @@ public static class DecoratorModelUtility { serviceType = ToUnboundGeneric(serviceType); } - return new DecoratorModel(serviceType, decoratorType, order, realm); + // Read from the decorator class, exactly as they are for a service. A decorator is a + // registration like any other, and one gated on Development has no other way to say so. + var conditions = EnvironmentConditionUtility.GetConditions( + context, typeDeclarationSyntax, cancellationToken); + + return new DecoratorModel(serviceType, decoratorType, order, realm, conditions); } /// diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/EnvironmentConditionWriter.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/EnvironmentConditionWriter.cs new file mode 100644 index 0000000..54659ef --- /dev/null +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/EnvironmentConditionWriter.cs @@ -0,0 +1,57 @@ +using DependencyModules.SourceGenerator.Impl.Models; +using static CSharpAuthor.SyntaxHelpers; + +namespace DependencyModules.SourceGenerator.Impl.Utilities; + +/// +/// Renders a set of environment conditions as the guard written into generated code. +/// +/// +/// Shared by every writer that emits something conditional — service registrations and decorators — +/// so the two cannot drift into testing the same attributes differently. +/// +public static class EnvironmentConditionWriter { + + private const string ConditionsType = + "global::" + KnownTypes.DependencyModules.Helpers.Namespace + ".EnvironmentConditions"; + + /// + /// The guard for one declaration's conditions, as it is written into the generated method. + /// + /// + /// Composed as text rather than through CSharpAuthor because the library has no combinators for + /// && or !, and nesting an if per condition to avoid them would emit a + /// staircase for something that reads as one line. The calls themselves are static, so the + /// generated file needs no using. + /// + /// Conditions to test, combined with and. + /// Name of the IModuleEnvironment parameter in scope. + public static string BuildCondition( + IReadOnlyList conditions, string environmentParameter) { + + var parts = new List(conditions.Count); + + foreach (var condition in conditions) { + // An empty condition tests nothing; it is reported as DM0012 and left out rather than + // emitted as a call that is constant either way. + if (EnvironmentConditionUtility.IsEmpty(condition)) { + continue; + } + + var call = condition.Kind == EnvironmentConditionKind.Name + ? $"{ConditionsType}.NameIs({environmentParameter}, {QuoteAll(condition.Values)})" + : condition.Values.Count > 0 + ? $"{ConditionsType}.ValueIs({environmentParameter}, {QuoteString(condition.Key!)}, {QuoteString(condition.Values[0])})" + : $"{ConditionsType}.HasValue({environmentParameter}, {QuoteString(condition.Key!)})"; + + parts.Add(condition.Negate ? "!" + call : call); + } + + // Every condition was empty, so there is nothing left to test and the declaration is + // unconditional. The diagnostic has already said so. + return parts.Count == 0 ? "true" : string.Join(" && ", parts); + } + + private static string QuoteAll(IReadOnlyList values) => + string.Join(", ", values.Select(QuoteString)); +} diff --git a/src/DependencyModules.Testing/Attributes/Interfaces/IServiceProviderBuilderAttribute.cs b/src/DependencyModules.Testing/Attributes/Interfaces/IServiceProviderBuilderAttribute.cs new file mode 100644 index 0000000..85ffe6c --- /dev/null +++ b/src/DependencyModules.Testing/Attributes/Interfaces/IServiceProviderBuilderAttribute.cs @@ -0,0 +1,26 @@ +using Microsoft.Extensions.DependencyInjection; + +namespace DependencyModules.Testing.Attributes.Interfaces; + +/// +/// Replaces the container a test runs against. +/// +/// +/// Found by walking method, class and assembly, and the first one found wins — unlike the other +/// hooks, which all contribute. Without one the collection is built with +/// BuildServiceProvider(). +/// +/// Implement this to hand the test a third-party container, or to build the default one with options +/// it would not otherwise get, such as scope validation. It runs last, after every other hook has +/// contributed, so it is also the final chance to inspect or amend the collection. +/// +public interface IServiceProviderBuilderAttribute { + + /// + /// Builds the container for the test. + /// + /// The test the container is being built for. + /// The fully populated collection. + /// The container the test resolves its parameters and services from. + IServiceProvider BuildServiceProvider(ITestMethodContext testMethod, IServiceCollection serviceCollection); +} diff --git a/src/DependencyModules.Testing/Attributes/Interfaces/ITestMethodContext.cs b/src/DependencyModules.Testing/Attributes/Interfaces/ITestMethodContext.cs new file mode 100644 index 0000000..11c0ff1 --- /dev/null +++ b/src/DependencyModules.Testing/Attributes/Interfaces/ITestMethodContext.cs @@ -0,0 +1,44 @@ +using System.Reflection; + +namespace DependencyModules.Testing.Attributes.Interfaces; + +/// +/// The test method a container is being built for. +/// +/// +/// Every test framework has its own model of a test method — xUnit v3 has IXunitTestMethod, +/// carrying identity that survives serialization, merged traits, and generic resolution alongside the +/// reflection. None of that is needed to decide what to register in a test's container, and taking a +/// dependency on it would bind every mocking package to a single test framework. This carries the part +/// that is common; a framework integration supplies its own implementation over its own model. +/// +public interface ITestMethodContext { + + /// + /// The method under test. + /// + /// + /// A real rather than an abstraction over one. The extension methods in + /// AttributeUtility hang off this, and it is the same instance the framework integration + /// reads parameters from, so a hook sees exactly the signature the test will be invoked with. + /// + MethodInfo Method { + get; + } + + /// + /// Every attribute in scope for the method, widest scope first: assembly, then declaring type, + /// then the method itself. + /// + /// + /// The integration has already walked and ordered these, so an implementation looking for one can + /// read the list rather than paying for the walk again. + /// + /// Note the ordering is the reverse of AttributeUtility.GetTestAttribute, which answers + /// "the most specific one wins" and so looks at the method first. This list is in the order things + /// are applied, where the most specific runs last and therefore wins. + /// + IReadOnlyList Attributes { + get; + } +} diff --git a/src/DependencyModules.Testing/Attributes/Interfaces/ITestParameterValueProvider.cs b/src/DependencyModules.Testing/Attributes/Interfaces/ITestParameterValueProvider.cs new file mode 100644 index 0000000..3c2248a --- /dev/null +++ b/src/DependencyModules.Testing/Attributes/Interfaces/ITestParameterValueProvider.cs @@ -0,0 +1,41 @@ +using System.Reflection; +using Microsoft.Extensions.DependencyInjection; + +namespace DependencyModules.Testing.Attributes.Interfaces; + +/// +/// Supplies the value for a single test method parameter, along with any services that value depends +/// on. +/// +/// +/// Applies to the parameter itself rather than being found by walking the attribute chain, so it +/// speaks for one parameter only. [Mock] is the canonical implementation. +/// +/// The two halves run at different points, and the gap between them is the whole point: the setup +/// runs before the container is built, so a parameter can change what the service under test is +/// constructed with, not merely what the test itself ends up holding. +/// +public interface ITestParameterValueProvider { + + /// + /// Adds whatever services are needed to supply this parameter. + /// + /// The test the container is being built for. + /// The collection backing the test's container. + /// The parameter being supplied. + void SetupServiceCollection( + ITestMethodContext testMethod, IServiceCollection serviceCollection, ParameterInfo parameter); + + /// + /// Produces the value to pass for this parameter. + /// + /// The test being run. + /// The test's container, fully built. + /// The parameter being supplied. + /// + /// The value, or null to stand aside — the next provider on the parameter is tried, and failing + /// that the parameter is resolved from the container like any other. + /// + Task GetParameterValueAsync( + ITestMethodContext testMethod, IServiceProvider serviceProvider, ParameterInfo parameter); +} diff --git a/src/DependencyModules.Testing/Attributes/Interfaces/ITestServiceSetupAttribute.cs b/src/DependencyModules.Testing/Attributes/Interfaces/ITestServiceSetupAttribute.cs new file mode 100644 index 0000000..db2cdb0 --- /dev/null +++ b/src/DependencyModules.Testing/Attributes/Interfaces/ITestServiceSetupAttribute.cs @@ -0,0 +1,28 @@ +using Microsoft.Extensions.DependencyInjection; + +namespace DependencyModules.Testing.Attributes.Interfaces; + +/// +/// Contributes service registrations to a test's container, with the test method in hand. +/// +/// +/// Applies to a method, a class or an assembly and is found by walking that chain, so a registration +/// every test needs can be declared once in an AssemblyInfo. +/// +/// Because the method is supplied, an implementation can register based on what the test actually +/// asked for rather than only on how the attribute was configured. DependencyModules.Moq uses +/// this to spot a Mock<T> parameter and register both the mock and the object it +/// produces, so the test holds the one and the service under test is built against the other. +/// +/// Runs before the container is built, in widest-scope-first order. Registrations are last-one-wins, +/// so an attribute on the method overrides the same service registered from the assembly. +/// +public interface ITestServiceSetupAttribute { + + /// + /// Adds services for the given test. + /// + /// The test the container is being built for. + /// The collection backing the test's container. + void SetupServiceCollection(ITestMethodContext testMethod, IServiceCollection serviceCollection); +} diff --git a/src/DependencyModules.Testing/Attributes/Interfaces/ITestStartupAttribute.cs b/src/DependencyModules.Testing/Attributes/Interfaces/ITestStartupAttribute.cs new file mode 100644 index 0000000..ff2ef4b --- /dev/null +++ b/src/DependencyModules.Testing/Attributes/Interfaces/ITestStartupAttribute.cs @@ -0,0 +1,24 @@ +namespace DependencyModules.Testing.Attributes.Interfaces; + +/// +/// Runs against a test's container once it has been built, before the test method is invoked. +/// +/// +/// Separate from because the two happen at different points +/// and most attributes want only one of them: registering services needs a collection and no +/// provider, while seeding state needs a provider and is too late to change registrations. An +/// attribute that genuinely does both implements both. +/// +/// Applies to a method, a class or an assembly, and is found by walking that chain. +/// +public interface ITestStartupAttribute { + + /// + /// Performs whatever asynchronous setup the test needs — seeding a store, opening a connection, + /// priming a cache. + /// + /// The test being prepared. + /// The test's container, fully built. + /// A task that completes when the test is ready to run. + Task StartupAsync(ITestMethodContext testMethod, IServiceProvider serviceProvider); +} diff --git a/src/DependencyModules.Testing/DependencyModules.Testing.csproj b/src/DependencyModules.Testing/DependencyModules.Testing.csproj index ba25fcb..aaef92e 100644 --- a/src/DependencyModules.Testing/DependencyModules.Testing.csproj +++ b/src/DependencyModules.Testing/DependencyModules.Testing.csproj @@ -1,7 +1,7 @@  - net8.0 + $(LibraryTargetFrameworks) enable enable True @@ -10,4 +10,13 @@ true + + + + + + + + + diff --git a/src/DependencyModules.xUnit/Attributes/Interfaces/IServiceProviderBuilderAttribute.cs b/src/DependencyModules.xUnit/Attributes/Interfaces/IServiceProviderBuilderAttribute.cs deleted file mode 100644 index 557393b..0000000 --- a/src/DependencyModules.xUnit/Attributes/Interfaces/IServiceProviderBuilderAttribute.cs +++ /dev/null @@ -1,32 +0,0 @@ -using Microsoft.Extensions.DependencyInjection; -using Xunit.v3; - -namespace DependencyModules.xUnit.Attributes.Interfaces; - -/// -/// Represents an interface for defining a mechanism to build an -/// within the context of a test method in the xUnit testing framework. -/// Implementations of this interface can be used to customize the service provider for a specific test case, -/// providing test dependencies by configuring the . -/// -public interface IServiceProviderBuilderAttribute { - - /// - /// Builds an based on the provided and optional attributes. - /// This method checks for custom attributes implementing to allow - /// test-specific service provider customization. If no applicable attribute is found, the default - /// service provider is built using the given service collection. - /// - /// - /// The instance containing service registrations to be used for building the service provider. - /// - /// - /// the test case context for the test - /// to customize the service provider construction. - /// - /// - /// An instance of configured with the services from the given , - /// optionally customized by a . - /// - IServiceProvider BuildServiceProvider(IXunitTestMethod testCaseContext, IServiceCollection serviceCollection); -} \ No newline at end of file diff --git a/src/DependencyModules.xUnit/Attributes/Interfaces/ITestParameterValueProvider.cs b/src/DependencyModules.xUnit/Attributes/Interfaces/ITestParameterValueProvider.cs deleted file mode 100644 index 0708452..0000000 --- a/src/DependencyModules.xUnit/Attributes/Interfaces/ITestParameterValueProvider.cs +++ /dev/null @@ -1,55 +0,0 @@ -using System.Reflection; -using Microsoft.Extensions.DependencyInjection; -using Xunit.v3; - -namespace DependencyModules.xUnit.Attributes.Interfaces; - -/// -/// Interface that defines the contract for providing custom parameter values for test methods in xUnit. -/// -/// -/// Implementations of this interface can modify the dependency injection service collection for a test method's -/// execution context and provide specific values for method parameters at runtime. This enables customization of -/// test cases by dynamically resolving parameter dependencies or injecting mock/fake objects. -/// -public interface ITestParameterValueProvider { - /// - /// Configures the dependency injection service collection for a test method's execution context. - /// - /// - /// An instance that represents the test method's execution context. - /// This provides metadata about the test method, including its attributes and parameters. - /// - /// - /// An instance of that defines the service collection for the test execution. - /// Services can be added or modified within this method. - /// - /// - /// A object representing the specific parameter of the test method - /// for which the dependency injection setup is being customized. - /// This allows fine-grained control over services related to specific parameters. - /// - void SetupServiceCollection(IXunitTestMethod testCaseContext, IServiceCollection serviceCollection, ParameterInfo parameter); - - /// - /// Asynchronously retrieves the value of a test method parameter at runtime based on the provided context, - /// service provider, and parameter metadata. - /// - /// - /// An instance of representing the context of the test method execution. - /// This provides information such as the test method's attributes, signatures, and parameters. - /// - /// - /// An instance of used to resolve services or dependencies - /// required for the parameter value generation. - /// - /// - /// An instance of that contains metadata about the parameter - /// for which the value is to be retrieved. - /// - /// - /// A that represents the asynchronous operation. - /// The result contains the parameter value, or null if no value could be resolved. - /// - Task GetParameterValueAsync(IXunitTestMethod context, IServiceProvider serviceProvider, ParameterInfo parameter); -} \ No newline at end of file diff --git a/src/DependencyModules.xUnit/Attributes/Interfaces/ITestStartupAttribute.cs b/src/DependencyModules.xUnit/Attributes/Interfaces/ITestStartupAttribute.cs deleted file mode 100644 index 2d262ba..0000000 --- a/src/DependencyModules.xUnit/Attributes/Interfaces/ITestStartupAttribute.cs +++ /dev/null @@ -1,49 +0,0 @@ -using Microsoft.Extensions.DependencyInjection; -using Xunit.v3; - -namespace DependencyModules.xUnit.Attributes.Interfaces; - -/// -/// Represents an interface for configuring and initializing services within the xUnit testing framework. -/// -/// -/// Implementations of this interface allow services to be added to the dependency injection container -/// and provide mechanisms to perform additional initialization steps required for test execution. -/// -/// -/// Thread safety is not guaranteed for implementations of this interface. Procedures should account for -/// potential issues in concurrent test execution scenarios. -/// -public interface ITestStartupAttribute { - - /// - /// Configures the dependency injection service collection by adding services or modifying the collection - /// specifically for a test method within the xUnit testing framework. - /// - /// - /// The test method for which the service collection is being configured. Provides context about the test - /// being executed. - /// - /// - /// The service collection to be configured. This can involve adding, replacing, or removing services - /// necessary for the test execution. - /// - void SetupServiceCollection(IXunitTestMethod testMethod, IServiceCollection serviceCollection); - - /// - /// Asynchronously initializes and configures services required for the execution of a specific test - /// within the xUnit testing framework. - /// - /// - /// The test method that is associated with the current initialization process. Provides the necessary - /// context for configuring the services specific to the test. - /// - /// - /// The service provider that offers access to the configured dependency injection container. - /// It enables the retrieval of services for initialization or further setup. - /// - /// - /// A task that represents the asynchronous operation of initializing and configuring the services. - /// - Task StartupAsync(IXunitTestMethod testMethod, IServiceProvider serviceProvider); -} \ No newline at end of file diff --git a/src/DependencyModules.xUnit/Attributes/MockAttribute.cs b/src/DependencyModules.xUnit/Attributes/MockAttribute.cs index 4d53056..a9d55b0 100644 --- a/src/DependencyModules.xUnit/Attributes/MockAttribute.cs +++ b/src/DependencyModules.xUnit/Attributes/MockAttribute.cs @@ -1,10 +1,7 @@ using System.Reflection; using DependencyModules.Testing.Attributes.Interfaces; using DependencyModules.Testing.Impl; -using DependencyModules.xUnit.Attributes.Interfaces; -using DependencyModules.xUnit.Impl; using Microsoft.Extensions.DependencyInjection; -using Xunit.v3; namespace DependencyModules.xUnit.Attributes; @@ -24,8 +21,8 @@ public class MockAttribute : Attribute, ITestParameterValueProvider { /// /// Configures a service collection with necessary dependencies for the test case context. /// - /// - /// The xUnit test method context providing access to test-related information and behavior. + /// + /// The test method context providing access to test-related information and behavior. /// /// /// The service collection to configure with services and dependencies required for the test. @@ -36,8 +33,9 @@ public class MockAttribute : Attribute, ITestParameterValueProvider { /// /// Thrown when a required mock library is not found, indicating that the type or assembly is not correctly attributed. /// - public void SetupServiceCollection(IXunitTestMethod testCaseContext, IServiceCollection serviceCollection, ParameterInfo parameter) { - var mockAttribute = testCaseContext.Method.GetTestAttribute(); + public void SetupServiceCollection( + ITestMethodContext testMethod, IServiceCollection serviceCollection, ParameterInfo parameter) { + var mockAttribute = testMethod.Method.GetTestAttribute(); if (mockAttribute == null) { throw new Exception("Mock library not found, please ensure the Type or Assembly is attributed correctly."); @@ -51,7 +49,7 @@ public void SetupServiceCollection(IXunitTestMethod testCaseContext, IServiceCol /// /// Retrieves the parameter value asynchronously using the provided context, service provider, and parameter information. /// - /// + /// /// The test method execution context containing metadata and runtime details of the test case. /// /// @@ -64,7 +62,8 @@ public void SetupServiceCollection(IXunitTestMethod testCaseContext, IServiceCol /// A task that represents the asynchronous operation. The task result contains the resolved value of the parameter, /// or null if the parameter could not be resolved. /// - public Task GetParameterValueAsync(IXunitTestMethod context, IServiceProvider serviceProvider, ParameterInfo parameter) { + public Task GetParameterValueAsync( + ITestMethodContext testMethod, IServiceProvider serviceProvider, ParameterInfo parameter) { return Task.FromResult(serviceProvider.GetService(parameter.ParameterType)); } } \ No newline at end of file diff --git a/src/DependencyModules.xUnit/Attributes/ModuleTestAttribute.cs b/src/DependencyModules.xUnit/Attributes/ModuleTestAttribute.cs index dfae56d..f275738 100644 --- a/src/DependencyModules.xUnit/Attributes/ModuleTestAttribute.cs +++ b/src/DependencyModules.xUnit/Attributes/ModuleTestAttribute.cs @@ -1,3 +1,4 @@ +using System.Runtime.CompilerServices; using DependencyModules.xUnit.Impl; using Xunit; using Xunit.v3; @@ -19,7 +20,51 @@ namespace DependencyModules.xUnit.Attributes; /// [XunitTestCaseDiscoverer(typeof(ModuleTestDiscoverer))] [AttributeUsage(AttributeTargets.Method)] -public class ModuleTestAttribute(params Type[] modules) : FactAttribute { +public class ModuleTestAttribute : FactAttribute { + + /// + /// Marks a test method, taking no modules. + /// + /// + /// The source parameters are never passed by hand — the compiler fills them in with the + /// location of the attribute usage, which is how a test reports where it is declared. They are + /// what FactAttribute exists to capture for a derived attribute, and leaving them off + /// makes every module test claim to live at this file instead of at the test. + /// + public ModuleTestAttribute( + [CallerFilePath] string? sourceFilePath = null, + [CallerLineNumber] int sourceLineNumber = -1) + : base(sourceFilePath, sourceLineNumber) => + ModuleTypes = []; + + /// + /// Marks a test method and names one module to configure the test's container with. + /// + /// + /// Separate from the params overload rather than folded into it, because a params array has to + /// be the last parameter and the caller-info parameters have to come after it — the two cannot + /// coexist in one signature. Overload resolution prefers this normal form over expanding the + /// params one, so the single-module case, which is the common one, keeps its source location. + /// + public ModuleTestAttribute( + Type module, + [CallerFilePath] string? sourceFilePath = null, + [CallerLineNumber] int sourceLineNumber = -1) + : base(sourceFilePath, sourceLineNumber) => + ModuleTypes = [module]; + + /// + /// Marks a test method and names several modules to configure the test's container with. + /// + /// + /// Two or more modules land here, and this overload cannot capture a source location for the + /// reason above: C# will not accept caller-info parameters after a params array. Such a test + /// still runs and still reports correctly; only navigation from a test explorer back to the + /// source is unavailable. Naming one module, or none, takes an overload that does capture it. + /// + public ModuleTestAttribute(params Type[] modules) => + ModuleTypes = modules; + /// /// Gets an array of module types associated with the test method decorated with /// the . @@ -31,5 +76,5 @@ public class ModuleTestAttribute(params Type[] modules) : FactAttribute { /// public Type[] ModuleTypes { get; - } = modules; + } } \ No newline at end of file diff --git a/src/DependencyModules.xUnit/Attributes/TestExportAttribute.cs b/src/DependencyModules.xUnit/Attributes/TestExportAttribute.cs index 83dd716..bb08472 100644 --- a/src/DependencyModules.xUnit/Attributes/TestExportAttribute.cs +++ b/src/DependencyModules.xUnit/Attributes/TestExportAttribute.cs @@ -1,6 +1,5 @@ -using DependencyModules.xUnit.Attributes.Interfaces; +using DependencyModules.Testing.Attributes.Interfaces; using Microsoft.Extensions.DependencyInjection; -using Xunit.v3; namespace DependencyModules.xUnit.Attributes; @@ -11,9 +10,8 @@ namespace DependencyModules.xUnit.Attributes; /// /// /// This attribute can be applied to assemblies, classes, or methods to provide granular service -/// configuration for specific testing contexts. It integrates with xUnit by implementing the -/// ITestStartupAttribute, enabling setup and initialization processes within the xUnit testing -/// framework. +/// configuration for specific testing contexts. It registers through +/// , which carries no test framework dependency. /// /// /// It enables dependency injection for testing by adding services to the service collection @@ -23,13 +21,13 @@ namespace DependencyModules.xUnit.Attributes; /// This attribute does not guarantee thread safety and should be used with appropriate considerations /// in concurrent test scenarios. /// -/// +/// [AttributeUsage( AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true)] -public class TestExportAttribute : Attribute, ITestStartupAttribute { +public class TestExportAttribute : Attribute, ITestServiceSetupAttribute { /// /// An attribute that configures and exports services to the dependency injection container /// for xUnit test scenarios. This supports customized service registrations with specific lifetimes @@ -81,14 +79,14 @@ public ServiceLifetime Lifetime { /// This method enables dynamic service registration during test execution, supporting dependency injection setup. /// /// - /// The xUnit test method for which the service collection is being configured. + /// The test method for which the service collection is being configured. /// This parameter provides context about the test and can be used for conditional service registrations. /// /// /// The service collection to which services are added. This collection is used to configure /// the dependency injection container for the test's execution environment. /// - public void SetupServiceCollection(IXunitTestMethod testMethod, IServiceCollection serviceCollection) { + public void SetupServiceCollection(ITestMethodContext testMethod, IServiceCollection serviceCollection) { var implementation = Implementation ?? Service; switch (Lifetime) { @@ -103,24 +101,4 @@ public void SetupServiceCollection(IXunitTestMethod testMethod, IServiceCollecti break; } } - - /// - /// Asynchronously initializes services and configurations required for a specific xUnit test method - /// by using the provided test method context and service provider. - /// - /// - /// The instance of the xUnit test method being executed. It provides context - /// for the current test, such as metadata and test execution information. - /// - /// - /// The service provider used to resolve dependencies and manage the service lifecycle - /// during the test execution. - /// - /// - /// A task that represents the asynchronous initialization operation. The task completes - /// when the service initialization and configurations for the test method are finalized. - /// - public Task StartupAsync(IXunitTestMethod testMethod, IServiceProvider serviceProvider) { - return Task.CompletedTask; - } } \ No newline at end of file diff --git a/src/DependencyModules.xUnit/DependencyModules.xUnit.csproj b/src/DependencyModules.xUnit/DependencyModules.xUnit.csproj index df39ed7..a2a4111 100644 --- a/src/DependencyModules.xUnit/DependencyModules.xUnit.csproj +++ b/src/DependencyModules.xUnit/DependencyModules.xUnit.csproj @@ -1,35 +1,42 @@  - net8.0 + $(LibraryTargetFrameworks) enable enable True - - Library - false DependencyModules.xUnit xUnit v3 integration for DependencyModules. Provides the [ModuleTest] attribute, which builds a service provider from your modules and injects the services a test method asks for, plus attributes for per-test service overrides and value injection. true - + + + + + + + + + + + + - - + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + diff --git a/src/DependencyModules.xUnit/Impl/ModuleTestCase.cs b/src/DependencyModules.xUnit/Impl/ModuleTestCase.cs index 020aafe..b86ede9 100644 --- a/src/DependencyModules.xUnit/Impl/ModuleTestCase.cs +++ b/src/DependencyModules.xUnit/Impl/ModuleTestCase.cs @@ -4,9 +4,7 @@ using DependencyModules.xUnit.Attributes; using DependencyModules.Testing.Attributes.Interfaces; using DependencyModules.Testing.Impl; -using DependencyModules.xUnit.Attributes.Interfaces; using Microsoft.Extensions.DependencyInjection; -using Xunit.Internal; using Xunit.Sdk; using Xunit.v3; @@ -35,6 +33,7 @@ public ModuleTestCase( string testCaseDisplayName, string uniqueID, bool @explicit, + Type[]? skipExceptions = null, string? skipReason = null, Type? skipType = null, string? skipUnless = null, @@ -44,19 +43,25 @@ public ModuleTestCase( string? sourceFilePath = null, int? sourceLineNumber = null, int? timeout = null) : base( - testMethod, - testCaseDisplayName, - uniqueID, - @explicit, - skipReason, - skipType, - skipUnless, - skipWhen, - traits, - testMethodArguments, - sourceFilePath, - sourceLineNumber, - timeout) { } + // Named rather than positional throughout. XunitTestCase's constructor takes thirteen + // parameters, eleven of them optional, and a version that inserts one mid-list rebinds + // every argument after it — silently where the types happen to line up, and as a wall of + // unrelated-looking conversion errors where they do not. Named arguments make an insertion + // either invisible or a single precise error. + testMethod: testMethod, + testCaseDisplayName: testCaseDisplayName, + uniqueID: uniqueID, + @explicit: @explicit, + skipExceptions: skipExceptions, + skipReason: skipReason, + skipType: skipType, + skipUnless: skipUnless, + skipWhen: skipWhen, + traits: traits, + testMethodArguments: testMethodArguments, + sourceFilePath: sourceFilePath, + sourceLineNumber: sourceLineNumber, + timeout: timeout) { } /// /// Executes logic before the invocation of the test method associated with the current test case. @@ -65,32 +70,35 @@ public ModuleTestCase( public override void PreInvoke() { } private record StartupValues( - IServiceProvider ServiceProvider, + ITestMethodContext Context, + IServiceProvider ServiceProvider, Dictionary> KnownValues); - + private async Task SetupServiceCollection() { var serviceCollection = new ServiceCollection(); var knownValues = new Dictionary>(); - + var knownAttributes = TestMethod.Method.GetTestAttributes().ToArray(); - + + var context = new XunitTestMethodContext(TestMethod, knownAttributes); + SetupTestCaseInfo(serviceCollection, knownAttributes); - + SetupModules(serviceCollection, knownAttributes); - SetValueProviders(serviceCollection, knownValues); + SetValueProviders(context, serviceCollection, knownValues); - var startupAttributes = SetupStartupAttributes(serviceCollection, knownAttributes); + SetupServiceSetupAttributes(context, serviceCollection, knownAttributes); + + var provider = BuildServiceProvider(context, serviceCollection, knownAttributes); - var provider = BuildServiceProvider(serviceCollection, knownAttributes); - DisposalTracker.Add(provider); - foreach (var startupAttribute in startupAttributes) { - await startupAttribute.StartupAsync(this.TestMethod, provider); + foreach (var startupAttribute in knownAttributes.OfType()) { + await startupAttribute.StartupAsync(context, provider); } - - return new StartupValues(provider, knownValues); + + return new StartupValues(context, provider, knownValues); } private void SetupTestCaseInfo(ServiceCollection serviceCollection, Attribute[] knownAttributes) { @@ -103,34 +111,49 @@ private void SetupTestCaseInfo(ServiceCollection serviceCollection, Attribute[] )); } - private IServiceProvider BuildServiceProvider(ServiceCollection serviceCollection, Attribute[] knownAttributes) { + private IServiceProvider BuildServiceProvider( + ITestMethodContext context, ServiceCollection serviceCollection, Attribute[] knownAttributes) { var serviceProviderBuilderAttribute = knownAttributes.OfType().FirstOrDefault(); if (serviceProviderBuilderAttribute != null) { - return serviceProviderBuilderAttribute.BuildServiceProvider(TestMethod, serviceCollection); + return serviceProviderBuilderAttribute.BuildServiceProvider(context, serviceCollection); } - + return serviceCollection.BuildServiceProvider(); } - private IReadOnlyList SetupStartupAttributes(ServiceCollection serviceCollection, Attribute[] knownAttributes) { - var startupAttributes = new List(); - foreach (var testStartupAttribute in knownAttributes.OfType()) { - testStartupAttribute.SetupServiceCollection(TestMethod, serviceCollection); - startupAttributes.Add(testStartupAttribute); + /// + /// The whole pass runs after the parameter value providers, so a [TestExport] overrides a + /// [Mock] of the same service rather than the other way round. + /// + /// Mock support goes first within the pass, everything else keeping its declared order behind it. + /// A mock is the stand-in a test falls back to, so naming a real implementation has to beat it — + /// and has to beat it whether [MoqSupport] sits on the assembly, the class or the method, + /// which relying on attribute order alone would not guarantee. + /// + private void SetupServiceSetupAttributes( + ITestMethodContext context, ServiceCollection serviceCollection, Attribute[] knownAttributes) { + var setupAttributes = knownAttributes + .OfType() + .OrderBy(attribute => attribute is IMockSupportAttribute ? 0 : 1); + + foreach (var setupAttribute in setupAttributes) { + setupAttribute.SetupServiceCollection(context, serviceCollection); } - return startupAttributes; } - private void SetValueProviders(ServiceCollection serviceCollection, Dictionary> knownValues) { + private void SetValueProviders( + ITestMethodContext context, + ServiceCollection serviceCollection, + Dictionary> knownValues) { foreach (var parameterInfo in TestMethod.Method.GetParameters()) { var list = new List(); knownValues.Add(parameterInfo, list); foreach (var valueProvider in parameterInfo.GetCustomAttributes().OfType()) { - valueProvider.SetupServiceCollection(TestMethod, serviceCollection, parameterInfo); + valueProvider.SetupServiceCollection(context, serviceCollection, parameterInfo); list.Add(valueProvider); } } @@ -191,16 +214,26 @@ private async Task> UnitTestFromDataAttributes(I var startupValues = await SetupServiceCollection(); unitTests.Add( + // testIndex is named for more than readability: XunitTest has a second + // nine-parameter constructor differing only at this position, taking a uniqueID + // string. Positionally the two are told apart by the argument's type alone. new XunitTest( - this, - TestMethod, - Explicit, - theoryDataRow.Skip ?? SkipReason, - GetRowDisplayName(theoryDataRow, data), - unitTests.Count, - theoryDataRow.Traits?.ToReadOnly() ?? Traits.ToReadOnly(), - theoryDataRow.Timeout ?? Timeout, - await ResolveArguments(data, startupValues) + testCase: this, + testMethod: TestMethod, + @explicit: Explicit, + skipReason: theoryDataRow.Skip ?? SkipReason, + // The row's own conditional-skip metadata takes precedence over the case's, + // matching how skipReason above already defers to it. These became required + // in xunit.v3 3.x; passing the case's values alone would have compiled and + // silently ignored [Theory]-style per-row skip conditions. + skipType: theoryDataRow.SkipType ?? SkipType, + skipUnless: theoryDataRow.SkipUnless ?? SkipUnless, + skipWhen: theoryDataRow.SkipWhen ?? SkipWhen, + testDisplayName: GetRowDisplayName(theoryDataRow, data), + testIndex: unitTests.Count, + traits: theoryDataRow.Traits?.ToReadOnlyTraits() ?? Traits.ToReadOnlyTraits(), + timeout: theoryDataRow.Timeout ?? Timeout, + testMethodArguments: await ResolveArguments(data, startupValues) ) ); } @@ -222,7 +255,13 @@ await ResolveArguments(data, startupValues) private string GetRowDisplayName(Xunit.ITheoryDataRow theoryDataRow, object?[] data) { var baseDisplayName = theoryDataRow.TestDisplayName ?? TestCaseDisplayName; - return TestMethod.GetDisplayName(baseDisplayName, data, null); + return TestMethod.GetDisplayName( + baseDisplayName: baseDisplayName, + // New in 3.x. The row may carry its own label, and xUnit folds it into the display name + // for [Theory]; passing null would compile and quietly drop it for module tests. + label: theoryDataRow.Label, + testMethodArguments: data, + methodGenericTypes: null); } private async Task> UnitTestWithNoDataAttributes() { @@ -230,15 +269,18 @@ private async Task> UnitTestWithNoDataAttributes return [ new XunitTest( - this, - TestMethod, - Explicit, - SkipReason, - TestCaseDisplayName, - 0, - Traits.ToReadOnly(), - Timeout, - await ResolveArguments([], startupValues) + testCase: this, + testMethod: TestMethod, + @explicit: Explicit, + skipReason: SkipReason, + skipType: SkipType, + skipUnless: SkipUnless, + skipWhen: SkipWhen, + testDisplayName: TestCaseDisplayName, + testIndex: 0, + traits: Traits.ToReadOnlyTraits(), + timeout: Timeout, + testMethodArguments: await ResolveArguments([], startupValues) ) ]; } @@ -272,7 +314,8 @@ await ResolveArguments([], startupValues) } else { foreach (var valueProvider in startupValues.KnownValues[parameterInfo]) { - value = await valueProvider.GetParameterValueAsync(TestMethod, startupValues.ServiceProvider, parameterInfo); + value = await valueProvider.GetParameterValueAsync( + startupValues.Context, startupValues.ServiceProvider, parameterInfo); if (value != null) { break; diff --git a/src/DependencyModules.xUnit/Impl/ModuleTestDiscoverer.cs b/src/DependencyModules.xUnit/Impl/ModuleTestDiscoverer.cs index 48833f9..c8d0cb6 100644 --- a/src/DependencyModules.xUnit/Impl/ModuleTestDiscoverer.cs +++ b/src/DependencyModules.xUnit/Impl/ModuleTestDiscoverer.cs @@ -1,5 +1,4 @@ using DependencyModules.xUnit.Attributes; -using Xunit.Internal; using Xunit.Sdk; using Xunit.v3; @@ -40,20 +39,36 @@ public ValueTask> Discover( // name is not unique across test classes, and xUnit silently drops a test case whose ID // collides with one already discovered. This also picks up display name formatting, the // skip and explicit attributes, and the timeout, consistently with [Fact] and [Theory]. - var details = TestIntrospectionHelper.GetTestCaseDetails(discoveryOptions, testMethod, factAttribute); + // + // label is named to pick an overload, not because a value is wanted. xunit.v3 3.x added a + // second GetTestCaseDetails taking a trailing label, and since every added parameter on + // both is optional, a three-argument call matches the two equally well and is ambiguous. + // Naming a parameter only the newer one declares resolves it. A module test has no label, + // which is what null says. + var details = TestIntrospectionHelper.GetTestCaseDetails( + discoveryOptions, testMethod, factAttribute, label: null); return new ValueTask>( new[] { new ModuleTestCase( - details.ResolvedTestMethod, - details.TestCaseDisplayName, - details.UniqueID, - details.Explicit, - details.SkipReason, - details.SkipType, - details.SkipUnless, - details.SkipWhen, - testMethod.Traits.ToReadWrite(StringComparer.OrdinalIgnoreCase), + testMethod: details.ResolvedTestMethod, + testCaseDisplayName: details.TestCaseDisplayName, + uniqueID: details.UniqueID, + @explicit: details.Explicit, + // New in 3.x, and forwarded rather than defaulted: introspection already reads + // [Fact(SkipExceptions = …)] off the attribute, so dropping it here would leave + // [ModuleTest] silently ignoring a skip condition that [Fact] honours. + skipExceptions: details.SkipExceptions, + skipReason: details.SkipReason, + skipType: details.SkipType, + skipUnless: details.SkipUnless, + skipWhen: details.SkipWhen, + traits: testMethod.Traits.ToWritableTraits(StringComparer.OrdinalIgnoreCase), + // Introspection reads these off the attribute, which captures them from the + // usage site. Not forwarding them left every module test without a source + // location, so a test explorer had nowhere to navigate to. + sourceFilePath: details.SourceFilePath, + sourceLineNumber: details.SourceLineNumber, timeout: details.Timeout ) } diff --git a/src/DependencyModules.xUnit/Impl/TraitDictionaryExtensions.cs b/src/DependencyModules.xUnit/Impl/TraitDictionaryExtensions.cs new file mode 100644 index 0000000..c72ff9f --- /dev/null +++ b/src/DependencyModules.xUnit/Impl/TraitDictionaryExtensions.cs @@ -0,0 +1,63 @@ +namespace DependencyModules.xUnit.Impl; + +/// +/// Conversions between the two shapes xUnit uses for a trait dictionary. +/// +/// +/// These replace ToReadOnly and ToReadWrite from Xunit.Internal. That +/// namespace is xUnit's own internal surface, not part of the extensibility contract, and carries +/// no compatibility guarantee across versions. Unlike the signature drift in the extensibility +/// interfaces — which the compiler catches — a behavioural change to an internal helper would +/// arrive as a wrong trait dictionary at run time. A dozen lines of dictionary copying is a +/// cheaper thing to own outright than that risk. +/// +/// Deliberately not named ToReadOnly/ToReadWrite: if Xunit.Internal is ever +/// pulled back into scope, matching names would produce ambiguity rather than a clean choice. +/// ToReadOnlyTraits also avoids colliding with the AsReadOnly that +/// supplies for dictionaries. +/// +internal static class TraitDictionaryExtensions { + + /// + /// Widens the mutable form xUnit stores traits in to the read-only form its constructors take. + /// + /// + /// The key comparer is carried over, which is what the Xunit.Internal helper this + /// replaces did — verified against 1.0.0 by running both over the same input and comparing. + /// It is not observable further downstream, because copies + /// traits into a fresh dictionary under the default comparer whatever it is handed. Carried + /// over regardless, so this is a faithful swap rather than one that merely happens to look + /// equivalent under today's xUnit. + /// + public static IReadOnlyDictionary> ToReadOnlyTraits( + this Dictionary> traits) => + traits.ToDictionary( + pair => pair.Key, + pair => (IReadOnlyCollection)pair.Value, + traits.Comparer); + + /// + /// Copies the read-only form into the mutable one, under the supplied key comparer. + /// + /// + /// A copy rather than a view: the result is handed to a test case that owns its own traits and + /// may add to them, and writing through to the method's metadata would leak across test cases. + /// + /// The comparer governs this intermediate dictionary only — + /// rebuilds what it is given under its own ordinal-ignore-case comparer, so passing a different + /// one here changes nothing observable. It is supplied to keep the intermediate consistent with + /// where the traits are headed, and to match the call this replaced. + /// + public static Dictionary> ToWritableTraits( + this IReadOnlyDictionary> traits, + IEqualityComparer comparer) { + + var result = new Dictionary>(comparer); + + foreach (var pair in traits) { + result[pair.Key] = new HashSet(pair.Value); + } + + return result; + } +} diff --git a/src/DependencyModules.xUnit/Impl/XunitTestMethodContext.cs b/src/DependencyModules.xUnit/Impl/XunitTestMethodContext.cs new file mode 100644 index 0000000..15168b4 --- /dev/null +++ b/src/DependencyModules.xUnit/Impl/XunitTestMethodContext.cs @@ -0,0 +1,48 @@ +using System.Reflection; +using DependencyModules.Testing.Attributes.Interfaces; +using Xunit.v3; + +namespace DependencyModules.xUnit.Impl; + +/// +/// The xUnit view of a test method, for hooks that need more than the neutral contract carries. +/// +/// +/// The hooks in DependencyModules.Testing are handed an so a +/// mocking package can implement them without referencing a test framework at all. An attribute that +/// is already xUnit-specific gives up nothing for that: the context it receives implements this, so +/// if (testMethod is IXunitTestMethodContext xunit) reaches the full model — unique ID, merged +/// traits, generic resolution, the test class and its collection. +/// +public interface IXunitTestMethodContext : ITestMethodContext { + + /// + /// xUnit's own model of the test method. + /// + IXunitTestMethod XunitTestMethod { + get; + } +} + +/// +/// Adapts to the neutral contract. +/// +/// +/// The attributes are passed in rather than walked here because the test case has already collected +/// and ordered them to decide which modules to load, and that walk reaches the assembly and the +/// declaring type as well as the method. +/// +internal sealed class XunitTestMethodContext( + IXunitTestMethod testMethod, + IReadOnlyList attributes) : IXunitTestMethodContext { + + public IXunitTestMethod XunitTestMethod { + get; + } = testMethod; + + public MethodInfo Method => XunitTestMethod.Method; + + public IReadOnlyList Attributes { + get; + } = attributes; +} diff --git a/tests/DependencyModules.Tests/DependencyModules.Tests.csproj b/tests/DependencyModules.Tests/DependencyModules.Tests.csproj index 8b7edc0..c3d529e 100644 --- a/tests/DependencyModules.Tests/DependencyModules.Tests.csproj +++ b/tests/DependencyModules.Tests/DependencyModules.Tests.csproj @@ -1,7 +1,7 @@  - net8.0 + $(LibraryTargetFrameworks) enable enable False @@ -37,13 +37,24 @@ - - + all runtime; build; native; contentfiles; analyzers; buildtransitive - + + + + + + + + + +