-
Couldn't load subscription status.
- Fork 0
Update to .NET 9 and other updates #2
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
base: main
Are you sure you want to change the base?
Changes from all commits
bebe59d
56bf0d6
f3a5b8d
1f79e60
3712995
f535fc7
a34a556
3e0ff76
627edbb
011800c
0f7401c
1dba7f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1 @@ | ||
| bin/ | ||
| obj/ | ||
| /artifacts/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| namespace Dispownership.Test; | ||
|
|
||
| public sealed class AsyncDisposableTest | ||
| { | ||
| [Fact] | ||
| public async Task DisposesInnerValueIfOwned() | ||
| { | ||
| var value = new AsyncDisposableStub(); | ||
|
|
||
| await using (AsyncDisposable.Owned(value)) | ||
| { | ||
| } | ||
|
|
||
| Assert.True(value.Disposed); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task DoesNotDisposeInnerValueIfBorrowed() | ||
| { | ||
| var value = new AsyncDisposableStub(); | ||
|
|
||
| await using (AsyncDisposable.Borrowed(value)) | ||
| { | ||
| } | ||
|
|
||
| Assert.False(value.Disposed); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task DoesNotDisposeOwnedValueAfterMove() | ||
| { | ||
| var value = new AsyncDisposableStub(); | ||
|
|
||
| await using (var disposable = AsyncDisposable.Owned(value)) | ||
| { | ||
| _ = disposable.Take(); | ||
| } | ||
|
|
||
| Assert.False(value.Disposed); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task ThrowsWhenMovingAValueTwice() | ||
| { | ||
| await using var disposable = AsyncDisposable.Owned(new AsyncDisposableStub()); | ||
| _ = disposable.Take(); | ||
| Assert.Throws<InvalidOperationException>(() => disposable.Take()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task ThrowsWhenMovingABorrowedValue() | ||
| { | ||
| await using var disposable = AsyncDisposable.Borrowed(new AsyncDisposableStub()); | ||
| Assert.Throws<InvalidOperationException>(() => disposable.Take()); | ||
| } | ||
|
|
||
| [Theory] | ||
| [MemberData(nameof(Disposables))] | ||
| public async Task ThrowsWhenAccessingADisposedValue(AsyncDisposable<AsyncDisposableStub> disposable) | ||
| { | ||
| #pragma warning disable IDISP007 | ||
| await disposable.DisposeAsync(); | ||
| #pragma warning restore IDISP007 | ||
| Assert.Throws<ObjectDisposedException>(() => _ = disposable.Value); | ||
| Assert.Throws<ObjectDisposedException>(() => _ = disposable.Take()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task OnlyCallsDisposeOnTheInnerValueOnce() | ||
| { | ||
| #pragma warning disable IDISP001 | ||
| var stub = new CountingDisposableStub(); | ||
| var disposable = AsyncDisposable.Owned(stub); | ||
| #pragma warning restore IDISP001 | ||
|
|
||
| foreach (var unused in Enumerable.Range(0, count: 10)) | ||
| { | ||
| await disposable.DisposeAsync(); | ||
| } | ||
|
|
||
| Assert.Equal(1, stub.Disposed); | ||
| } | ||
|
|
||
| public static TheoryData<AsyncDisposable<AsyncDisposableStub>> Disposables() | ||
| => [ | ||
| AsyncDisposable.Owned(new AsyncDisposableStub()), | ||
| AsyncDisposable.Borrowed(new AsyncDisposableStub()), | ||
| ]; | ||
|
|
||
| public sealed class AsyncDisposableStub : IAsyncDisposable | ||
| { | ||
| public bool Disposed { get; private set; } | ||
|
|
||
| public ValueTask DisposeAsync() | ||
| { | ||
| Disposed = true; | ||
| return ValueTask.CompletedTask; | ||
| } | ||
| } | ||
|
|
||
| public sealed class CountingDisposableStub : IAsyncDisposable | ||
| { | ||
| public int Disposed { get; private set; } | ||
|
|
||
| public ValueTask DisposeAsync() | ||
| { | ||
| Disposed += 1; | ||
| return default; | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,3 @@ | ||
| using System; | ||
| using Xunit; | ||
|
|
||
| namespace Dispownership.Test; | ||
|
|
||
| public sealed class DisposableTest | ||
|
|
@@ -67,10 +64,52 @@ public void ThrowsWhenMovingABorrowedValue() | |
| Assert.Throws<InvalidOperationException>(() => disposable.Take()); | ||
| } | ||
|
|
||
| private sealed class DisposableStub : IDisposable | ||
| [Theory] | ||
| [MemberData(nameof(Disposables))] | ||
| public void ThrowsWhenAccessingADisposedValue(Disposable<DisposableStub> disposable) | ||
| { | ||
| #pragma warning disable IDISP007 | ||
| disposable.Dispose(); | ||
| #pragma warning restore IDISP007 | ||
| Assert.Throws<ObjectDisposedException>(() => _ = disposable.Value); | ||
| Assert.Throws<ObjectDisposedException>(() => _ = disposable.Take()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void OnlyCallsDisposeOnTheInnerValueOnce() | ||
| { | ||
| #pragma warning disable IDISP001 | ||
| var stub = new CountingDisposableStub(); | ||
| var disposable = Disposable.Owned(stub); | ||
| #pragma warning restore IDISP001 | ||
|
|
||
| foreach (var unused in Enumerable.Range(0, count: 10)) | ||
| { | ||
| disposable.Dispose(); | ||
| } | ||
|
|
||
| Assert.Equal(1, stub.Disposed); | ||
| } | ||
|
|
||
| #pragma warning disable IDISP004 | ||
| public static TheoryData<Disposable<DisposableStub>> Disposables() | ||
| => [ | ||
| Disposable.Owned(new DisposableStub()), | ||
| Disposable.Borrowed(new DisposableStub()), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see why this test case should throw on double dispose, isn't this the point, that it is not disposed when borrowed? Maybe I just don't understand. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the inner disposable is not disposed when borrowed. What I changed is that the wrapper tracks that it got disposed and throws in that case. I thought it would be a good thing for consistency since that's what other disposables do too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, never mind I misunderstood the dispose pattern 😆 Dispose should do nothing when called multiple times, but anything that accesses the resource should throw. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure we should implement this for our consumers. I know this is correct according the dispose pattern as per Microsoft, but if we track dispose, the contained value can't decide if it throws when used after dispose. This is a huge change and not necessarily in the interest of consumers, I fear? What do you think? There might be existing code that doesn't care if dispose is called, and then take after. This would be a breaking change for them, at least theoretically. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree it might be a bit of a change, but I think it's a good one :) Consumers of the disposable wrapper currently see two different behaviours when the wrapper is disposed:
One of the use cases of the disposable is to treat owned and borrowed objects the same way. Always throwing if disposed might help catch bugs :) |
||
| ]; | ||
| #pragma warning restore IDISP004 | ||
|
|
||
| public sealed class DisposableStub : IDisposable | ||
| { | ||
| public bool Disposed { get; private set; } | ||
|
|
||
| public void Dispose() => Disposed = true; | ||
| } | ||
|
|
||
| public sealed class CountingDisposableStub : IDisposable | ||
| { | ||
| public int Disposed { get; private set; } | ||
|
|
||
| public void Dispose() => Disposed += 1; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,21 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <PropertyGroup> | ||
| <TargetFramework>net7.0</TargetFramework> | ||
| <TargetFramework>net9.0</TargetFramework> | ||
| <IsPackable>false</IsPackable> | ||
| <OutputType>Exe</OutputType> | ||
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <Nullable>enable</Nullable> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <Using Include="Xunit" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <ProjectReference Include="..\Dispownership\Dispownership.csproj" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" /> | ||
| <PackageReference Include="xunit" /> | ||
| <PackageReference Include="xunit.v3" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" /> | ||
| </ItemGroup> | ||
| </Project> |
Uh oh!
There was an error while loading. Please reload this page.