Skip to content

Commit

Permalink
Improve PipeWriterTests.CompleteWithLargeWriteThrows (#65506)
Browse files Browse the repository at this point in the history
Allocate the buffer outside of the loop so we don't hit OOM issues.

While looking at the test I noticed that it actually had a bug too:
Nothing was asserting that we indeed throw an InvalidOperationException when the writer is completed. Adding `Assert.ThrowsAsync` revealed that the current loop iteration count was too low to hit the exception reliably
within the 10ms delay so instead check for the execution time.

Fixes #64930
  • Loading branch information
akoeplinger authored Feb 18, 2022
1 parent fb69200 commit 3deadcf
Showing 1 changed file with 15 additions and 13 deletions.
28 changes: 15 additions & 13 deletions src/libraries/System.IO.Pipelines/tests/PipeWriterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -228,33 +228,35 @@ public async Task WritesUsingGetMemoryWorks()
pipe.Reader.Complete();
}

[Fact]
[SkipOnPlatform(TestPlatforms.Browser, "allocates too much memory")]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
public async Task CompleteWithLargeWriteThrows()
{
var completeDelay = TimeSpan.FromMilliseconds(10);
var testTimeout = TimeSpan.FromMilliseconds(10000);
var pipe = new Pipe();
pipe.Reader.Complete();

var task = Task.Run(async () =>
{
await Task.Delay(10);
await Task.Delay(completeDelay);
pipe.Writer.Complete();
});

try
// Complete while writing
await Assert.ThrowsAsync<InvalidOperationException>(async () =>
{
for (int i = 0; i < 1000; i++)
var testStartTime = DateTime.UtcNow;
var buffer = new byte[10000000];
ulong i = 0;
while (true)
{
var buffer = new byte[10000000];
await pipe.Writer.WriteAsync(buffer);
}
}
catch (InvalidOperationException)
{
// Complete while writing
}
await task;
// abort test if we're executing for more than the testTimeout (check every 10000th iteration)
if (i++ % 10000 == 0 && DateTime.UtcNow - testStartTime > testTimeout)
break;
}
});
}

[Fact]
Expand Down

0 comments on commit 3deadcf

Please sign in to comment.