ci: split integration tests into per-provider jobs to fix container pressure#555
Merged
Merged
Conversation
…ressure
Running the whole solution in one CI job started every provider's
Testcontainers at once (KurrentDB + Postgres + SQL Server + Mongo +
Kafka + RabbitMQ + ...), saturating the GitHub-hosted runner and making
container startup — and the integration tests — flaky. The old
self-hosted runners masked this with far more CPU/RAM.
- Replace the single whole-solution `dotnet test` with a {suite x
framework} matrix. Each integration provider runs on its own runner,
so only one container family starts per box. All container-less unit
tests share a `core` suite.
- Add the missing parallel limiter to the Postgres test project (it was
the only integration project without one, so it could start unbounded
concurrent containers).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
PR Summary by QodoCI: split integration tests into per-provider matrix jobs; cap Postgres parallelism
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f82291ac66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| set -euo pipefail | ||
| for proj in ${{ matrix.suite.projects }}; do | ||
| echo "::group::dotnet test $proj (net${{ matrix.dotnet-version }})" | ||
| dotnet test "$proj" -c "Debug CI" -f net${{ matrix.dotnet-version }} |
There was a problem hiding this comment.
Because this now tests only matrix.suite.projects, the PR workflow no longer builds projects that are in Eventuous.slnx but not referenced by any listed test project. I checked the listed projects' ProjectReference closure and it misses shipped projects such as src/Extensions/src/Eventuous.Subscriptions.Polly/Eventuous.Subscriptions.Polly.csproj, src/GooglePubSub/src/Eventuous.GooglePubSub.CloudRun/Eventuous.GooglePubSub.CloudRun.csproj, and src/Experimental/src/Eventuous.ElasticSearch/Eventuous.ElasticSearch.csproj, all of which the previous root dotnet test built as part of the solution. A PR that breaks one of those projects can now go green until a later publish workflow, so please add a separate solution build or include those projects.
Useful? React with 👍 / 👎.
Test Results 46 files + 24 46 suites +24 10m 39s ⏱️ - 2m 22s Results for commit 23e74bd. ± Comparison against base commit c55af3b. This pull request removes 5 and adds 9 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Code Review by Qodo
1. Redundant dotnet test runs
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+91
to
+96
| set -euo pipefail | ||
| for proj in ${{ matrix.suite.projects }}; do | ||
| echo "::group::dotnet test $proj (net${{ matrix.dotnet-version }})" | ||
| dotnet test "$proj" -c "Debug CI" -f net${{ matrix.dotnet-version }} | ||
| echo "::endgroup::" | ||
| done |
Contributor
There was a problem hiding this comment.
1. Redundant dotnet test runs 🐞 Bug ➹ Performance
The new CI logic runs dotnet test once per project, multiplying CLI/MSBuild/test-host startup and project-evaluation overhead, particularly for the core suite which expands to many projects. This can noticeably increase CI runtime and runner cost compared to a single invocation per suite/framework.
Agent Prompt
### Issue description
The PR changed the test step to loop over projects and invoke `dotnet test` for each one. This increases CI overhead (multiple `dotnet test` processes per job) and is most pronounced for the `core` suite.
### Issue Context
- The `core` suite contains many projects, so the loop causes many separate `dotnet test` invocations per framework.
### Fix Focus Areas
- .github/workflows/pull-request.yml[35-48]
- .github/workflows/pull-request.yml[89-96]
### Suggested fix approaches
Pick one (in order of robustness):
1) **Create suite-level solution filters** (e.g., `Eventuous.core.slnf`, `Eventuous.postgres.slnf`, etc.) and run a single `dotnet test <filter>.slnf -c "Debug CI" -f net${{ matrix.dotnet-version }}` per job.
2) If you keep the loop, **reduce repeated work** by doing one explicit restore/build first and then using `dotnet test --no-restore --no-build` inside the loop.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| run: | | ||
| dotnet test -c "Debug CI" -f net${{ matrix.dotnet-version }} | ||
| set -euo pipefail | ||
| for proj in ${{ matrix.suite.projects }}; do |
Contributor
There was a problem hiding this comment.
2. Brittle project list splitting 🐞 Bug ⚙ Maintainability
The loop relies on bash word-splitting of matrix.suite.projects, which is safe for the current whitespace-free paths but is brittle to maintain (e.g., accidental formatting changes or any future path/token containing whitespace or glob characters). This increases the chance of a future CI break where dotnet test receives malformed paths.
Agent Prompt
### Issue description
`for proj in ${{ matrix.suite.projects }}` relies on shell tokenization. While current paths work, the representation is fragile and easy to break with future edits.
### Issue Context
The `core` suite uses a folded YAML scalar to hold multiple project paths, which are then consumed via bash word-splitting.
### Fix Focus Areas
- .github/workflows/pull-request.yml[35-48]
- .github/workflows/pull-request.yml[91-93]
### Suggested fix
Represent `projects` as a structured list and iterate safely, e.g.:
- Store `projects` as a JSON array string in the matrix and iterate with `fromJSON(...)` + a step matrix, or
- Keep it line-delimited and iterate with a `while IFS= read -r proj; do ...; done` loop fed by a heredoc.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Splitting into 33 per-provider-per-framework jobs multiplied Docker Hub
image pulls (each integration job pulls its DB image + the Testcontainers
Ryuk reaper), tripping the pull rate limit ("toomanyrequests:
unauthenticated pull rate limit") even though docker login succeeds.
- Split into two jobs: `unit-tests` (container-less projects, all three
TFMs) and `integration-tests` (one provider per runner, net10.0 only).
DB adapters behave the same across runtimes, so multi-TFM integration
runs mostly re-pull the same images for little extra signal.
- Cap integration concurrency with max-parallel: 4 to stagger image pulls
under the rate limit.
- Add a concurrency group so superseded runs are cancelled.
Cuts CI from 33 container-pulling jobs to 10 (staggered) + 3 unit jobs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The login step reported "Login Succeeded!" but Testcontainers still pulled
images anonymously ("unauthenticated pull rate limit"), so it never bought
authenticated pulls. GitHub-hosted runners get a more generous anonymous
Docker Hub allowance, and authenticating against a rate-limited free account
can pull from a lower ceiling. Remove the login and rely on the runners'
anonymous allowance, kept under the limit by the reduced, throttled job set.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The KurrentDB (and other) integration tests fail randomly in CI. Root cause is container pressure: CI ran the whole solution in one
dotnet test, so a single GitHub-hosted runner started every provider's Testcontainers at once — KurrentDB + Postgres + SQL Server (azure-sql-edge, ~2 GB) + Mongo + Kafka + Zookeeper + RabbitMQ + Redis. On a 4-vCPU/16 GB runner that saturates CPU/RAM, container startup slows past the Testcontainers wait-strategy timeouts, and tests flake. The previous self-hostedcpx52runners masked this with far more headroom; the flakiness surfaced after moving to GitHub-hosted runners.Contributing factor: the Postgres test project had no
ParallelLimiter(KurrentDB and SQL Server do), so it could start unbounded concurrent Postgres containers.Fix
{ suite × framework }matrix (fail-fast: false). Each integration provider (kurrentdb, postgres, sqlserver, mongo, kafka, rabbitmq, redis, azure-servicebus, googlepubsub, signalr-integration) runs on its own runner, so only one container family starts per box. All container-less unit tests share a singlecoresuite.Limit => 4, mirroring the other providers).Net effect: no single runner stacks multiple DB families, and wall-clock improves because providers run in parallel across runners.
Notes
core:Eventuous.Tests.OpenTelemetry(only a commented-out test), and the shared basesEventuous.Tests.Subscriptions.Base/Eventuous.Tests.Persistence.Base— all produce zero runnable tests, so no coverage is lost.test-results.ymlneeds no change: it globs all downloaded artifacts, and each job uploads a uniquely-namedTest Results <suite> <framework>artifact.🤖 Generated with Claude Code