Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have MergedLinesEnumerable implement IAsyncEnumerable<string> #109

Open
wants to merge 30 commits into
base: release-1.7
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
3fddb89
Update README.md
madelson Nov 15, 2020
15815ce
Update README.md
madelson Mar 10, 2023
ca60f2a
Update README.md
madelson Mar 10, 2023
da45635
Update README.md
madelson Mar 10, 2023
e45f1f6
Update README.md
madelson Apr 4, 2023
87316df
Close #98: Have MergedLinesEnumerable implement IAsyncEnumerable<string>
Bartleby2718 Feb 26, 2024
7976a96
Merge branch 'release-1.7' into IAsyncEnumerable
Bartleby2718 Mar 7, 2024
d3720b9
Fix spaces, preprocessor directives, pull out common assertions, Add …
Bartleby2718 Mar 7, 2024
990605e
Use var
Bartleby2718 Mar 7, 2024
0ab96b0
Pass CancellationToken to GetAsyncEnumeratorInternal
Bartleby2718 Mar 7, 2024
fdd9137
Unfixed
Bartleby2718 Mar 8, 2024
750b1e0
Fix all bugs to pass all tests (but seeing some transient failures of…
Bartleby2718 Mar 10, 2024
caa9428
Clean up per Visual Studio's suggestions and remove comments
Bartleby2718 Mar 10, 2024
3b99204
Remove System.Linq.Async and use an extension method instead
Bartleby2718 Mar 10, 2024
6a2baae
Fix Condition for Microsoft.Bcl.AsyncInterfaces
Bartleby2718 Mar 10, 2024
983a28f
Revert primary constructor changes to fix CI failures
Bartleby2718 Mar 10, 2024
7104cea
Also revert collection expressions changes
Bartleby2718 Mar 10, 2024
f6a555a
Revert primary constructor in AsyncEnumerableAdapter
Bartleby2718 Mar 10, 2024
9030874
Revert primary constructor in AsyncEnumeratorAdapter
Bartleby2718 Mar 10, 2024
b2b7bbe
Revert collection expression in MergedLinesEnumerableTestBase
Bartleby2718 Mar 10, 2024
f9f0813
Fix spacing and variable names
Bartleby2718 Mar 11, 2024
8d18b9d
Fix preprocessor directives for IAsyncEnumerable/IAsyncEnumerator
Bartleby2718 Mar 11, 2024
5151a25
Replace AsyncEnumerableAdapter with AsAsyncEnumerable
Bartleby2718 Mar 11, 2024
97198a3
Revert to doing Task.WaitAll(task1, task2, consumeTask)
Bartleby2718 Mar 11, 2024
1420db9
Revert AsAsyncEnumerable changes to disallow repeated consumptions
Bartleby2718 Mar 11, 2024
4c68836
Minor style changes
Bartleby2718 Jun 29, 2024
263d7dd
Do not wait consumeTask with other tasks, and revert everything else
Bartleby2718 Jun 29, 2024
16b38df
Merge branch 'release-1.7' into IAsyncEnumerable
Bartleby2718 Jun 29, 2024
7830e3e
Try bumping the timeout, considering the CI pipeline
Bartleby2718 Jun 29, 2024
ff9af41
Try replacing Task.Run with an async method
Bartleby2718 Jul 1, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions MedallionShell.Tests/MedallionShell.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
<IsPackable>false</IsPackable>
<LangVersion>Latest</LangVersion>
<Nullable>enable</Nullable>
<GenerateDocumentationFile>True</GenerateDocumentationFile>
<CodeAnalysisRuleSet>..\stylecop.analyzers.ruleset</CodeAnalysisRuleSet>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<GenerateDocumentationFile>True</GenerateDocumentationFile>
<CodeAnalysisRuleSet>..\stylecop.analyzers.ruleset</CodeAnalysisRuleSet>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<NoWarn>1591</NoWarn>
<RootNamespace>Medallion.Shell.Tests</RootNamespace>
</PropertyGroup>
Expand All @@ -17,15 +17,16 @@
<PackageReference Include="nunit" Version="3.13.3" />
<PackageReference Include="NUnit3TestAdapter" Version="4.4.0-beta.1" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.4.1" />
<PackageReference Include="Moq" Version="4.7.63" />
<PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.435">
<PackageReference Include="Moq" Version="4.7.63" />
<PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="8.0.0" />
<PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.435">
<PrivateAssets>All</PrivateAssets>
</PackageReference>
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\MedallionShell\MedallionShell.csproj" />
<ProjectReference Include="..\SampleCommand\SampleCommand.csproj" />
<ProjectReference Include="..\SampleCommand\SampleCommand.csproj" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'net462'">
Expand Down
39 changes: 39 additions & 0 deletions MedallionShell.Tests/Streams/AsyncEnumerableAdapter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;

namespace Medallion.Shell.Tests.Streams;

Bartleby2718 marked this conversation as resolved.
Show resolved Hide resolved
public class AsyncEnumerableAdapter : IAsyncEnumerable<string>
{
private readonly IEnumerable<string> strings;

public AsyncEnumerableAdapter(IEnumerable<string> strings)
{
this.strings = strings;
}

public IAsyncEnumerator<string> GetAsyncEnumerator(CancellationToken cancellationToken = default) =>
// this does not allow consuming the same IEnumerable twice
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@madelson TestConsumeTwice started failing after making this change. Explanation from ChatGPT

In the AsyncEnumerableAdapter class, the GetAsyncEnumerator() method creates a new AsyncEnumeratorAdapter instance that wraps the original IEnumerator<string>. This means that it's directly using the original enumerator from the IEnumerable<string>, which can only be enumerated once. If you try to get the async enumerator twice, it will fail because the underlying enumerator has already been exhausted.

On the other hand, the AsAsyncEnumerable() extension method creates a new enumerator each time it's called. It does this by iterating over the IEnumerable<T> items in a foreach loop. This means you can call GetAsyncEnumerator() multiple times without an issue because each call creates a new enumerator.

So, if your test expects an InvalidOperationException when calling GetAsyncEnumerator() twice, it will fail when using AsAsyncEnumerable() because this method allows multiple enumerations. If you want to preserve the single-use behavior, you should stick with the AsyncEnumerableAdapter class. If you want to allow multiple enumerations, then AsAsyncEnumerable() is the way to go. It all depends on the specific requirements of your code.

For posterity, I added this comment.

new AsyncEnumeratorAdapter(this.strings.GetEnumerator());

private class AsyncEnumeratorAdapter : IAsyncEnumerator<string>
{
private readonly IEnumerator<string> enumerator;

public AsyncEnumeratorAdapter(IEnumerator<string> enumerator)
{
this.enumerator = enumerator;
}

public string Current => this.enumerator.Current;

public ValueTask DisposeAsync()
{
this.enumerator.Dispose();
return new(Task.CompletedTask);
Bartleby2718 marked this conversation as resolved.
Show resolved Hide resolved
}

public ValueTask<bool> MoveNextAsync() => new(this.enumerator.MoveNext());
}
}
13 changes: 13 additions & 0 deletions MedallionShell.Tests/Streams/MergedLinesEnumerableTestAsync.cs
Bartleby2718 marked this conversation as resolved.
Show resolved Hide resolved
Bartleby2718 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#if NETCOREAPP3_0_OR_GREATER || NETSTANDARD2_0_OR_GREATER
using System.Collections.Generic;
using System.IO;
using Medallion.Shell.Streams;

namespace Medallion.Shell.Tests.Streams;

public class MergedLinesEnumerableTestAsync : MergedLinesEnumerableTestBase
{
protected override IAsyncEnumerable<string> Create(TextReader reader1, TextReader reader2) =>
new MergedLinesEnumerable(reader1, reader2);
}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Medallion.Shell.Streams;
Expand All @@ -11,66 +10,68 @@

namespace Medallion.Shell.Tests.Streams
{
public class MergedLinesEnumerableTest
public abstract class MergedLinesEnumerableTestBase
{
protected abstract IAsyncEnumerable<string> Create(TextReader reader1, TextReader reader2);

[Test]
public void TestOneIsEmpty()
public async Task TestOneIsEmpty()
{
var empty1 = new StringReader(string.Empty);
var nonEmpty1 = new StringReader("abc\r\ndef\r\nghi\r\njkl");

var enumerable1 = new MergedLinesEnumerable(empty1, nonEmpty1);
var list1 = enumerable1.ToList();
var enumerable1 = this.Create(empty1, nonEmpty1);
var list1 = await enumerable1.ToListAsync();
list1.SequenceEqual(new[] { "abc", "def", "ghi", "jkl" })
.ShouldEqual(true, string.Join(", ", list1));

var empty2 = new StringReader(string.Empty);
var nonEmpty2 = new StringReader("a\nbb\nccc\n");
var enumerable2 = new MergedLinesEnumerable(nonEmpty2, empty2);
var list2 = enumerable2.ToList();
var enumerable2 = this.Create(nonEmpty2, empty2);
var list2 = await enumerable2.ToListAsync();
list2.SequenceEqual(new[] { "a", "bb", "ccc" })
.ShouldEqual(true, string.Join(", ", list2));
}

[Test]
public void TestBothAreEmpty()
public async Task TestBothAreEmpty()
{
var list = new MergedLinesEnumerable(new StringReader(string.Empty), new StringReader(string.Empty)).ToList();
var list = await this.Create(new StringReader(string.Empty), new StringReader(string.Empty)).ToListAsync();
list.Count.ShouldEqual(0, string.Join(", ", list));
}

[Test]
public void TestBothArePopulatedEqualSizes()
public async Task TestBothArePopulatedEqualSizes()
{
var list = new MergedLinesEnumerable(
var list = await this.Create(
new StringReader("a\nbb\nccc"),
new StringReader("1\r\n22\r\n333")
)
.ToList();
.ToListAsync();
string.Join(", ", list).ShouldEqual("a, 1, bb, 22, ccc, 333");
}

[Test]
public void TestBothArePopulatedDifferenceSizes()
public async Task TestBothArePopulatedDifferenceSizes()
{
var lines1 = string.Join("\n", new[] { "x", "y", "z" });
var lines2 = string.Join("\n", new[] { "1", "2", "3", "4", "5" });

var list1 = new MergedLinesEnumerable(new StringReader(lines1), new StringReader(lines2))
.ToList();
var list1 = await this.Create(new StringReader(lines1), new StringReader(lines2))
.ToListAsync();
string.Join(", ", list1).ShouldEqual("x, 1, y, 2, z, 3, 4, 5");

var list2 = new MergedLinesEnumerable(new StringReader(lines2), new StringReader(lines1))
.ToList();
var list2 = await this.Create(new StringReader(lines2), new StringReader(lines1))
.ToListAsync();
string.Join(", ", list2).ShouldEqual("1, x, 2, y, 3, z, 4, 5");
}

[Test]
public void TestConsumeTwice()
{
var enumerable = new MergedLinesEnumerable(new StringReader("a"), new StringReader("b"));
enumerable.GetEnumerator();
Assert.Throws<InvalidOperationException>(() => enumerable.GetEnumerator());
var asyncEnumerable = this.Create(new StringReader("a"), new StringReader("b"));
asyncEnumerable.GetAsyncEnumerator();
Assert.Throws<InvalidOperationException>(() => asyncEnumerable.GetAsyncEnumerator());
}

[Test]
Expand All @@ -84,54 +85,69 @@ void TestOneThrows(bool reverse)
mockReader.Setup(r => r.ReadLineAsync())
.ReturnsAsync(() => ++count < 3 ? "LINE" : throw new TimeZoneNotFoundException());

Assert.Throws<TimeZoneNotFoundException>(
() => new MergedLinesEnumerable(
Assert.ThrowsAsync<TimeZoneNotFoundException>(
async () => await this.Create(
reverse ? mockReader.Object : reader1,
reverse ? reader1 : mockReader.Object
)
.ToList()
).ToListAsync()
);
}

TestOneThrows(reverse: false);
TestOneThrows(reverse: true);
}

[Timeout(10000)] // something's wrong if it's taking more than 15 seconds
Bartleby2718 marked this conversation as resolved.
Show resolved Hide resolved
[Test]
public void FuzzTest()
{
var pipe1 = new Pipe();
var pipe2 = new Pipe();

var enumerable = new MergedLinesEnumerable(new StreamReader(pipe1.OutputStream), new StreamReader(pipe2.OutputStream));
var asyncEnumerable = this.Create(new StreamReader(pipe1.OutputStream), new StreamReader(pipe2.OutputStream));

var strings1 = Enumerable.Range(0, 2000).Select(_ => Guid.NewGuid().ToString()).ToArray();
var strings2 = Enumerable.Range(0, 2300).Select(_ => Guid.NewGuid().ToString()).ToArray();

void WriteStrings(IReadOnlyList<string> strings, Pipe pipe)
static void WriteStrings(IReadOnlyList<string> strings, TextWriter writer)
{
var spinWait = default(SpinWait);
var random = new Random(Guid.NewGuid().GetHashCode());
using (var writer = new StreamWriter(pipe.InputStream))
foreach (var line in strings)
{
foreach (var line in strings)
if (random.Next(110) == 1)
Bartleby2718 marked this conversation as resolved.
Show resolved Hide resolved
{
if (random.Next(4) == 1)
{
spinWait.SpinOnce();
}

writer.WriteLine(line);
spinWait.SpinOnce();
}

writer.WriteLine(line);
}
}

var task1 = Task.Run(() => WriteStrings(strings1, pipe1));
var task2 = Task.Run(() => WriteStrings(strings2, pipe2));
var consumeTask = Task.Run(() => enumerable.ToList());

var task1 = Task.Run(() =>
{
using StreamWriter writer1 = new(pipe1.InputStream);
WriteStrings(strings1, writer1);
});
var task2 = Task.Run(() =>
{
using StreamWriter writer2 = new(pipe2.InputStream);
WriteStrings(strings2, writer2);
});
var consumeTask = asyncEnumerable.ToListAsync();
Task.WaitAll(task1, task2, consumeTask);

CollectionAssert.AreEquivalent(strings1.Concat(strings2).ToList(), consumeTask.Result);
}
}
Bartleby2718 marked this conversation as resolved.
Show resolved Hide resolved

public static class AsyncEnumerableExtensions
{
public static async Task<List<string>> ToListAsync(this IAsyncEnumerable<string> strings)
Bartleby2718 marked this conversation as resolved.
Show resolved Hide resolved
{
List<string> result = new();
Bartleby2718 marked this conversation as resolved.
Show resolved Hide resolved
await foreach (var item in strings) { result.Add(item); }
return result;
}
}
}
11 changes: 11 additions & 0 deletions MedallionShell.Tests/Streams/MergedLinesEnumerableTestSync.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using System.Collections.Generic;
using System.IO;
using Medallion.Shell.Streams;

namespace Medallion.Shell.Tests.Streams;

public class MergedLinesEnumerableTestSync : MergedLinesEnumerableTestBase
{
protected override IAsyncEnumerable<string> Create(TextReader reader1, TextReader reader2) =>
new AsyncEnumerableAdapter(new MergedLinesEnumerable(reader1, reader2));
}
5 changes: 4 additions & 1 deletion MedallionShell/MedallionShell.csproj
Bartleby2718 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
</PropertyGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard1.3'">
<PackageReference Include="System.Diagnostics.Process" version="4.3.0" />
<PackageReference Include="System.Diagnostics.Process" version="4.3.0" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'net46' or '$(TargetFramework)' == 'net45'">
<PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
Expand Down Expand Up @@ -80,4 +80,7 @@
<LogicalName>MedallionShell.ProcessSignaler.exe</LogicalName>
</EmbeddedResource>
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
<PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="8.0.0" />
</ItemGroup>
</Project>
Loading