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

Fixed QueueSource PostStop and added a missing test case #4991

Merged
merged 2 commits into from
May 3, 2021

Conversation

ismaelhamed
Copy link
Member

Ports #23151 and #23509

@@ -153,7 +153,7 @@ public void QueueSink_should_timeout_future_when_stream_cannot_provide_data()
}, _materializer);
}

[Fact(Skip = "Racy, see https://github.com/akkadotnet/akka.net/pull/4424#issuecomment-632284459")]
[Fact]
Copy link
Member Author

Choose a reason for hiding this comment

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

Let's see if this was fixed too

t.Exception.InnerException.Should().BeOfType<IllegalStateException>();
}, TaskContinuationOptions.OnlyOnFaulted).Wait();
var exception = Record.ExceptionAsync(async () => await queue.PullAsync()).Result;
exception.Should().BeOfType<StreamDetachedException>();
Copy link
Member Author

Choose a reason for hiding this comment

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

Using XUnit API to catch the exception and unwrap. Also, this should always run, not just on OnlyOnFaulted, to make sure we catch regressions.

@ismaelhamed ismaelhamed force-pushed the fix-queuesourcespec branch from 331cd3a to 437a9b4 Compare May 1, 2021 10:13
Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

LGTM

@Aaronontheweb Aaronontheweb enabled auto-merge (squash) May 3, 2021 21:20
@Aaronontheweb Aaronontheweb merged commit 63e0fb9 into akkadotnet:dev May 3, 2021
@Aaronontheweb Aaronontheweb added this to the 1.4.20 milestone May 3, 2021
This was referenced May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants