Skip to content

Adopt Aspire workflows and parallelize integration tests#2205

Open
ejsmith wants to merge 36 commits intomainfrom
parallel-integration-test-slices
Open

Adopt Aspire workflows and parallelize integration tests#2205
ejsmith wants to merge 36 commits intomainfrom
parallel-integration-test-slices

Conversation

@ejsmith
Copy link
Copy Markdown
Member

@ejsmith ejsmith commented Apr 17, 2026

Summary

Adopt Aspire-first local and CI workflows, parallelize backend integration tests, and add a sample event generator for new accounts.

What Changed

Test infrastructure

  • share Aspire-backed integration test infrastructure across the test process instead of restarting it per class
  • allocate one AppScope slice per integration test fixture using test, test-1, and so on (the first fixture keeps the plain test scope so single-test runs stay easy to inspect)
  • reset Elasticsearch, queues, storage, and cache within each fixture scope rather than behind a global lock
  • make TestServer readiness tracking safe for multiple concurrent hosts without retaining disposed servers
  • harden baseline and fixture-file tests for parallel execution and line-ending differences

Aspire and local dev

  • update the AppHost, devcontainer, tasks, and docs to use aspire run as the default local startup path
  • wire the legacy Angular and new Svelte UIs into Aspire with developer-cert support and a shared API_HTTPS/API_HTTP proxy convention
  • replace ASPNETCORE_URLS env wiring in connect.js / vite.config.ts with the values Aspire injects

CI/CD workflow

  • consolidate gating logic into version job outputs (should_publish, is_prod_deploy, is_dev_deploy) so publish/deploy jobs are skipped cleanly on forks and PRs
  • keep the all-in-one exceptionless Docker image tag-only; standard PRs build api, job, and app
  • simplify dev environment retention from hours to days and run scheduled stop checks at 5am America/Chicago

Frontend

  • consolidate ClientApp dependencies into devDependencies (SvelteKit static SPA — nothing is consumed at runtime)
  • simplify vite.config.ts and fix dev:api to use API_HTTPS instead of the no-op ASPNETCORE_URLS
  • minor footer/layout polish

Sample data

  • new admin endpoint plus GenerateSampleEventsWorkItem / RandomEventGenerator to seed realistic events for fresh accounts

Docs

  • trim and correct README, AGENTS, and repo skill guidance to match the current Angular/Svelte split and Aspire startup flow

Verification

  • dotnet test
  • dotnet test -- --filter-class Exceptionless.Tests.Controllers.OpenApiControllerTests
  • dotnet test -- --filter-class Exceptionless.Tests.Controllers.AuthControllerTests
  • verified aspire run from the repo root starts the app

Notes

  • the legacy Angular app in src/Exceptionless.Web/ClientApp.angular still powers the main site while the Svelte 5 app in src/Exceptionless.Web/ClientApp remains under development

Copilot AI review requested due to automatic review settings April 17, 2026 22:11
Comment thread tests/Exceptionless.Tests/AppWebHostFactory.cs Fixed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables bounded class-level parallelism for the backend integration test suite by scoping test data per fixture (AppScope) while sharing the Aspire-managed infrastructure across the overall test process.

Changes:

  • Enable xUnit parallel execution with a fixed MaxParallelThreads value.
  • Introduce per-fixture AppScope slices (test, test-1, …) while sharing a single Aspire-backed Elasticsearch instance.
  • Update test helpers/baselines to behave correctly under parallel execution (server readiness tracking, file sharing, OpenAPI baseline comparison).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/Exceptionless.Tests/Utility/StackData.cs Allow reading stack JSON files without exclusive locks (parallel-safe file access).
tests/Exceptionless.Tests/Properties/PropertyInfo.cs Enable bounded xUnit class-level parallelism.
tests/Exceptionless.Tests/IntegrationTestsBase.cs Remove global reset lock; reset data per fixture scope and clear additional queues.
tests/Exceptionless.Tests/Extensions/TestServerExtensions.cs Make readiness wait tracking per-TestServer instead of global.
tests/Exceptionless.Tests/Controllers/OpenApiControllerTests.cs Compare OpenAPI baseline structurally and attempt to normalize line endings.
tests/Exceptionless.Tests/AppWebHostFactory.cs Share Aspire infrastructure across the run and allocate/reuse AppScope slices per fixture.
AGENTS.md Document new parallel integration test behavior and AppScope slicing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/Exceptionless.Tests/AppWebHostFactory.cs
Comment thread tests/Exceptionless.Tests/Controllers/OpenApiControllerTests.cs Outdated
Comment thread tests/Exceptionless.Tests/IntegrationTestsBase.cs
Comment thread tests/Exceptionless.Tests/Extensions/TestServerExtensions.cs Outdated
Comment thread tests/Exceptionless.Tests/AppWebHostFactory.cs Fixed
Comment thread tests/Exceptionless.Tests/AppWebHostFactory.cs Fixed
Comment thread tests/Exceptionless.Tests/AppWebHostFactory.cs Fixed
@ejsmith ejsmith requested a review from niemyjski April 18, 2026 22:09
@ejsmith ejsmith changed the title Enable parallel integration test slices Adopt Aspire workflows and parallelize integration tests Apr 18, 2026
@ejsmith
Copy link
Copy Markdown
Member Author

