Skip to content

test: make allocation assertion deterministic#348

Merged
thomhurst merged 1 commit into
mainfrom
issue-346-fix-allocation-test
Jul 21, 2026
Merged

test: make allocation assertion deterministic#348
thomhurst merged 1 commit into
mainfrom
issue-346-fix-allocation-test

Conversation

@thomhurst

Copy link
Copy Markdown
Owner

Summary

  • Measure wrapper construction with GC.GetAllocatedBytesForCurrentThread() instead of process-wide live heap size.
  • Cache the task factory before measurement so the delta covers only the wrapper array and completion sources.
  • Retain the existing value-type assertions that detect a regression from structs to heap-allocated wrapper objects.

Root cause

GC.GetTotalMemory(false) includes unrelated allocations from concurrently running TUnit tests and target frameworks. That made the fixed 200 KB threshold nondeterministic even when wrapper behavior was unchanged.

Test plan

  • Two consecutive dotnet test TomLonghurst.EnumerableAsyncProcessor.sln -c Release passes
  • 1,074/1,074 tests passed per run across concurrent net8.0 and net9.0 execution
  • git diff --check

Closes #346

@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes the wrapper allocation test deterministic. The main changes are:

  • Replaces process-wide live-memory measurement with per-thread allocated-byte counters.
  • Creates the shared task factory before the measured region.
  • Keeps the existing allocation threshold and value-type checks.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • Both allocation snapshots occur in the same uninterrupted synchronous region.
  • The cached delegate matches the test's goal of measuring wrapper storage and completion sources.

Important Files Changed

Filename Overview
EnumerableAsyncProcessor.UnitTests/TaskWrapperStructValidationTests.cs Uses thread-local allocation accounting and excludes task-factory creation from the measured wrapper-construction region.

Reviews (1): Last reviewed commit: "test: isolate wrapper allocation measure..." | Re-trigger Greptile

@thomhurst
thomhurst merged commit 022bda4 into main Jul 21, 2026
5 checks passed
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.

Fix flaky process-wide allocation assertion

1 participant