Uh oh!
There was an error while loading. Please reload this page.
ci: share sharded test jobs between PR and preview workflows - #583
Merged
Conversation
The preview publish workflow still ran the whole solution's tests in a single dotnet test on one runner — the setup PR CI was moved away from because stacking all provider containers on one box made KurrentDB container startup (and the tests under load) flaky. Recent dev pushes failed exactly that way while the sharded PR runs stayed green. Extract the unit and integration test jobs into a reusable workflow (tests.yml) called from both pull-request.yml and preview.yml, and gate the NuGet publish job on the test matrix via needs. Test results from preview runs are now published by the Test Results workflow, replacing the inline EnricoMi step (and the Python setup it needed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
PR Summary by QodoCI: reuse sharded test matrix for both PR and preview workflows
AI Description
Diagram
High-Level Assessment
Files changed (4) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:2d3cebf17c
ℹ️ 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".
Comment on lines
+101
to
+102
| - name: azure-servicebus | ||
| project: src/Azure/test/Eventuous.Tests.Azure.ServiceBus/Eventuous.Tests.Azure.ServiceBus.csproj |
There was a problem hiding this comment.
On pushes to dev, preview.yml now replaces the solution-wide dotnet test with this curated matrix, but the matrix omits src/Azure/test/Eventuous.Tests.Azure.Storage.Blobs/Eventuous.Tests.Azure.Storage.Blobs.csproj. That project is included in Eventuous.slnx, contains 20 Azurite-backed tests, and was therefore covered by the previous preview command; after this change, preview packages can be published without exercising the blob-storage integration at all. Add it as an integration suite alongside the other Azure provider.
Useful? React with 👍 / 👎.
ContributorAuthor
There was a problem hiding this comment.
Fixed in 90c6cd2 — added an azure-blobs suite entry for Eventuous.Tests.Azure.Storage.Blobs. Checked the rest of the solution for the same gap: the other test-named projects not in the matrix (Eventuous.Tests.Persistence.Base, Eventuous.Tests.Subscriptions.Base, Eventuous.Tests.OpenTelemetry) are IsTestProject=false base libraries whose concrete tests run inside the provider suites, so the blob storage project was the only real omission.
Contributor
Code Review by Qodo
1. Publish ignores event file |
Uh oh!
There was an error while loading. Please reload this page.
Test Results 45 files + 22 45 suites +22 13m 33s ⏱️ - 3m 37s Results for commit 90c6cd2. ± Comparison against base commit 989e12d. This pull request removes 5 and adds 9 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Eventuous.Tests.Azure.Storage.Blobs was only covered by the old solution-wide preview run; the sharded matrix omitted it. The other uncovered test-named projects (Persistence.Base, Subscriptions.Base, OpenTelemetry) are IsTestProject=false base libraries whose concrete tests run inside the provider suites, so they need no matrix entries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Summary
The preview publish workflow (
preview.yml) still ran the whole solution's tests in a singledotnet teston one runner — the exact setup PR CI was sharded away from, because stacking KurrentDB + Postgres + SQL Server + Mongo + Kafka + RabbitMQ containers on one box makes container startup (and tests under load) flaky. The last three pushes todevfailed exactly that way in the KurrentDB suite (Testcontainers health-check aborts at fixture startup, plus one load-induced soft-delete read race), while the sharded PR runs for the same commits were green.unit-testsandintegration-testsjobs frompull-request.ymlinto a reusable workflowtests.yml(workflow_call), so PR and preview runs use identical sharding — one runner per container family.preview.ymlcalls the shared tests and gates thenugetpublish job on them vianeeds: tests.workflow_run), replacing the inline EnricoMi step — the preview run uploads the event file artifact like PR runs do. This also drops the Python 3.8 setup that only the composite EnricoMi variant needed.Tests /prefix (no required status checks reference the old names).Test plan
actionlintpasses on all four workflow files.devruns the sharded matrix and only publishes the preview packages when all shards pass; the Test Results workflow should attach the combined results to the commit.🤖 Generated with Claude Code