[Async TestKit] Fix WithinAsync and Remaining logic #6010
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Arkatufus
changed the title
…com:Arkatufus/akka.net into async_testkit/Fix_WithinAsync_and_Remaining
|
@Arkatufus looks like you have a DocFx build error here |
|
Looks like it creeped past the last merge, it should be fixed in #6012 |
Aaronontheweb
approved these changes
Jun 24, 2022
There was a problem hiding this comment.
LGTM - caught a lot of small errors in the testkit itself. Should help a lot with test hygiene going forward. Nice work @Arkatufus
| @@ -150,7 +150,8 @@ protected async Task<TimeSpan> TimeUntilPassivate(IActorRef region, TestProbe pr | |||
| probe.ExpectMsg<Entity.GotIt>().Id.ShouldBe("2"); | |||
|
|
|||
| var timeSinceOneSawAMessage = DateTime.Now.Ticks - timeOneSawMessage; | |||
| return settings.PassivateIdleEntityAfter - TimeSpan.FromTicks(timeSinceOneSawAMessage) + smallTolerance; | |||
| var time = settings.PassivateIdleEntityAfter - TimeSpan.FromTicks(timeSinceOneSawAMessage) + smallTolerance; | |||
| return time < smallTolerance ? smallTolerance : time; | |||
There was a problem hiding this comment.
LGTM - prevents negative timespans from being returned. Nice catch.
| @@ -94,7 +94,7 @@ await outputStream.WriteAsync(_bytesArray, 0, _bytesArray.Length) | |||
| var f = outputStream.FlushAsync(); | |||
|
|
|||
| await ExpectTimeout(f, Timeout); | |||
| await probe.ExpectNoMsgAsync(TimeSpan.MinValue); | |||
There was a problem hiding this comment.
Yep, this was definitely an error.
| @@ -130,7 +130,7 @@ public static bool IsInfiniteTimeout(this TimeSpan? timeSpan) | |||
| public static void EnsureIsPositiveFinite(this TimeSpan timeSpan, string parameterName) | |||
| { | |||
| if(!IsPositiveFinite(timeSpan)) | |||
| throw new ArgumentException($"The timespan must be greater than zero. Actual value: {timeSpan}", nameof(parameterName)); | |||
There was a problem hiding this comment.
Yeah, this was a bug. Nice catch.
| throw new InvalidOperationException(@"Remaining may not be called outside of ""within"""); | ||
|
|
||
| if (_testState.End < TimeSpan.Zero) | ||
| throw new InvalidOperationException($"End can not be negative, was: {_testState.End}"); |
| @@ -448,7 +456,7 @@ protected TimeSpan RemainingOr(TimeSpan duration) | |||
| public TimeSpan RemainingOrDilated(TimeSpan? duration) | |||
| { | |||
| if(!duration.HasValue) return RemainingOrDefault; | |||
| if(duration.IsInfinite()) throw new ArgumentException("max duration cannot be infinite"); | |||
There was a problem hiding this comment.
This only considered -1, whereas now we check for all possible negative timespan values.
| @@ -267,7 +267,7 @@ public async ValueTask<object> ReceiveOneAsync(TimeSpan? max = null, Cancellatio | |||
| } | |||
| else if (maxDuration == Timeout.InfiniteTimeSpan) | |||
| { | |||
| ConditionalLog(shouldLog, "Trying to receive message from TestActor queue. Will wait indefinitely."); | |||
| Log.Warning("Trying to receive message from TestActor queue with infinite timeout! Will wait indefinitely!"); | |||
There was a problem hiding this comment.
Need to explicitly warn here.
| var executionTask = function(); | ||
| // Limit the execution time block to the maximum allowed execution time. | ||
| // 200 milliseconds is added because Task.Delay() timer is not precise and can return prematurely. | ||
| var resultTask = await Task.WhenAny(executionTask, Task.Delay(max + 200.Milliseconds(), cts.Token)); |
There was a problem hiding this comment.
Got it, so this ensures that even if the CTS isn't fired we can still cap the execution time for any spec to max + 200.milliseconds
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Remainingalways return a positiveTimeSpanWithinAsyncwill wait for the function block to complete. Prevents bad unit tests from deadlocking the entire unit test suite.WithinAsyncuse thecancellationTokenpassed in the function parameterDilated