ci(coverage): fix hang by excluding Redis Testcontainers from coverage loop#99
Merged
Merged
Conversation
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
5e902f7 to
40570d1
Compare
6141501 to
40570d1
Compare
Owner
Author
|
Reset to original approved commit (40570d1). The two follow-up commits expanded scope by converting the solution-level verify steps to per-project loops, which broke Cli.Tests and Audit.Tests for unrelated reasons. The original hang was only in the coverage loop, which 40570d1 fixes correctly. Re-running CI now. |
…e loop The 'Collect coverage (per-project)' step iterated through all test projects including ExperimentFramework.Governance.Persistence.Redis.Tests, which uses Testcontainers.Redis. Even though integration tests are filtered out via --filter, Testcontainers starts the Redis container in IAsyncLifetime.InitializeAsync before the filter applies, causing the test host to hang indefinitely waiting for Docker. The 60-minute job timeout was being hit on most runs. Fix: - Remove Redis.Tests from the coverage project list in both pr-checks and release jobs - Add a runtime guard skipping any project matching *Redis.Tests* - Wrap each dotnet test invocation with timeout 300 as defense-in-depth - Surface timeouts as ::warning:: while propagating real failures via exit code (previously all non-zero exits were silently swallowed) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…fy-step hang
xUnit's IAsyncLifetime fires during fixture construction BEFORE
the test filter is applied. Per-class IAsyncLifetime on the three
Redis test classes meant each class instantiated and started a
Testcontainers Redis container regardless of whether any of its
tests passed the Category!=integration filter. On CI runners
this blocked the solution-level dotnet test for the net8.0 and
net9.0 target framework runs indefinitely, hitting the 60-minute
job timeout.
Switching to a shared ICollectionFixture<RedisFixture> means the
fixture is instantiated only when xUnit finds at least one
non-filtered test in the [Collection("Redis")] collection. Since
all tests in these classes are tagged Category=integration, the
filter eliminates them at discovery time and the container never
starts.
Also added --blame-hang-timeout 60s to the verify step's
dotnet test invocations as defense in depth so any future
hang surfaces as a fast, visible failure.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…sink DI cycle - ConfigurationFileDiscovery now sorts files alphabetically so the "last file wins" merge contract holds across filesystems. - AddExperimentAuditComposite captures sink ServiceDescriptors at registration time and resolves them by ImplementationType to avoid the circular IAuditSink singleton resolution that caused a 60s blame-hang in Audit.Tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
5e5ef4d to
ea2ee84
Compare
The tests/ExperimentFramework.Configuration.Tests project does not exist on this branch (only on feat/coverage-phase-5) and was added to the coverage loop prematurely in bbab3c6. The previous Redis Testcontainers hang masked this — now that the hang is fixed, the coverage step reaches the missing project and fails with MSB1009. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Code Coverage |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #99 +/- ##
==========================================
+ Coverage 77.07% 80.81% +3.74%
==========================================
Files 250 264 +14
Lines 9888 10494 +606
Branches 1634 1837 +203
==========================================
+ Hits 7621 8481 +860
+ Misses 2267 2013 -254
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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
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
Collect coverage (per-project)step iterated through all test projects includingExperimentFramework.Governance.Persistence.Redis.Tests, which uses Testcontainers.Redis--filterexcluding integration tests, Testcontainers starts the Redis container inIAsyncLifetime.InitializeAsyncbefore the filter applies, causing indefinite hang waiting for DockerFix
Redis.Testsfrom the coverage project list in bothpr-checksandreleasejobs*Redis.Tests*dotnet testinvocation withtimeout 300as defense-in-depth::warning::while propagating real failures via exit code (previously all non-zero exits were silently swallowed)Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com