Add AsyncEnumerable support for TestCaseSource, ValueSource, and TestFixtureSource

[jnm2]

The .NET Core API proposal for IAsyncDisposable and IAsyncEnumerable is up:
https://github.com/dotnet/corefx/issues/32640

The matching C# 8.0 features foreach async and using async and async iterators will land in a few months.

• Right now we call IDisposable.Dispose on fixtures. Would we want to recognize and await IAsyncDisposable.DisposeAsync on fixtures?

• Would we want Assert.That(asyncSequence, Is.EquivalentTo(expectedAsyncSequence)) to light up and just work? I lean towards thinking we should require await AsyncAssert.That( (once we add those APIs) in order to light this one up since it's not an entry point into user code from the framework the way Dispose/DisposeAsync is, and adapting async to sync is an ugly thing for user code work with.

The C# 8 language feature is shape-based, just like the existing async and foreach pattern recognition since C# 5 and 1. We can implement this using reflection without depending on .NET Core or C# 8.

[stevenaw]

@nunit/framework-team @nunit/core-team
Any objections to if I were to try to complete this for v4?

[manfred-brands]

None here, although you might want to split it in smaller chunks:

1. Disposal
2. Async Enumerable constraints

[stevenaw]

Yep, that was my thinking @manfred-brands . Disposable first as it's likely simpler and applicable to more types.

Curious, @nunit/framework-team , how do we feel about taking on a dependency on Microsoft.Bcl.AsyncInterfaces on the framework netfx build to support this? The original issue description described a shape-based solution but this issue is also long-standing enough that it predates that package's existence.

Our own tests here would also likely be simplified by taking on this dependency as well, but that can happen independent of any framework changes.

[manfred-brands]

At my work we added that dependency on multitargeting projects with no issues.

[jnm2]

It will mean that people who declare their own IAsyncDisposable interface in the system namespace (or use an alternate package) will not be supported. That could be fine.

[stevenaw]

Thanks @jnm2 !
Shape -based is fine with me too, and sounds like there could be a compatibility case for it too.

[stevenaw]

@nunit/framework-team
Looking for confirmation of a design decision or two.

I assume that in the case that a class has both a Dispose() and DisposeAsync() method that we should invoke only the async one and rely on the user to delegate any sync disposal internally in the DisposeAsync() implementation?
That is, at least, how I read the recommendation here : https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-disposeasync

I'm also leaning away from a truly shape-based approach and instead relying on the fixture code having an explicit implementation of the IAsyncDisposable interface (from a source of the user's choosing) without requiring the framework take a hard dependency on it. In another words, a reflection-based interface lookup. Based on the discussion here only ref structs can be truly shape based on this area: dotnet/csharplang#93

[manfred-brands]

we should invoke only the async one and rely on the user to delegate any sync disposal internally

I would agree with that, it doesn't make sense to call both.

leaning away from a truly shape-based approach

Not sure what you mean by shape-based? I did read the issue and the referred ref struct Dispose
Do you mean simply calling "DisposeAsync" if found, regardless of interface?

I'm happy with the library checking a fixture for an interface "System.IAsyncDisposable" by name.
That way we don't need to depend on "Microsoft.Bcl.AsyncInterface" (for net462) and if users declared their own, that is fine.
One could do this conditionally and the .NET 6.0 build tests on 'fixture is IAsyncDisposable'.

[jnm2]

When we said our code would be shape-based, we did mean for driving explicit implementations of IAsyncEnumerable. That's how we implement the part "from a source of the user's choosing." Ref structs are a great point, but we won't be dealing with ref structs since you can't use reflection on them last I heard (they can't be boxed).

[stevenaw]

Excellent! I was about to suggest that I might be misusing the term "shape-based" 😅
I meant that we could rely on the presence of an IAsyncEnumerable interface implementation here to identify the any presence of a method - as compared to the way we determine if a type supports custom awaiting - which it sounds like we're in agreement on.

And yes, definitely to opting in to a non-reflection based approach on non-legacy framework builds.