Skip to content
This repository was archived by the owner on Aug 2, 2023. It is now read-only.

Add more cancellation tests #1489

Merged
merged 4 commits into from
Apr 20, 2017
Merged

Add more cancellation tests #1489

merged 4 commits into from
Apr 20, 2017

Conversation

pakrym
Copy link
Contributor

@pakrym pakrym commented Apr 20, 2017

No description provided.

Pipe.Writer.WriteAsync(new byte[] {});
Pipe.Reader.CancelPendingRead();

Assert.True(awaitable.IsCompleted);
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this assert in the other test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason

awaitable.OnCompleted(() => {});

Pipe.Reader.Advance(Pipe.Reader.ReadAsync().GetResult().Buffer.End);
Pipe.Writer.CancelPendingFlush();
Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense to test flipping the order of the above two lines to verify Advance won't clobber the canceled state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding

Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

While I would have loved actually awaiting instead of faking it by calling IsCompleted/OnCompleted manually, this was a pretty clever way to avoid mocking an IScheduler.

@pakrym pakrym merged commit 3e5372e into master Apr 20, 2017
@@ -215,6 +215,44 @@ public void FlushAsyncCompletedAfterPreCancellation()
}

[Fact]
public void FlushAsyncReturnsIsCancelOnCancelPendingFlushBeforeGetResult()

Choose a reason for hiding this comment

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

nit: ReturnsIsCancelled

@pakrym pakrym deleted the pakrym/c-test branch January 26, 2018 19:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants