From eee0cd5c2d118509d79ffe234ddf93f3cfa4a162 Mon Sep 17 00:00:00 2001 From: Marcus Griep Date: Sun, 17 May 2015 16:03:08 -0400 Subject: [PATCH 1/2] Add failing test for supervisor strategy constructor bug --- .../Actor/SupervisorStrategySpecs.cs | 99 +++++++++++++++++++ src/core/Akka.Tests/Akka.Tests.csproj | 1 + 2 files changed, 100 insertions(+) create mode 100644 src/core/Akka.Tests/Actor/SupervisorStrategySpecs.cs diff --git a/src/core/Akka.Tests/Actor/SupervisorStrategySpecs.cs b/src/core/Akka.Tests/Actor/SupervisorStrategySpecs.cs new file mode 100644 index 00000000000..52974d91a2f --- /dev/null +++ b/src/core/Akka.Tests/Actor/SupervisorStrategySpecs.cs @@ -0,0 +1,99 @@ +using System; +using Akka.Actor; +using Xunit; + +namespace Akka.Tests.Actor +{ + public class SupervisorStrategySpecs + { + public static readonly object[][] RetriesTestData = new[] + { + new object[] { new int?(), -1 }, + new object[] { new int?(-1), -1 }, + new object[] { new int?(0), 0 }, + new object[] { new int?(5), 5 }, + }; + + public static readonly object[][] TimeoutTestData = new[] + { + new object[] { new TimeSpan?(), -1 }, + new object[] { new TimeSpan?(System.Threading.Timeout.InfiniteTimeSpan), -1 }, + new object[] { new TimeSpan?(TimeSpan.FromMilliseconds(0)), 0 }, + new object[] { new TimeSpan?(TimeSpan.FromMilliseconds(100)), 100 }, + new object[] { new TimeSpan?(TimeSpan.FromMilliseconds(100).Add(TimeSpan.FromTicks(75))), 100 }, + new object[] { new TimeSpan?(TimeSpan.FromMilliseconds(10000)), 10000 }, + }; + + [Theory] + [MemberData("RetriesTestData")] + public void A_constructed_OneForOne_supervisor_strategy_with_nullable_retries_has_the_expected_properties(int? retries, int expectedRetries) + { + var uut = new OneForOneStrategy(retries, null, exn => Directive.Restart); + + Assert.Equal(uut.MaxNumberOfRetries, expectedRetries); + } + + [Theory] + [MemberData("TimeoutTestData")] + public void A_constructed_OneForOne_supervisor_strategy_with_nullable_timeouts_has_the_expected_properties(TimeSpan? timeout, int expectedTimeoutMilliseconds) + { + var uut = new OneForOneStrategy(-1, timeout, exn => Directive.Restart); + + Assert.Equal(uut.WithinTimeRangeMilliseconds, expectedTimeoutMilliseconds); + } + + [Theory] + [MemberData("RetriesTestData")] + public void A_constructed_OneForOne_supervisor_strategy_with_nullable_retries_and_a_decider_has_the_expected_properties(int? retries, int expectedRetries) + { + var uut = new OneForOneStrategy(retries, null, Decider.From(Directive.Restart)); + + Assert.Equal(uut.MaxNumberOfRetries, expectedRetries); + } + + [Theory] + [MemberData("TimeoutTestData")] + public void A_constructed_OneForOne_supervisor_strategy_with_nullable_timeouts_and_a_decider_has_the_expected_properties(TimeSpan? timeout, int expectedTimeoutMilliseconds) + { + var uut = new OneForOneStrategy(-1, timeout, Decider.From(Directive.Restart)); + + Assert.Equal(uut.WithinTimeRangeMilliseconds, expectedTimeoutMilliseconds); + } + + [Theory] + [MemberData("RetriesTestData")] + public void A_constructed_AllForOne_supervisor_strategy_with_nullable_retries_has_the_expected_properties(int? retries, int expectedRetries) + { + var uut = new AllForOneStrategy(retries, null, exn => Directive.Restart); + + Assert.Equal(uut.MaxNumberOfRetries, expectedRetries); + } + + [Theory] + [MemberData("TimeoutTestData")] + public void A_constructed_AllForOne_supervisor_strategy_with_nullable_timeouts_has_the_expected_properties(TimeSpan? timeout, int expectedTimeoutMilliseconds) + { + var uut = new AllForOneStrategy(-1, timeout, exn => Directive.Restart); + + Assert.Equal(uut.WithinTimeRangeMilliseconds, expectedTimeoutMilliseconds); + } + + [Theory] + [MemberData("RetriesTestData")] + public void A_constructed_AllForOne_supervisor_strategy_with_nullable_retries_and_a_decider_has_the_expected_properties(int? retries, int expectedRetries) + { + var uut = new OneForOneStrategy(retries, null, Decider.From(Directive.Restart)); + + Assert.Equal(uut.MaxNumberOfRetries, expectedRetries); + } + + [Theory] + [MemberData("TimeoutTestData")] + public void A_constructed_AllForOne_supervisor_strategy_with_nullable_timeouts_and_a_decider_has_the_expected_properties(TimeSpan? timeout, int expectedTimeoutMilliseconds) + { + var uut = new OneForOneStrategy(-1, timeout, Decider.From(Directive.Restart)); + + Assert.Equal(uut.WithinTimeRangeMilliseconds, expectedTimeoutMilliseconds); + } + } +} diff --git a/src/core/Akka.Tests/Akka.Tests.csproj b/src/core/Akka.Tests/Akka.Tests.csproj index ba7d59185c7..5ffbc9c7f09 100644 --- a/src/core/Akka.Tests/Akka.Tests.csproj +++ b/src/core/Akka.Tests/Akka.Tests.csproj @@ -124,6 +124,7 @@ + From 0931aec64daa61913a48a2feba2046721ce4f749 Mon Sep 17 00:00:00 2001 From: Marcus Griep Date: Sun, 17 May 2015 16:07:51 -0400 Subject: [PATCH 2/2] Fix Nullable Supervisor Strategy constructors In chaining to other constructors, the constructors for the `AllForOne` and `OneForOne` strategies pull timeouts from `TimeSpan`s using the `Milliseconds` property. This is not the correct behavior for `TimeSpan` values greater than or equal to 1 second as it provides only the milliseconds portion of the timespan. Instead, use the `TotalMilliseconds` property and cast it to an `int`. --- src/core/Akka/Actor/SupervisorStrategy.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/core/Akka/Actor/SupervisorStrategy.cs b/src/core/Akka/Actor/SupervisorStrategy.cs index fbe356dd36e..5879d3e6f6e 100644 --- a/src/core/Akka/Actor/SupervisorStrategy.cs +++ b/src/core/Akka/Actor/SupervisorStrategy.cs @@ -231,7 +231,7 @@ public IDecider Decider /// duration of the time window for maxNrOfRetries, Duration.Inf means no window. /// mapping from Exception to public OneForOneStrategy(int? maxNrOfRetries, TimeSpan? withinTimeRange, Func localOnlyDecider) - : this(maxNrOfRetries.GetValueOrDefault(-1), withinTimeRange.GetValueOrDefault(Timeout.InfiniteTimeSpan).Milliseconds, localOnlyDecider) + : this(maxNrOfRetries.GetValueOrDefault(-1), (int) withinTimeRange.GetValueOrDefault(Timeout.InfiniteTimeSpan).TotalMilliseconds, localOnlyDecider) { //Intentionally left blank } @@ -248,7 +248,7 @@ public OneForOneStrategy(int? maxNrOfRetries, TimeSpan? withinTimeRange, Funcduration of the time window for maxNrOfRetries, Duration.Inf means no window. /// mapping from Exception to public OneForOneStrategy(int? maxNrOfRetries, TimeSpan? withinTimeRange, IDecider decider) - : this(maxNrOfRetries.GetValueOrDefault(-1), withinTimeRange.GetValueOrDefault(Timeout.InfiniteTimeSpan).Milliseconds, decider) + : this(maxNrOfRetries.GetValueOrDefault(-1), (int) withinTimeRange.GetValueOrDefault(Timeout.InfiniteTimeSpan).TotalMilliseconds, decider) { //Intentionally left blank } @@ -265,7 +265,8 @@ public OneForOneStrategy(int? maxNrOfRetries, TimeSpan? withinTimeRange, IDecide /// duration in milliseconds of the time window for , negative values means no window. /// Mapping from an to /// If true failures will be logged - public OneForOneStrategy(int maxNrOfRetries, int withinTimeMilliseconds, Func localOnlyDecider, bool loggingEnabled = true) : this(maxNrOfRetries,withinTimeMilliseconds,new LocalOnlyDecider(localOnlyDecider),loggingEnabled) + public OneForOneStrategy(int maxNrOfRetries, int withinTimeMilliseconds, Func localOnlyDecider, bool loggingEnabled = true) + : this(maxNrOfRetries, withinTimeMilliseconds, new LocalOnlyDecider(localOnlyDecider), loggingEnabled) { //Intentionally left blank } @@ -403,7 +404,7 @@ public IDecider Decider /// duration of the time window for maxNrOfRetries, means no window. /// mapping from Exception to public AllForOneStrategy(int? maxNrOfRetries, TimeSpan? withinTimeRange, Func localOnlyDecider) - : this(maxNrOfRetries.GetValueOrDefault(-1), withinTimeRange.GetValueOrDefault(Timeout.InfiniteTimeSpan).Milliseconds, localOnlyDecider) + : this(maxNrOfRetries.GetValueOrDefault(-1), (int) withinTimeRange.GetValueOrDefault(Timeout.InfiniteTimeSpan).TotalMilliseconds, localOnlyDecider) { //Intentionally left blank } @@ -420,7 +421,7 @@ public AllForOneStrategy(int? maxNrOfRetries, TimeSpan? withinTimeRange, Funcduration of the time window for maxNrOfRetries, means no window. /// mapping from Exception to public AllForOneStrategy(int? maxNrOfRetries, TimeSpan? withinTimeRange, IDecider decider) - : this(maxNrOfRetries.GetValueOrDefault(-1), withinTimeRange.GetValueOrDefault(Timeout.InfiniteTimeSpan).Milliseconds, decider) + : this(maxNrOfRetries.GetValueOrDefault(-1), (int) withinTimeRange.GetValueOrDefault(Timeout.InfiniteTimeSpan).TotalMilliseconds, decider) { //Intentionally left blank } @@ -437,7 +438,8 @@ public AllForOneStrategy(int? maxNrOfRetries, TimeSpan? withinTimeRange, IDecide /// duration in milliseconds of the time window for , negative values means no window. /// Mapping from an to /// If true failures will be logged - public AllForOneStrategy(int maxNrOfRetries, int withinTimeMilliseconds, Func localOnlyDecider, bool loggingEnabled=true) : this(maxNrOfRetries,withinTimeMilliseconds,new LocalOnlyDecider(localOnlyDecider),loggingEnabled) + public AllForOneStrategy(int maxNrOfRetries, int withinTimeMilliseconds, Func localOnlyDecider, bool loggingEnabled=true) + : this(maxNrOfRetries, withinTimeMilliseconds, new LocalOnlyDecider(localOnlyDecider), loggingEnabled) { //Intentionally left blank }