Skip to content

test: stop WebIntegrationTests.Versioning asserting an exact structured-log count - #5638

Merged
jamescrosswell merged 1 commit into
version7from
fix/versioning-snapshot-flake
Sep 29, 2026
Merged

jamescrosswell merged 1 commit into
version7from
fix/versioning-snapshot-flake

Conversation

@jamescrosswell

Copy link
Copy Markdown
Collaborator

Fixes the recurring Sentry.AspNetCore.Tests.WebIntegrationTests.Versioning snapshot flake on version7.

StructuredLog.Length is _items.Length — the number of structured log items in the envelope batch. In this test that is however many logs ASP.NET Core's own infrastructure happens to emit while serving one request, and they race the flush triggered by server.Dispose(). CI alternates between Length: 7 and Length: 6, so the assertion is nondeterministic by design. Most recently it failed on both linux-arm64 (net8.0) and linux-x64 (net11.0) in a single run.

The fix ignores that one member for this test and regenerates the four WebIntegrationTests.Versioning.DotNet* snapshots (Source: { Length: 7 } → Source: {}). Nothing else in them changes, and all four TFMs regenerated on macOS.

Why not in IgnoreStandardSentryMembers

The issue suggests putting the ignore in the shared test/Sentry.Testing/VerifyExtensions.cs helper, on the basis that "only one committed snapshot contains a Length: line". That turns out not to hold — there are 19:

  • 4 × WebIntegrationTests.Versioning.DotNet* (this test)
  • 15 across the Serilog, NLog and log4net integration-test projects

In all 19, Length: is the only member inside Source: { }, so it provides no real coverage anywhere. But changing the shared helper would rewrite all 19, including IntegrationTests.Simple.Net4_8.verified.txt and IntegrationTests.Simple.Mono4_0.verified.txt in the Serilog and NLog projects, which can't be regenerated on macOS and would need hand-editing. Only Versioning actually flakes — the other 18 are single-threaded tests with deterministic log counts. Keeping the change local also keeps it clear of the four stacked logging PRs in flight against version7 (#5573, #5585, #5592, #5595).

Ignoring it in the shared helper is still the tidier end state; worth a follow-up alongside a CI-sourced regeneration of the .NET Framework/Mono snapshots.

main does not need this

On main, EnableLogs still defaults to false, so no StructuredLog item reaches this test's envelopes and its snapshots contain no Length: line at all. The count only became load-bearing on version7, where #5504 made logs always enabled. main inherits the fix when version7 merges.

Unrelated to #5617 (msbuild integration-test flush timeout on net9.0 Windows) — different test, different signature.

Closes #5616

#skip-changelog

🤖 Generated with Claude Code

…ed-log count

StructuredLog.Length is the number of SentryLog items in the envelope batch. In
this test that is however many logs ASP.NET Core's own infrastructure emits while
serving one request, and they race the flush triggered by server.Dispose(), so CI
alternates between 7 and 6.

Ignore the member for this test rather than in IgnoreStandardSentryMembers: 19
committed snapshots contain a Length: line, two of which (the Net4_8 and Mono4_0
Serilog/NLog integration snapshots) cannot be regenerated on macOS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (version7@aa02031). Learn more about missing BASE report.

Additional details and impacted files
@@             Coverage Diff             @@
##             version7    #5638   +/-   ##
===========================================
  Coverage            ?   74.75%           
===========================================
  Files               ?      515           
  Lines               ?    18905           
  Branches            ?     3689           
===========================================
  Hits                ?    14133           
  Misses              ?     3894           
  Partials            ?      878           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jamescrosswell
jamescrosswell marked this pull request as ready for review September 29, 2026 04:25
@github-actions github-actions Bot added the risk: low PR risk score: low label Sep 29, 2026
@ric-oliv
ric-oliv self-requested a review September 29, 2026 08:18
@jamescrosswell
jamescrosswell merged commit 7625e24 into version7 Sep 29, 2026
31 checks passed
@jamescrosswell
jamescrosswell deleted the fix/versioning-snapshot-flake branch September 29, 2026 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: low PR risk score: low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants