Skip to content

Commit

Permalink
Remove Hudl.Config and make it injectable
Browse files Browse the repository at this point in the history 
A few things didn't carry over as nicely as I'd have liked, need to come
back and revisit a few TODOs left in this commit.
  • Loading branch information
@robhruska
robhruska committed Apr 3, 2017
1 parent 0e36dd6 commit 13320cc
Show file tree
Hide file tree
Showing 24 changed files with 848 additions and 477 deletions.
221 changes: 167 additions & 54 deletions Hudl.Mjolnir.Tests/Breaker/FailurePercentageCircuitBreakerTests.cs
View file

Large diffs are not rendered by default.

16 changes: 10 additions & 6 deletions Hudl.Mjolnir.Tests/Command/BaseCommandTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
using System;
using System.Threading;
using System.Threading.Tasks;
using Hudl.Config;
using Hudl.Mjolnir.Command;
using Hudl.Mjolnir.Tests.Helper;
using Xunit;
using Hudl.Mjolnir.Key;
using Hudl.Mjolnir.External;
using Moq;

namespace Hudl.Mjolnir.Tests.Command
{
Expand Down Expand Up @@ -222,15 +223,18 @@ public void NoInvocationTimeout_NoConfiguredTimeout_NoConstructorTimeout_UsesDef

private void TestTimeouts(TimeSpan expected, long? invocationMs, long configuredMs, long? constructorMs)
{
// The 'configured' value can't be nullable because the Hudl.Config library may pass
// along a default(long) (i.e. 0) if the config value isn't set, and we need to be
// prepared for that and not treat it as an actual timeout.
// The 'configured' value can't be nullable because the injected config
// implementation may pass along a default(long) (i.e. 0) if the config value
// isn't set, and we need to be prepared for that and not treat it as an actual
// timeout.

var constructorTs = (constructorMs == null ? (TimeSpan?) null : TimeSpan.FromMilliseconds(constructorMs.Value));
var command = new TestCommand(AnyString, AnyString, AnyString, constructorTs);
ConfigProvider.Instance.Set("mjolnir.command." + command.Name + ".Timeout", configuredMs);

var determined = command.DetermineTimeout(invocationMs);
var mockConfig = new Mock<IConfig>(MockBehavior.Strict);
mockConfig.Setup(m => m.GetConfig<long>($"mjolnir.command.{command.Name}.Timeout", It.IsAny<long>())).Returns(configuredMs);

var determined = command.DetermineTimeout(mockConfig.Object, invocationMs);

Assert.Equal(expected, determined);
}
Expand Down
95 changes: 73 additions & 22 deletions Hudl.Mjolnir.Tests/Command/BulkheadInvokerTests.cs
View file

Large diffs are not rendered by default.

97 changes: 49 additions & 48 deletions Hudl.Mjolnir.Tests/Command/CommandContextTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using Hudl.Config;
using Hudl.Mjolnir.Command;
using Hudl.Mjolnir.Command;
using Hudl.Mjolnir.Key;
using Hudl.Mjolnir.Tests.Helper;
using Xunit;
Expand Down Expand Up @@ -29,72 +28,74 @@ public void DefaultBulkheadSizeIs10()
{
// This is mainly to ensure we don't introduce a breaking change for clients
// who rely on the default configs.

var key = GroupKey.Named(Rand.String());
var context = new CommandContextImpl();

var bulkhead = context.GetBulkhead(key);
Assert.Equal(10, bulkhead.CountAvailable);
}

[Fact]
public void ReturnsNewBulkheadWhenConfigChanges()
{
// Config can be used to resize the bulkhead at runtime, which results in a new
// bulkhead being created.
// TODO rewrite this test now that config is mock-able and change handlers have changed
//[Fact]
//public void ReturnsNewBulkheadWhenConfigChanges()
//{
// // Config can be used to resize the bulkhead at runtime, which results in a new
// // bulkhead being created.

// To ensure consistency, callers who retrieve a bulkhead and call TryEnter()
// on it should keep a local reference to the same bulkhead to later call
// Release() on (rather than re-retrieving the bulkhead from the context).
// That behavior is tested elsewhere (with the BulkheadInvoker tests).
// // To ensure consistency, callers who retrieve a bulkhead and call TryEnter()
// // on it should keep a local reference to the same bulkhead to later call
// // Release() on (rather than re-retrieving the bulkhead from the context).
// // That behavior is tested elsewhere (with the BulkheadInvoker tests).

var key = Rand.String();
var groupKey = GroupKey.Named(key);
var configKey = "mjolnir.bulkhead." + key + ".maxConcurrent";
var context = new CommandContextImpl();
// var key = Rand.String();
// var groupKey = GroupKey.Named(key);
// var configKey = "mjolnir.bulkhead." + key + ".maxConcurrent";
// var context = new CommandContextImpl();

int initialExpectedCount = 5;
// int initialExpectedCount = 5;

ConfigProvider.Instance.Set(configKey, initialExpectedCount);
// ConfigProvider.Instance.Set(configKey, initialExpectedCount);

var bulkhead = context.GetBulkhead(groupKey);
// var bulkhead = context.GetBulkhead(groupKey);

Assert.Equal(5, initialExpectedCount);

// Now, change the config value and make sure it gets applied.
// Assert.Equal(5, initialExpectedCount);

int newExpectedCount = 2;
ConfigProvider.Instance.Set(configKey, newExpectedCount);
// // Now, change the config value and make sure it gets applied.

// Shouldn't change any existing referenced bulkheads...
Assert.Equal(initialExpectedCount, bulkhead.CountAvailable);
// int newExpectedCount = 2;
// ConfigProvider.Instance.Set(configKey, newExpectedCount);

// ...but newly-retrieved bulkheads should get a new instance
// with the updated count.
var newBulkhead = context.GetBulkhead(groupKey);
Assert.NotEqual(bulkhead, newBulkhead);
Assert.Equal(newExpectedCount, newBulkhead.CountAvailable);
}
// // Shouldn't change any existing referenced bulkheads...
// Assert.Equal(initialExpectedCount, bulkhead.CountAvailable);

[Fact]
public void IgnoresConfigChangeToInvalidValues()
{
var key = Rand.String();
var groupKey = GroupKey.Named(key);
var configKey = "mjolnir.bulkhead." + key + ".maxConcurrent";
var context = new CommandContextImpl();
// // ...but newly-retrieved bulkheads should get a new instance
// // with the updated count.
// var newBulkhead = context.GetBulkhead(groupKey);
// Assert.NotEqual(bulkhead, newBulkhead);
// Assert.Equal(newExpectedCount, newBulkhead.CountAvailable);
//}

// Should have a valid default value initially.
Assert.Equal(10, context.GetBulkhead(groupKey).CountAvailable);
// TODO rewrite this test now that config is mock-able and change handlers have changed
//[Fact]
//public void IgnoresConfigChangeToInvalidValues()
//{
// var key = Rand.String();
// var groupKey = GroupKey.Named(key);
// var configKey = "mjolnir.bulkhead." + key + ".maxConcurrent";
// var context = new CommandContextImpl();

// Negative limits aren't valid.
ConfigProvider.Instance.Set(configKey, -1);
Assert.Equal(10, context.GetBulkhead(groupKey).CountAvailable);
// // Should have a valid default value initially.
// Assert.Equal(10, context.GetBulkhead(groupKey).CountAvailable);

// Zero (disallow all) is a valid value.
ConfigProvider.Instance.Set(configKey, 0);
Assert.Equal(0, context.GetBulkhead(groupKey).CountAvailable);
}
// // Negative limits aren't valid.
// ConfigProvider.Instance.Set(configKey, -1);
// Assert.Equal(10, context.GetBulkhead(groupKey).CountAvailable);

// // Zero (disallow all) is a valid value.
// ConfigProvider.Instance.Set(configKey, 0);
// Assert.Equal(0, context.GetBulkhead(groupKey).CountAvailable);
//}
}
}
}

0 comments on commit 13320cc

Please sign in to comment.