ejsmith commented Apr 18, 2026

Follow-up on the review feedback:

Addressed in 9c7c6cb2b:

  • tests/Exceptionless.Tests/Controllers/OpenApiControllerTests.cs: normalized both physical line endings and escaped \\r\\n sequences so the OpenAPI baseline comparison is stable across formatting differences.
  • tests/Exceptionless.Tests/Extensions/TestServerExtensions.cs: switched readiness tracking to ConditionalWeakTable<TestServer, object> so disposed servers are not retained.

Verification:

  • dotnet test --project ".\\tests\\Exceptionless.Tests\\Exceptionless.Tests.csproj" --configuration Release -- --filter-class Exceptionless.Tests.Controllers.OpenApiControllerTests
  • dotnet test --project ".\\tests\\Exceptionless.Tests\\Exceptionless.Tests.csproj" --no-restore --no-build --configuration Release -- --filter-class Exceptionless.Tests.Controllers.AuthControllerTests

The matching OpenAPI and TestServer review threads are resolved.

Still not addressed:

  • the shared Aspire application teardown comment in AppWebHostFactory
  • the global Log.DefaultLogLevel mutation comment in IntegrationTestsBase
  • the remaining static-analysis suggestions in AppWebHostFactory around static state allocation and empty catch blocks

Those are still open on purpose rather than being silently ignored.

Comment on lines +29 to +33
.ReplaceLineEndings("\n")
.Replace("\\r\\n", "\\n");
actualJson = actualJson
.ReplaceLineEndings("\n")
.Replace("\\r\\n", "\\n");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this really needed? should we be using Environment.NewLine so it's less magic strings?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the ReplaceLineEndings calls and added tests/Exceptionless.Tests/Controllers/Data/openapi.json text eol=lf to .gitattributes so the baseline is always LF on disk. Kept the \r\n\n replace because those are escape sequences embedded inside JSON string values (XML doc comments processed on Windows get serialized as "...\r\n..."), not on-disk line endings — Environment.NewLine would be CRLF on Windows runners and break the comparison there.

{
var startupContext = server.Services.GetService<StartupActionsContext>();
var maxWaitTime = !_alreadyWaited ? TimeSpan.FromSeconds(30) : TimeSpan.FromSeconds(2);
var maxWaitTime = !s_waitedServers.TryGetValue(server, out _) ? TimeSpan.FromSeconds(30) : TimeSpan.FromSeconds(2);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really wish we got rid of the s_

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — dropped the ConditionalWeakTable cache and both static fields entirely. The cache only narrowed the failure timeout (30s → 2s) on warm servers; on the success path the first /ready call returns immediately anyway, so the optimization wasn't pulling its weight. Now there's just a single 30s timeout (already covers cold starts and slow CI).

public class AppWebHostFactory : WebApplicationFactory<Startup>, IAsyncLifetime
{
private DistributedApplication? _app;
private static readonly Uri s_elasticsearchUri = new("http://127.0.0.1:9200");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why hard code this, can we also get rid of s_

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the hardcoded URL and the s_ field. The Elasticsearch URL is now resolved from the running Aspire app via elasticsearch.Resource.GetConnectionStringAsync() after app.StartAsync(), then passed into WaitForElasticsearchAsync as a parameter. Single source of truth — the port (9200) is declared once on AddElasticsearch(...) and Aspire owns the rest.

public class AppWebHostFactory : WebApplicationFactory<Startup>, IAsyncLifetime
{
private DistributedApplication? _app;
private static readonly Uri s_elasticsearchUri = new("http://127.0.0.1:9200");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also shouldn't this come from configuration?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The connection string is now derived from the Aspire ElasticsearchResource itself (see reply above), so it's no longer hardcoded — the test AppHost is the configuration. I'd rather not pull it from appsettings here because then we'd have two places declaring the test ES endpoint that could drift; AddElasticsearch("Elasticsearch", port: 9200) is the single declaration and GetConnectionStringAsync reads it back.

Comment thread Dockerfile
Comment on lines +21 to +26
FROM build AS build-node
RUN apt-get update -yq \
&& apt-get install -yq curl ca-certificates \
&& curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \
&& apt-get install -yq nodejs \
&& rm -rf /var/lib/apt/lists/*
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never understood why we didn't do this first, so this layer could be cached more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, agreed — the reorder lets Docker cache the dotnet restore layer across source-only changes, which should noticeably speed up CI builds when only .cs files change.

public AppWebHostFactory()
{
if (!s_pool.TryDequeue(out var instanceId))
instanceId = Interlocked.Increment(ref s_counter);
Comment on lines +74 to +76
catch (HttpRequestException)
{
}
Comment on lines +77 to +79
catch (TaskCanceledException)
{
}
@github-actions
Copy link
Copy Markdown

Code Coverage

Package Line Rate Branch Rate Complexity Health
Exceptionless.Core 65% 59% 7704
Exceptionless.Insulation 25% 23% 203
Exceptionless.Web 57% 43% 3651
Exceptionless.AppHost 19% 9% 84
Summary 60% (11924 / 19794) 53% (5956 / 11192) 11642

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants