diff --git a/.config/purview-build.json b/.config/purview-build.json index 6062685..0181f8f 100644 --- a/.config/purview-build.json +++ b/.config/purview-build.json @@ -3,7 +3,15 @@ "Solution": "src/ResourceKit.slnx", "TestRoot": "src/tests", "TestPatterns": "*Tests.csproj", - "TestFilter": "/*/*/*/*" + // The full suite, not the unit tier: both PR validation and the release run every discovered + // test project, including the AppHost integration tests. PipelineTests fails if this is + // narrowed, because a unit-only filter looks green while the expensive suite never runs. + "TestFilter": "/*/*/*/*", + // The integration suite starts Docker containers and several AppHosts; running it beside the + // Roslyn suites on a shared CI runner widens Aspire's probe-then-bind port race and starves + // the containers. Run one test project at a time; each project still runs its TFM apps in + // parallel, which the integration project handles with its own port ranges. + "MaxTestParallelism": 1 }, "PackValidation": { // Explicit because Purview.Build 0.5.3 defaults RequiredCompilerFlags to empty. Earlier diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 4541be0..b5605c0 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -12,4 +12,8 @@ jobs: build: name: Build and test uses: purview-dev/build/.github/workflows/purview-build.yml@main + with: + # The full test filter is owned by .config/purview-build.json; this only states that the + # pipeline must run tests rather than rely on a shared-workflow default. + run-tests: true secrets: inherit diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 01283a5..edc110a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -30,6 +30,9 @@ jobs: with: release-mode: NuGet trusted-publishing: true + # The full test filter is owned by .config/purview-build.json; this only states that the + # pipeline must run tests rather than rely on the composite action's default. + run-tests: true # The nuget.org profile name, not the sign-in email. nuget-user: kieronlanning # A composite action has no `secrets` context, so the read token is passed explicitly. diff --git a/AGENTS.md b/AGENTS.md index d33d1d0..d2c09b9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,6 +63,9 @@ Work is **not complete** until relevant tests pass. - Use **TUnit** and **TUnit.Mocks**. - Do **not** introduce NUnit, xUnit, or MSTest patterns/packages. +- Both PR validation and the release run the **full** suite on both target frameworks. Do not add a + `Build:TestFilter` category filter or a workflow `test-filter` to skip the integration tests; + `Repository.UnitTests/PipelineTests.cs` fails if the filter is narrowed. ### Test structure and style @@ -95,7 +98,8 @@ If any method invoked in the test body supports a `CancellationToken`, pass a to Before considering a change done: 1. Relevant tests are updated/added using the conventions above. -2. Test runs pass for the impacted scope. +2. Test runs pass for the impacted scope. Both PR validation and the release run every test project, + including the AppHost integration tests, so `just test-unit` alone is not evidence a change passes CI. 3. New/updated docs stay consistent with this file and the authored skills in `skills/`. 4. A change to a project reference is reflected in `.config/architecture-policy.json` — the policy is enforced by `src/tests/Repository.UnitTests/ArchitectureTests.cs`, and widening `mayDependOn` is a diff --git a/CHANGELOG.md b/CHANGELOG.md index 559ae66..cebbabc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,15 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed +- **PR validation and the release run the full test suite.** `.config/purview-build.json` deliberately + uses the `/*/*/*/*` filter, so both pipelines execute every test project — including + `ResourceKit.IntegrationTests` — on both target frameworks, one project at a time so the Docker-heavy + AppHost suite does not compete with the Roslyn suites. `PipelineTests` fails if a workflow narrows the + filter or disables tests. The integration suite also starts its AppHost fixtures serially and gives + each target-framework test app a disjoint Aspire target-port range, so two AppHosts can no longer be + handed the same target port and fail with `address already in use` — the release-pipeline failure + that a PR on the same commit did not reproduce. + - **The repository builds without the Aspire CLI installed.** `Example.AppHost` and `Example.ManualAppHost` set `AspireUseCliBundle=true`, which makes a *build* require a resolvable CLI bundle: with none present, `Aspire.Hosting.AppHost.targets` runs `dnx aspire.cli@ setup` once per diff --git a/Directory.Packages.props b/Directory.Packages.props index ec11b68..3891229 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -22,6 +22,11 @@ + + diff --git a/docs/wiki/Contributing.md b/docs/wiki/Contributing.md index 0694fd9..94164ea 100644 --- a/docs/wiki/Contributing.md +++ b/docs/wiki/Contributing.md @@ -49,9 +49,10 @@ opt-in out of the project files for exactly that reason. - Test names mirror production structure: `{ClassUnderTest}Tests` and `{SubjectOrMethodUnderTest}_{Scenario}_{Expectation}`. - When a method under test accepts a `CancellationToken`, pass one and make it the final argument. -- A unit test runs in CI because of the **project it lives in**, not an attribute: `Purview.BuildSdk` - derives the tier from the project name (`*.UnitTests` → `Category=Unit`), and the pipeline filters on - that. Splitting a suite into a second project is how a test leaves the fast tier. +- The project name sets the tier: `Purview.BuildSdk` derives `Category=Unit` for `*.UnitTests`. Both + PR validation and the release run the **full** suite, so the tier is a local convenience for the + fast loop (`just test-unit`), not a CI gate. A test added to an integration project runs in CI. + Do not narrow `Build:TestFilter` or pass `test-filter` from a workflow; `PipelineTests` fails if you do. Test projects: @@ -135,7 +136,9 @@ Releases are produced by the shared pipeline when a version bump is merged to `m Pull requests target `main` and are validated by `.github/workflows/pr.yml`, which delegates to the shared [Purview.Build](https://github.com/purview-dev/build) pipeline (restore, build, CSharpier lint, -unit tests, pack, package-content validation). See [Release Flow](Release-Flow.md). +the full test suite including the AppHost integration tests, pack, package-content validation). The +integration tests need a Docker-capable runner; GitHub's `ubuntu-latest` is one. See +[Release Flow](Release-Flow.md). Commits follow [Conventional Commits](https://www.conventionalcommits.org/), enforced by commitlint and Lefthook hooks. \ No newline at end of file diff --git a/docs/wiki/Release-Flow.md b/docs/wiki/Release-Flow.md index 56190ee..4a2fc70 100644 --- a/docs/wiki/Release-Flow.md +++ b/docs/wiki/Release-Flow.md @@ -16,14 +16,18 @@ workflow. It runs: 1. `dotnet restore` of `src/ResourceKit.slnx` 2. `dotnet build --no-restore --configuration Release` 3. CSharpier lint across the repository -4. Unit tests (discovered under `src/tests` matching `*Tests.csproj`, run with the - `/*/*/*/*[Category=Unit]` TUnit tree-node filter) +4. **The full test suite** — every project discovered under `src/tests` matching `*Tests.csproj`, + including the AppHost integration tests, with the `/*/*/*/*` TUnit tree-node filter 5. `dotnet pack` and package-content validation -Integration tests are never discovered in CI: `purview-build.json` sets `Build:TestPatterns` to -`*Tests.csproj` and `Build:TestFilter` to `/*/*/*/*[Category=Unit]`, so only unit-test projects (tagged -`[Category=Unit]` by the `Purview.BuildSdk`) are executed; integration tests (which require -Docker/Testcontainers) run locally via `just test`. +The release pipeline runs the same test step against the same `purview-build.json`, so a suite the PR +passes is the suite the release runs. `src/tests/Repository.UnitTests/PipelineTests.cs` fails if either +workflow narrows the filter or disables tests. + +The integration tests start the example AppHosts (and their Docker containers), so the test step runs +one test project at a time (`Build:MaxTestParallelism`) and the AppHost fixtures inside a project start +serially. The two target-framework test apps still run in parallel, but each is given its own Aspire +target-port range so Aspire cannot allocate the same port to both. The PR workflow does not tag, release, or publish packages. @@ -51,7 +55,8 @@ The shared workflow: 2. Skips the entire release when `v` already exists — the job finishes green and publishes nothing — so re-merging to `main`, or merging `main` into a `release` branch, releases exactly once. A policy violation instead fails the job, naming the `REL0nn` rule that rejected it. -3. Restores, builds, lints, runs unit tests, packs, and validates packages. +3. Restores, builds, lints, runs the full test suite (unit and AppHost integration), packs, and + validates packages. 4. Pushes every `.nupkg` to nuget.org (`--skip-duplicate`). 5. Creates the `v` GitHub release with notes generated by GitHub. Package artifacts are **not** attached unless the caller passes `upload-artifacts: true`, which this repository does @@ -131,7 +136,8 @@ federated credential before the shared pipeline runs; the shared workflow itself | `Build:Solution` | `src/ResourceKit.slnx` | Solution passed to restore/build/pack | | `Build:TestRoot` | `src/tests` | Test project discovery root | | `Build:TestPatterns` | `*Tests.csproj` | Test projects discovered for the test step | -| `Build:TestFilter` | `/*/*/*/*[Category=Unit]` | TUnit tree-node filter (unit-only) | +| `Build:TestFilter` | `/*/*/*/*` | TUnit tree-node filter — every test, unit and integration | +| `Build:MaxTestParallelism` | `1` | One test project at a time; the Aspire integration suite must not compete with the Roslyn suites for a CI runner | | `PackValidation:RequireSymbolPackage` | `true` | Every `.nupkg` needs a matching `.snupkg` | | `PackValidation:RequireSymbolFiles` | `true` | Every `.snupkg` must contain PDBs | | `PackValidation:RequiredContent` | Expected package contents | Asserts the package ships its expected output — the `lib/net10.0` **and** `lib/net11.0` runtime assemblies + XML docs, the generator **and** code-fix assemblies under `analyzers/dotnet/cs`, `buildTransitive/Purview.Aspire.ResourceKit.props`, `README.md`, and `purview-logo-light.png`. It also fails on **undeclared** package content, so a new asset has to be added here in the same change that introduces it | diff --git a/skills/purview-resourcekit-testing/SKILL.md b/skills/purview-resourcekit-testing/SKILL.md index 1d12696..a42c04f 100644 --- a/skills/purview-resourcekit-testing/SKILL.md +++ b/skills/purview-resourcekit-testing/SKILL.md @@ -155,18 +155,23 @@ to test lifecycle semantics without starting anything. ## Running them -Unit tier only — integration tests need Docker/Testcontainers: +The fast local loop — unit tests only; the integration tests need Docker/Testcontainers: ```bash just test "/*/*/*/*[Category=Unit]" ``` -Everything, locally: +Everything, locally, including the AppHost integration tests: ```bash just test ``` +CI runs **everything**, not the unit tier: PR validation and the release both run every test project in +this table. A change that only passes `just test-unit` is not a passing PR. The integration project's +AppHost fixtures start serially and each target-framework test app gets its own Aspire port range, so +several AppHosts cannot be handed the same target port; keep those guarantees if you add a fixture. + Filtering uses the TUnit **tree-node** filter (`--treenode-filter`), not `--filter`: this repository runs on Microsoft.Testing.Platform, where `dotnet test --filter` reports "Zero tests ran". diff --git a/src/tests/Repository.UnitTests/PipelineTests.cs b/src/tests/Repository.UnitTests/PipelineTests.cs new file mode 100644 index 0000000..2299e6a --- /dev/null +++ b/src/tests/Repository.UnitTests/PipelineTests.cs @@ -0,0 +1,112 @@ +using System.Text.Json; + +namespace Purview.Aspire.ResourceKit.Repository; + +/// +/// Holds both shared pipelines to running the whole suite: every discovered test project, on every +/// target framework, in PR validation and in the release. +/// +/// +/// The repository previously ran only the unit tier in CI. Turning that back on is a one-line edit to +/// purview-build.json or a test-filter argument in a workflow, and both are easy to add +/// while debugging a slow or failing suite. The integration tests are the suite most likely to be +/// dropped this way - they start Docker containers and real AppHosts - so the contract belongs in a +/// failing test rather than in a comment. +/// +public sealed class PipelineTests +{ + static readonly string RepositoryRoot = RepositoryPaths.Root; + + static readonly string[] WorkflowPaths = [".github/workflows/pr.yml", ".github/workflows/release.yml"]; + + [Test] + public async Task PurviewBuildConfig_GivenTestFilter_RunsEveryDiscoveredProject(CancellationToken cancellationToken) + { + // Arrange + cancellationToken.ThrowIfCancellationRequested(); + var configPath = Path.Combine(RepositoryRoot, ".config", "purview-build.json"); + using var document = JsonDocument.Parse( + await File.ReadAllTextAsync(configPath, cancellationToken), + new JsonDocumentOptions { CommentHandling = JsonCommentHandling.Skip, AllowTrailingCommas = true } + ); + var build = document.RootElement.GetProperty("Build"); + + // Act + var filter = build.GetProperty("TestFilter").GetString(); + var patterns = build.GetProperty("TestPatterns").GetString(); + var projects = build.TryGetProperty("TestProjects", out var projectsElement) + ? projectsElement.GetString() + : null; + var runTests = !build.TryGetProperty("RunTests", out var runTestsElement) || runTestsElement.GetBoolean(); + + // Assert + await Assert + .That(filter) + .IsEqualTo("/*/*/*/*") + .Because( + "PR validation and the release must run every test project, including the AppHost " + + "integration tests; a category filter such as '[Category=Unit]' silently removes them." + ); + await Assert + .That(patterns) + .IsEqualTo("*Tests.csproj") + .Because("the integration test projects match the same pattern as the unit suites"); + await Assert + .That(projects is null or "*") + .IsTrue() + .Because("an explicit project list would stop a newly added test project from running"); + await Assert + .That(runTests) + .IsTrue() + .Because("Build:RunTests=false would make the pipeline succeed without running anything"); + } + + [Test] + public async Task Workflows_GivenSharedPipelineCalls_DoNotNarrowOrDisableTests(CancellationToken cancellationToken) + { + // Arrange + cancellationToken.ThrowIfCancellationRequested(); + var workflows = WorkflowPaths + .Select(relative => Path.Combine(RepositoryRoot, relative.Replace('/', Path.DirectorySeparatorChar))) + .Select(path => (Path: path, Lines: File.ReadAllLines(path))) + .ToArray(); + + // Act + var narrowed = workflows + .SelectMany(workflow => + workflow + .Lines.Select(static line => line.Trim()) + .Where(static line => + line.Contains("[Category=Unit]", StringComparison.Ordinal) + || line.StartsWith("test-filter:", StringComparison.Ordinal) + || line.StartsWith("run-tests: false", StringComparison.OrdinalIgnoreCase) + ) + .Select(line => $"{Path.GetRelativePath(RepositoryRoot, workflow.Path)}: {line}") + ) + .ToArray(); + var withoutTestsEnabled = workflows + .Where(workflow => + !workflow.Lines.Any(static line => + string.Equals(line.Trim(), "run-tests: true", StringComparison.Ordinal) + ) + ) + .Select(workflow => Path.GetRelativePath(RepositoryRoot, workflow.Path)) + .ToArray(); + + // Assert + await Assert + .That(narrowed) + .IsEmpty() + .Because( + "the test filter belongs in .config/purview-build.json so both pipelines run the same " + + "full suite; a workflow-level filter or RunTests=false makes one pipeline weaker." + ); + await Assert + .That(withoutTestsEnabled) + .IsEmpty() + .Because( + "both pipelines should state that they run tests, rather than depending on a shared " + + "workflow or action default that could change." + ); + } +} diff --git a/src/tests/ResourceKit.IntegrationTests/TestProcessConfiguration.cs b/src/tests/ResourceKit.IntegrationTests/TestProcessConfiguration.cs new file mode 100644 index 0000000..583dc29 --- /dev/null +++ b/src/tests/ResourceKit.IntegrationTests/TestProcessConfiguration.cs @@ -0,0 +1,39 @@ +// Every test class in this project starts an AppHost, and an AppHost allocates each project resource's +// target port by probing it and then releasing it before the project binds it. Two AppHosts starting at +// the same time can therefore be handed the same port, and the second api process dies with "address +// already in use" - which is how the release pipeline failed while a PR on the same commit passed. +// +// Serializing the fixtures removes the race inside one test process. The net10.0 and net11.0 test apps +// are still started in parallel by 'dotnet test', so the hook below gives each target framework its own +// non-overlapping Aspire port range as well. +[assembly: NotInParallel] + +namespace Purview.Aspire.ResourceKit; + +/// +/// Isolates the Aspire port allocations of the two target-framework test apps this project produces. +/// +static class TestProcessConfiguration +{ + internal const string PortRangeVariable = "ASPIRE_PROXYLESS_ENDPOINT_PORT_RANGE"; + + [Before(TestSession)] + public static void ConfigureAspirePortRange() + { + var major = TargetFrameworkMajorVersion(); + var start = 10000 + ((Math.Max(major, 10) - 10) * 10000); + var end = Math.Min(start + 9999, 60000); + Environment.SetEnvironmentVariable(PortRangeVariable, $"{start}-{end}"); + } + + static int TargetFrameworkMajorVersion() + { + // AppContext.TargetFrameworkName is the framework the test app was compiled for, not the runtime + // it may have rolled forward onto, so net10.0 and net11.0 cannot collapse onto one range. + const string marker = "Version=v"; + var name = AppContext.TargetFrameworkName; + var index = name?.IndexOf(marker, StringComparison.Ordinal) ?? -1; + + return index >= 0 && int.TryParse(name.AsSpan(index + marker.Length, 2), out var major) ? major : 10; + } +}