Skip to content

NCBC-4373: Add a nightly cross-platform unit test workflow - #154

Merged
davidkelly merged 12 commits into
masterfrom
dk/ncbc-4373
Jul 30, 2026
Merged

NCBC-4373: Add a nightly cross-platform unit test workflow#154
davidkelly merged 12 commits into
masterfrom
dk/ncbc-4373

Conversation

@davidkelly

Copy link
Copy Markdown
Contributor

Reproduces the Jenkins nightly: all three unit test projects on seven OS/arch legs, across net8.0, net10.0 and net48.

The PR gate runs only Couchbase.UnitTests, so the
DependencyInjection and OpenTelemetry unit tests (103 tests) have no CI coverage, and macos-latest being arm64 leaves x64 macOS uncovered. TFMs still come from SdkTestTargets rather than being duplicated in YAML; build-and-test.yml is untouched.

The temporary pull_request trigger must be removed before merge - nothing can run this until it is on the default branch.

Reproduces the Jenkins nightly: all three unit test projects
on seven OS/arch legs, across net8.0, net10.0 and net48.

The PR gate runs only Couchbase.UnitTests, so the
DependencyInjection and OpenTelemetry unit tests (103 tests)
have no CI coverage, and macos-latest being arm64 leaves x64
macOS uncovered. TFMs still come from SdkTestTargets rather
than being duplicated in YAML; build-and-test.yml is
untouched.

The temporary pull_request trigger must be removed before
merge - nothing can run this until it is on the default
branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread .github/workflows/nightly-unit-tests.yml Fixed
davidkelly and others added 7 commits July 29, 2026 14:53
Fixes the CodeQL actions/missing-workflow-permissions alert.
Checkout is the only step that needs the token.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The matrix is validated - all seven legs green, including
net48 on both Windows legs. Schedule and manual dispatch
only from here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Enumerate the unit test projects from the solution instead
of naming them, mirroring the **UnitTest** glob the Jenkins
nightly used. Hardcoding one project in build-and-test.yml
is how the DependencyInjection and OpenTelemetry tests went
unrun after the GHA migration; this stops the next project
added hitting the same trap. The loop runs every project
before failing once at the end, so the per-step
!cancelled() conditions are no longer needed.

ubuntu-22.04-arm restores a cell the Jenkins matrix had.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The project-discovery step and ubuntu-22.04-arm postdate the
last validation run. Remove again once the matrix is green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All eight legs green: 50 test runs, every leg discovering
all three projects, net48 on both Windows legs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Run one TFM per leg instead of letting a single dotnet test
invocation run them side by side. The framework is now in
the job name, so a red leg says which one broke, and each
test host gets its own runner - three TFMs sharing a machine
adds CPU contention that makes pool- and timing-sensitive
tests flakier.

Projects are skipped when they do not target the leg's TFM
(only Couchbase.UnitTests picks up net48), queried from
MSBuild rather than assumed. A leg that ends up testing
nothing fails, which is also what keeps net48 honest: if it
ever stops being added to SdkTestTargets on Windows, those
legs go red rather than quietly dropping the framework.

Costs a solution build per leg rather than per OS, which is
free on a public repo and does not change wall clock since
the legs run in parallel.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All 18 legs green in 5m05s, 50 test runs - identical
coverage to the pre-split matrix, with net48 correctly
limited to Couchbase.UnitTests on the Windows legs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#
# --blame-hang turns an intermittent hung test host into a fast red plus a dump, instead of a
# job that sits until the timeout with no diagnostics.
- name: Test - all unit test projects

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we could simplify this a fair bit by dropping the tfm matrix axis and giving each project its own step. dotnet test already runs every framework a project targets (so net48 still gets covered on the Windows part via SdkTestTargets, something I jsut noticed in build-and-test.yml actually. The separate build-and-test-net48.yml is redundant we can delete it).

From our friend Claude:

      - name: Test - Couchbase.UnitTests
        if: ${{ !cancelled() && steps.build.outcome == 'success' }}
        run: >
          dotnet test tests/Couchbase.UnitTests/Couchbase.UnitTests.csproj
          --configuration Release --no-build
          --logger "trx;LogFilePrefix=Couchbase.UnitTests" --results-directory TestResults
          --blame-hang --blame-hang-timeout 5min --blame-hang-dump-type full

      - name: Test - Couchbase.Extensions.DependencyInjection.UnitTests
        if: ${{ !cancelled() && steps.build.outcome == 'success' }}
        run: >
          dotnet test tests/Couchbase.Extensions.DependencyInjection.UnitTests/Couchbase.Extensions.DependencyInjection.UnitTests.csproj
          --configuration Release --no-build
          --logger "trx;LogFilePrefix=Couchbase.Extensions.DependencyInjection.UnitTests" --results-directory TestResults
          --blame-hang --blame-hang-timeout 5min --blame-hang-dump-type full

      - name: Test - Couchbase.Extensions.OpenTelemetry.UnitTests
        if: ${{ !cancelled() && steps.build.outcome == 'success' }}
        run: >
          dotnet test tests/Couchbase.Extensions.OpenTelemetry.UnitTests/Couchbase.Extensions.OpenTelemetry.UnitTests.csproj
          --configuration Release --no-build
          --logger "trx;LogFilePrefix=Couchbase.Extensions.OpenTelemetry.UnitTests" --results-directory TestResults
          --blame-hang --blame-hang-timeout 5min --blame-hang-dump-type full

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.

