-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Akka.TestKit: ensure that EventFilter
respects WithinAsync
timeout blocks
#7541
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
Akka.TestKit: ensure that EventFilter
respects WithinAsync
timeout blocks
#7541
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Detailed my changes
*/ | ||
const double epsilonPercentage = 0.15; // 15% fudge factor | ||
const double minimumEpsilonValueMs = 50.0d; // 50ms minimum epsilon value | ||
epsilonValue ??= TimeSpan.FromMilliseconds(Math.Max(max.TotalMilliseconds * epsilonPercentage, minimumEpsilonValueMs)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calculate a default epsilon value
} | ||
else | ||
{ | ||
timeoutValue = _testkit.RemainingOrDefault; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure we use any time values set by Within
/ WithinAsync
I threw some TBD removals in here while I was poking around, since the test suite that was failing as a result of my first set of changes (have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some questions
src/core/Akka.TestKit/EventFilter/Internal/EventFilterApplier.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Akka.TestKit.Tests.TestKitBaseTests.WithinTests.Within_should_increase_max_timeout_by_the_provided_epsilon_value
Guess I'm going to need to look at that test too - first time I've seen it fail in 30-something attempts |
Changes
Discovered while debugging one of the
MergeHub
specs that a 10s timeout from aWithinAsync
block was not being passed to theEventFilter.ExpectOneAsync
method, which instead timed out to 3s.Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):