Add IAsyncDispose support #51
Conversation
|
I've opened this in draft mode because I don't yet know whether you would like SpecFlowOSS/SpecFlow#2575 to be implemented. That's the only reason I'm looking to add this. If you do want that, I want to prototype the SpecFlow side of the implementation to the point that it can validate that the BoDi changes in this PR are sufficient. I was aiming to set this to "ready for review" once I've done that validation. |
| @@ -705,7 +711,12 @@ public void RegisterInstanceAs(object instance, Type interfaceType, string name | |||
|
|
|||
| private static object GetPoolableInstance(object instance, bool dispose) | |||
| { | |||
| return (instance is IDisposable) && !dispose ? new NonDisposableWrapper(instance) : instance; | |||
| bool implementsDisposable = | |||
There was a problem hiding this comment.
I know the original code is that way, but could you use proper ifs here?
With the pragmas this gets unreadable for me. No idea what is going on.
| @@ -1008,16 +1019,59 @@ public void Dispose() | |||
| { | |||
| isDisposed = true; | |||
|
|
|||
| #if !ASYNC_DISPOSE | |||
There was a problem hiding this comment.
please not use negative pragmas
| container.ShouldImplement<IAsyncDisposable>(); | ||
| } | ||
|
|
||
| [Test/*ExpectedException(typeof(ObjectContainerException), ExpectedMessage = "disposed", MatchType = MessageMatch.Contains)*/] |
There was a problem hiding this comment.
is this comment somehow relevant?
|
The comment was from a time when SpecFlowOSS/SpecFlow#1614 was way in the future. So I think, when async/await is fixed in SpecFlow, this PR is useful. |
Although #49 was closed on the grounds that SpecFlow "won't probably use it for a long time", @SabotageAndi did say:
So here's a PR for it.
This necessarily expands the set of target frameworks. Previously, the target frameworks were
net45andnetcoreapp2.0, we need to addnetcoreapp3.1because each of these has a different relationship withIAsyncDisposablenet45: no support availablenetstandard2.0: implemented viaMicrosoft.Bcl.AsyncInterfacesNuGet packagenetcoreapp3.1: built into .NET runtime class librariesIf we continued to target only
netstandard2.0, we'd be introducing an unnecessary dependency onMicrosoft.Bcl.AsyncInterfacesfor any apps using .NET Core 3.1 or later. so although .NET Core 3.1 does of course supportnetstandard2.0, it's better to provide a .NET Core 3.1 target that doesn't need the extra reference to getIAsyncDisposable.