oh interesting - ok yea! I'll make the change and check that

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.

Agreed about the redundancy - was going to deal with that later but now is perfectly fine. I wanted to avoid contention (the PR gate has more, brings up timing issues, and figured here we needed a "cleaner" signal), hence the tfm matrix and so on. Here's what Claude thinks:

Thanks — took both suggestions.

LogFilePrefix: good catch, and worse than it looks. I pulled the artifact from an earlier run: Couchbase.UnitTests.trx had
total="2898", i.e. one framework's results, not both. LogFileName pins a single path so the frameworks overwrite each other. That means
every artifact build-and-test.yml has uploaded is missing most of its results — filing that separately rather than widening this PR.

build-and-test-net48.yml: agreed, deleted here. Same commit ran 2754 tests either way, so it was pure duplication at ~2m30s per PR.
Worth noting the check name Build and Unit Tests (Windows .NET Framework 4.8) will disappear from PRs — nothing requires it, the master
ruleset has no required status checks (and is currently disabled), so nothing should break.

On the tfm axis — I tried dropping it and hit something I didn't expect: dotnet test --framework net48 against a project that
doesn't target net48 exits 0 having run nothing. No error, no warning (verified with and without --no-build). So per-framework legs need
the MSBuild TargetFrameworks query and the ran == 0 guard, or a leg can go green having tested nothing.

The reason I want the axis isn't really the job names — it's contention. Three test hosts on one runner oversubscribe the CPU, and we hit
that on this very PR: SlicedMemoryOwnerTests went red on net48 in build-and-test.yml while the standalone net48 workflow passed on the
same SHA. Those tests assert MemoryPool.Rent(32) returns exactly 32 bytes, which the API doesn't guarantee — .NET Framework's pool hands
back 64 under contention, .NET Core doesn't. Separate ticket for the test.

On the hardcoded steps — this is the one I'm less sure about, so tell me what you think. The reason I went with discovery is that naming
projects by hand is how we got here: build-and-test.yml names one, and the DependencyInjection + OpenTelemetry tests (103, including the
Reflection.Emit proxy generators) have been compiled on every leg but never run since the GHA migration. Jenkins globbed for **UnitTest**
and always caught all three. My worry is the next *UnitTests project gets silently skipped the same way. The cost is that ~50 lines of
bash is meaningfully harder to read than three declarative steps, which is a fair hit. If you'd rather have the readable version, a middle
option is your three steps plus a two-line check that the solution still contains exactly three unit-test projects, so it fails loudly when
someone adds a fourth.

@emilienbev

Copy link
Copy Markdown
Contributor

LGTM but added a comment thinking of ways we could improve/simplify the workflow.
Found out the build-and-test-net48.yml is redundant, net48 is automatically targeted on WIndows in build-and-test.yml, so I think the explicit net4.8 TFM here isn't needed as well.

davidkelly and others added 3 commits July 30, 2026 08:49
The trx logger appends _<tfm>_<timestamp> to a prefix, so
results can never overwrite each other. LogFileName pins a
single path, and a project run against several frameworks
then keeps only the last one's results - which is what
build-and-test.yml does today, losing two thirds of the
results in its uploaded artifact.

Suggested by emilienbev in review.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
build-and-test.yml's windows leg already covers net48:
SdkTestTargets adds it on Windows, and dotnet test runs
every framework a project targets. Both ran the same 2754
tests on the same commit, so this workflow only duplicated
a check that already exists, at ~2m30s per pull request.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Confirms the LogFilePrefix change produces one trx per
project per framework. Remove again once green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
18/18 legs green, 50 test runs, and LogFilePrefix confirmed:
one trx per project per framework, no overwrites.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@davidkelly
davidkelly merged commit 6d20081 into master Jul 30, 2026
10 checks passed
@davidkelly
davidkelly deleted the dk/ncbc-4373 branch July 30, 2026 15:56
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.

3 participants