From 9b1a0c464e1f26ec1fb0f11bd64d317500bc8849 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Sat, 8 Aug 2026 09:56:34 -0600 Subject: [PATCH] NCBC-4279: Stop unit tests asserting exact sizes on pooled buffers Motivation ========== Five tests in SlicedMemoryOwnerTests fail intermittently on net48 only, most recently the 2026-08-08 nightly (run 31243388414, windows-2025). They call MemoryPool.Shared.Rent(32) and then assume exactly 32 bytes, but Rent promises only *at least* that many. Given 64, the expected lengths 32/22/1 come back as 64/54/33 and start=33 is genuinely in range, so the guard correctly does not throw. net48 alone because .NET Framework's pool falls back to a larger bucket when the requested one is exhausted, where Core's allocates the exact bucket size. That makes it load-dependent, which is why the per-TFM split in nightly-unit-tests.yml reduced it without fixing it: xUnit still parallelizes collections within the net48 process. Four EnsureCapacity tests in OperationBuilderTests share the mechanism. Capacity returns the length of a buffer rented from ArrayPool, so asserting it doubles or quadruples asserts the pool's bucket rounding - OperationBuilder has no growth logic, EnsureCapacity just rents. These have not yet failed, the 16KB+ buckets being far less contended than the 32-byte one, but the defect is the same. Modification ============ SlicedMemoryOwnerTests: replace every Rent(32) with a FakeMemoryOwner over a plain byte[], as the ..._SliceStart tests in the same file already do. Two tests that cannot fail today are converted too, so the pattern is not left behind to be copied. OperationBuilderTests: assert capacity is at least what was requested, which is what EnsureCapacity guarantees and what its own Debug.Assert states, and rename _Doubles/_Quadruples to _Grows to match. No production code changes. Results ======= All 84 tests in the two classes pass on net8.0 locally. net48 cannot be built on macOS, so this shows the rewrites are behaviour-preserving, not that the flake is gone; the case for that is structural, since neither class now reads a pooled buffer's length. CI covers net48. Other MemoryPool uses in tests go through RentAndSlice, which slices to the requested length, and are unaffected. GetBuffer is likewise sliced to the written length; no test asserts on the length GetMemory/GetSpan return. Co-Authored-By: Claude Opus 5 (1M context) --- .../IO/Operations/OperationBuilderTests.cs | 42 ++++++++++++------- .../Utils/SlicedMemoryOwnerTests.cs | 18 +++++--- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/tests/Couchbase.UnitTests/Core/IO/Operations/OperationBuilderTests.cs b/tests/Couchbase.UnitTests/Core/IO/Operations/OperationBuilderTests.cs index 0cbb5c470..c1892b141 100644 --- a/tests/Couchbase.UnitTests/Core/IO/Operations/OperationBuilderTests.cs +++ b/tests/Couchbase.UnitTests/Core/IO/Operations/OperationBuilderTests.cs @@ -494,74 +494,84 @@ private void EnsureCapacity_Zero_NoChange() Assert.Equal(currentCapacity, builder.Capacity); } + // The growth tests below assert an inequality. EnsureCapacity rents from ArrayPool and + // promises only that the buffer is at least as large as requested; any doubling is the + // pool's bucket rounding, not something OperationBuilder implements. Asserting an exact + // size makes these depend on pool internals, which on .NET Framework overshoot to a larger + // bucket when the requested one is exhausted. + [Fact] - private void EnsureCapacity_OneMore_Doubles() + private void EnsureCapacity_OneMore_Grows() { // Arrange using var builder = new OperationBuilder(); - var currentCapacity = builder.Capacity; + var requested = builder.Capacity + 1; // Act - builder.EnsureCapacity(currentCapacity + 1); + builder.EnsureCapacity(requested); // Assert - Assert.Equal(currentCapacity * 2, builder.Capacity); + AssertAtLeast(requested, builder.Capacity); } [Fact] - private void EnsureCapacity_Double_Doubles() + private void EnsureCapacity_Double_Grows() { // Arrange using var builder = new OperationBuilder(); - var currentCapacity = builder.Capacity; + var requested = builder.Capacity * 2; // Act - builder.EnsureCapacity(currentCapacity * 2); + builder.EnsureCapacity(requested); // Assert - Assert.Equal(currentCapacity * 2, builder.Capacity); + AssertAtLeast(requested, builder.Capacity); } [Fact] - private void EnsureCapacity_OneMoreThanDouble_Quadruples() + private void EnsureCapacity_OneMoreThanDouble_Grows() { // Arrange using var builder = new OperationBuilder(); - var currentCapacity = builder.Capacity; + var requested = builder.Capacity * 2 + 1; // Act - builder.EnsureCapacity(currentCapacity * 2 + 1); + builder.EnsureCapacity(requested); // Assert - Assert.Equal(currentCapacity * 4, builder.Capacity); + AssertAtLeast(requested, builder.Capacity); } [Fact] - private void EnsureCapacity_Quadruple_Quadruples() + private void EnsureCapacity_Quadruple_Grows() { // Arrange using var builder = new OperationBuilder(); - var currentCapacity = builder.Capacity; + var requested = builder.Capacity * 4; // Act - builder.EnsureCapacity(currentCapacity * 4); + builder.EnsureCapacity(requested); // Assert - Assert.Equal(currentCapacity * 4, builder.Capacity); + AssertAtLeast(requested, builder.Capacity); } + private static void AssertAtLeast(int requested, int actualCapacity) => + Assert.True(actualCapacity >= requested, + $"Expected a capacity of at least {requested}, found {actualCapacity}."); + #endregion #region Helpers diff --git a/tests/Couchbase.UnitTests/Utils/SlicedMemoryOwnerTests.cs b/tests/Couchbase.UnitTests/Utils/SlicedMemoryOwnerTests.cs index a0e7818f1..33fd138ac 100644 --- a/tests/Couchbase.UnitTests/Utils/SlicedMemoryOwnerTests.cs +++ b/tests/Couchbase.UnitTests/Utils/SlicedMemoryOwnerTests.cs @@ -1,5 +1,4 @@ using System; -using System.Buffers; using System.Linq; using Couchbase.UnitTests.Helpers; using Couchbase.Utils; @@ -9,6 +8,13 @@ namespace Couchbase.UnitTests.Utils { public class SlicedMemoryOwnerTests { + // These tests use FakeMemoryOwner rather than MemoryPool.Shared.Rent, because Rent + // only promises *at least* the requested size. On .NET Framework the pool falls back to a + // larger bucket when the requested one is exhausted, so a Rent(32) under load returns 64 + // and every assertion below that depends on the buffer being exactly 32 bytes fails. + private static FakeMemoryOwner ExactlySized(int length) => + new FakeMemoryOwner(new byte[length]); + #region ctor1 [Fact] @@ -27,7 +33,7 @@ public void ctor1_NullMemoryOwner_ArgumentNullException() public void ctor1_StartOutsideRange_ArgumentOutOfRangeException(int start) { // Act/Assert - using (var memory = MemoryPool.Shared.Rent(32)) + using (var memory = ExactlySized(32)) { Assert.Throws(() => new SlicedMemoryOwner(memory, start)); } @@ -40,7 +46,7 @@ public void ctor1_StartOutsideRange_ArgumentOutOfRangeException(int start) public void ctor1_StartWithinRange_SliceUntilEnd(int start, int expectedLength) { // Act/Assert - using (var memory = new SlicedMemoryOwner(MemoryPool.Shared.Rent(32), start)) + using (var memory = new SlicedMemoryOwner(ExactlySized(32), start)) { Assert.Equal(expectedLength, memory.Memory.Length); } @@ -82,7 +88,7 @@ public void ctor2_NullMemoryOwner_ArgumentNullException() public void ctor2_NegativeStart_ArgumentOutOfRangeException() { // Act/Assert - using (var memory = MemoryPool.Shared.Rent(32)) + using (var memory = ExactlySized(32)) { Assert.Throws(() => new SlicedMemoryOwner(memory, -1, 10)); } @@ -96,7 +102,7 @@ public void ctor2_NegativeStart_ArgumentOutOfRangeException() public void ctor2_StartOutsideRange_ArgumentOutOfRangeException(int start) { // Act/Assert - using (var memory = MemoryPool.Shared.Rent(32)) + using (var memory = ExactlySized(32)) { Assert.Throws(() => new SlicedMemoryOwner(memory, start, 1)); } @@ -109,7 +115,7 @@ public void ctor2_StartOutsideRange_ArgumentOutOfRangeException(int start) public void ctor2_StartWithinRange_SliceLength(int start, int length) { // Act/Assert - using (var memory = new SlicedMemoryOwner(MemoryPool.Shared.Rent(32), start, length)) + using (var memory = new SlicedMemoryOwner(ExactlySized(32), start, length)) { Assert.Equal(length, memory.Memory.Length); }