Skip to content

ci: share sharded test jobs between PR and preview workflows - #583

Merged
alexeyzimarev merged 2 commits into
devfrom
ci/shard-preview-tests
Aug 21, 2026
Merged

ci: share sharded test jobs between PR and preview workflows#583
alexeyzimarev merged 2 commits into
devfrom
ci/shard-preview-tests

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Contributor

Summary

The preview publish workflow (preview.yml) still ran the whole solution's tests in a single dotnet test on 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 to dev failed 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.

  • Extract the unit-tests and integration-tests jobs from pull-request.yml into a reusable workflow tests.yml (workflow_call), so PR and preview runs use identical sharding — one runner per container family.
  • preview.yml calls the shared tests and gates the nuget publish job on them via needs: tests.
  • Preview test results are now published by the existing Test Results workflow (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.
  • No behavior change for PR CI beyond job names gaining a Tests / prefix (no required status checks reference the old names).

Test plan

  • actionlint passes on all four workflow files.
  • After merge, the push to dev runs 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

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>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

CI: reuse sharded test matrix for both PR and preview workflows

⚙️ Configuration changes ✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Extract sharded unit/integration test jobs into reusable workflow shared by PR and preview.
• Gate preview NuGet publish on sharded test completion to reduce flaky container startups.
• Publish preview test results via workflow_run by uploading the GitHub event file artifact.
Diagram

graph TD
  PR["pull-request.yml (PR CI)"] --> T["tests.yml (reusable tests)"] --> TR["test-results.yml (publisher)"]
  PV["preview.yml (dev preview)"] --> T --> N["nuget job (publish)"]
  PR --> E["Event File artifact"] --> TR
  PV --> E
  TR --> C["Commit checks / annotations"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep duplicated test jobs in PR and preview workflows
  • ➕ No reusable-workflow indirection; each workflow remains self-contained
  • ➕ Callers can diverge independently if preview needs special handling
  • ➖ High drift risk (exact issue that caused preview to keep single-runner tests)
  • ➖ More maintenance when adding/removing suites or TFMs
2. Use a composite action for test execution steps
  • ➕ Shares step logic while keeping job definitions in each workflow
  • ➕ Easier to parameterize per-caller environment variables
  • ➖ Doesn't naturally share job-level strategy/matrix sharding
  • ➖ Still duplicates matrix/job structure across callers
3. Make preview call PR workflow via workflow_call and add publish job
  • ➕ Single 'source of truth' pipeline file
  • ➕ Avoids duplicating event-file plumbing
  • ➖ Blurs PR vs push concerns and permissions; can become harder to reason about
  • ➖ More churn if PR workflow has PR-specific triggers or concurrency constraints

Recommendation: The reusable workflow_call approach in tests.yml is the best fit because it centralizes the sharded matrix structure (the core reliability requirement) and keeps PR/preview responsibilities separate. A composite action would share steps but not the matrix/job sharding, and duplicated jobs would likely drift again.

Files changed (4) +157 / -143

Refactor (1) +5 / -121
pull-request.ymlRefactor PR tests into reusable workflow call +5/-121

Refactor PR tests into reusable workflow call

• Removes inlined unit-tests and integration-tests job definitions and replaces them with a single 'Tests' reusable workflow invocation. Adds clarifying comments to the Event File job about consumption by test-results.yml for correct PR context.

.github/workflows/pull-request.yml

Other (3) +152 / -22
preview.ymlCall shared sharded tests and gate NuGet publish +20/-21

Call shared sharded tests and gate NuGet publish

• Adds an Event File artifact upload job so the Test Results workflow can publish results for preview runs. Replaces the inline single-runner dotnet test + EnricoMi publish step with a call to the shared tests.yml workflow, and gates the nuget job via needs: tests (dropping Python setup).

.github/workflows/preview.yml

test-results.ymlPublish test results for both PR and preview workflows +1/-1

Publish test results for both PR and preview workflows

• Expands the workflow_run trigger to also react to completed runs of the 'Publish Preview NuGet' workflow, enabling the shared results publishing path for preview pushes.

.github/workflows/test-results.yml

tests.ymlIntroduce reusable sharded test workflow +131/-0

Introduce reusable sharded test workflow

• Adds a new workflow_call reusable workflow defining the unit test multi-TFM matrix and the integration test provider-sharded matrix (one container family per runner). Uploads test result artifacts from each shard to be aggregated/published by the existing test-results workflow.

.github/workflows/tests.yml

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add the blob-storage suite to the shared matrix

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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Publish ignores event file 🐞 Bug ☼ Reliability
Description
In preview.yml, the nuget publish job depends only on tests, so it can publish packages even
if the event_file job fails. When that happens, test-results.yml will fail to publish results
because it unconditionally expects artifacts/Event File/event.json from the completed run.
Code

.github/workflows/preview.yml[R29-32]

  nuget:
    runs-on: ubuntu-latest
-#    runs-on: [ self-hosted, type-cpx52, setup-docker, volume-cache-50GB ]
+    needs: tests
    permissions:
Evidence
The preview workflow publishes only after tests, but the Test Results workflow requires the `Event
File artifact; if event_file fails, results publishing breaks while nuget` may still run.

.github/workflows/preview.yml[11-35]
.github/workflows/test-results.yml[21-35]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`nuget` can run even if the `event_file` artifact upload failed, which causes `test-results.yml` to fail later (missing `event.json`) while preview packages are already published.

### Issue Context
- `test-results.yml` always reads `artifacts/Event File/event.json`.
- `preview.yml` currently does not require `event_file` to succeed before publishing.

### Fix Focus Areas
- .github/workflows/preview.yml[29-32]
- .github/workflows/test-results.yml[21-35]

### Suggested change
Update `preview.yml` so `nuget` requires both `tests` and `event_file`:
```yaml
nuget:
 needs: [tests, event_file]
```
This ensures preview publishing only occurs for runs where the downstream Test Results workflow can publish results.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/workflows/preview.yml
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

Test Results

   45 files  + 22     45 suites  +22   13m 33s ⏱️ - 3m 37s
  583 tests +  4    583 ✅ +  9  0 💤 ±0  0 ❌  - 5 
1 152 runs  +562  1 152 ✅ +567  0 💤 ±0  0 ❌  - 5 

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.
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Passes(08/21/2026 11:58:16 +00:00)
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Passes(08/21/2026 11:58:16)
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Passes(f9d95c9d-b08a-4a92-8184-8af0b559b1b4)
Eventuous.Tests.Subscriptions.SequenceTests ‑ ShouldReturnFirstBefore(CommitPosition { Position: 0, Sequence: 1, Timestamp: 2026-08-21T11:59:35.7192750+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T11:59:35.7192750+00:00 }, CommitPosition { Position: 0, Sequence: 4, Timestamp: 2026-08-21T11:59:35.7192750+00:00 }, CommitPosition { Position: 0, Sequence: 6, Timestamp: 2026-08-21T11:59:35.7192750+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T11:59:35.7192750+00:00 })
Eventuous.Tests.Subscriptions.SequenceTests ‑ ShouldReturnFirstBefore(CommitPosition { Position: 0, Sequence: 1, Timestamp: 2026-08-21T11:59:35.7192750+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T11:59:35.7192750+00:00 }, CommitPosition { Position: 0, Sequence: 6, Timestamp: 2026-08-21T11:59:35.7192750+00:00 }, CommitPosition { Position: 0, Sequence: 8, Timestamp: 2026-08-21T11:59:35.7192750+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T11:59:35.7192750+00:00 })
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Passes(08/21/2026 12:29:15 +00:00)
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Passes(08/21/2026 12:29:15)
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Passes(39d8372a-f6a9-4d14-8632-7de185b692fb)
Eventuous.Tests.Subscriptions.SequenceTests ‑ ShouldReturnFirstBefore(CommitPosition { Position: 0, Sequence: 1, Timestamp: 2026-08-21T12:25:07.3451584+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:07.3451584+00:00 }, CommitPosition { Position: 0, Sequence: 4, Timestamp: 2026-08-21T12:25:07.3451584+00:00 }, CommitPosition { Position: 0, Sequence: 6, Timestamp: 2026-08-21T12:25:07.3451584+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:07.3451584+00:00 })
Eventuous.Tests.Subscriptions.SequenceTests ‑ ShouldReturnFirstBefore(CommitPosition { Position: 0, Sequence: 1, Timestamp: 2026-08-21T12:25:07.3451584+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:07.3451584+00:00 }, CommitPosition { Position: 0, Sequence: 6, Timestamp: 2026-08-21T12:25:07.3451584+00:00 }, CommitPosition { Position: 0, Sequence: 8, Timestamp: 2026-08-21T12:25:07.3451584+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:07.3451584+00:00 })
Eventuous.Tests.Subscriptions.SequenceTests ‑ ShouldReturnFirstBefore(CommitPosition { Position: 0, Sequence: 1, Timestamp: 2026-08-21T12:25:08.8175915+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:08.8175915+00:00 }, CommitPosition { Position: 0, Sequence: 4, Timestamp: 2026-08-21T12:25:08.8175915+00:00 }, CommitPosition { Position: 0, Sequence: 6, Timestamp: 2026-08-21T12:25:08.8175915+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:08.8175915+00:00 })
Eventuous.Tests.Subscriptions.SequenceTests ‑ ShouldReturnFirstBefore(CommitPosition { Position: 0, Sequence: 1, Timestamp: 2026-08-21T12:25:08.8175915+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:08.8175915+00:00 }, CommitPosition { Position: 0, Sequence: 6, Timestamp: 2026-08-21T12:25:08.8175915+00:00 }, CommitPosition { Position: 0, Sequence: 8, Timestamp: 2026-08-21T12:25:08.8175915+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:08.8175915+00:00 })
Eventuous.Tests.Subscriptions.SequenceTests ‑ ShouldReturnFirstBefore(CommitPosition { Position: 0, Sequence: 1, Timestamp: 2026-08-21T12:25:14.4929399+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:14.4929399+00:00 }, CommitPosition { Position: 0, Sequence: 4, Timestamp: 2026-08-21T12:25:14.4929399+00:00 }, CommitPosition { Position: 0, Sequence: 6, Timestamp: 2026-08-21T12:25:14.4929399+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:14.4929399+00:00 })
Eventuous.Tests.Subscriptions.SequenceTests ‑ ShouldReturnFirstBefore(CommitPosition { Position: 0, Sequence: 1, Timestamp: 2026-08-21T12:25:14.4929399+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:14.4929399+00:00 }, CommitPosition { Position: 0, Sequence: 6, Timestamp: 2026-08-21T12:25:14.4929399+00:00 }, CommitPosition { Position: 0, Sequence: 8, Timestamp: 2026-08-21T12:25:14.4929399+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2026-08-21T12:25:14.4929399+00:00 })

♻️ 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>
@alexeyzimarev
alexeyzimarev merged commit 17f4951 into dev Aug 21, 2026
17 checks passed
@alexeyzimarev
alexeyzimarev deleted the ci/shard-preview-tests branch August 21, 2026 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant