diff --git a/README.md b/README.md index 8a8aa43..afa1c96 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ public bool CanAccess(IUser user) Wrap a `Use` block around the code's original behavior, and wrap `Try` around the new behavior. Invoking `Scientist.Science` will always return whatever the `Use` block returns, but it does a bunch of stuff behind the scenes: * It decides whether or not to run the `Try` block, -* Randomizes the order in which `Use` and `Try` blocks are run, +* Randomizes the order in which `Use` and `Try` blocks are run (Also see [Ensure control runs first](#control-first) option), * Measures the durations of all behaviors, * Compares the result of `Try` to the result of `Use`, * Swallows (but records) any exceptions raised in the `Try` block, and @@ -341,6 +341,18 @@ public bool CanAccess(IUser user) } ``` +### Ensuring Control is ran first {#control-first} + +Sometimes you've got to run the control first and then the candidate(s) and we get it! +```csharp +Scientist.Science("ExperimentN", experiment => +{ + experiment.EnsureControlRunsFirst(); + // ... +}); +``` +> The candidates are still ran in a randomised order, its just the control get ran first. + ## Alternatives Here are other implementations of Scientist available in different languages. diff --git a/src/Scientist/IExperiment.cs b/src/Scientist/IExperiment.cs index d4d037e..511a199 100644 --- a/src/Scientist/IExperiment.cs +++ b/src/Scientist/IExperiment.cs @@ -32,6 +32,11 @@ public interface IExperiment /// /// The delegate to handle exceptions thrown from an experiment. void Thrown(Action block); + + /// + /// Sets the control to run first and candidates after. Does not affect candidate randomisation. + /// + void EnsureControlRunsFirst(); } /// diff --git a/src/Scientist/Internals/Experiment.cs b/src/Scientist/Internals/Experiment.cs index bb7bd8d..e46d900 100644 --- a/src/Scientist/Internals/Experiment.cs +++ b/src/Scientist/Internals/Experiment.cs @@ -28,6 +28,8 @@ private static readonly Action _alwaysThrow private readonly Dictionary _contexts = new Dictionary(); private readonly IResultPublisher _resultPublisher; + private bool _ensureControlRunsFirst = false; + public Experiment(string name, Func> enabled, int concurrentTasks, IResultPublisher resultPublisher) { if (concurrentTasks <= 0) @@ -130,7 +132,8 @@ internal ExperimentInstance Build() => RunIf = _runIf, Thrown = _thrown, ThrowOnMismatches = ThrowOnMismatches, - ResultPublisher = _resultPublisher + ResultPublisher = _resultPublisher, + EnsureControlRunsFirst = _ensureControlRunsFirst, }); public void Compare(Func comparison) @@ -156,5 +159,9 @@ public void BeforeRun(Func action) { _beforeRun = action; } + + public void EnsureControlRunsFirst() { + _ensureControlRunsFirst = true; + } } } diff --git a/src/Scientist/Internals/ExperimentInstance.cs b/src/Scientist/Internals/ExperimentInstance.cs index 6e418bb..aa4799b 100644 --- a/src/Scientist/Internals/ExperimentInstance.cs +++ b/src/Scientist/Internals/ExperimentInstance.cs @@ -16,7 +16,8 @@ internal class ExperimentInstance internal readonly string Name; internal readonly int ConcurrentTasks; - internal readonly List Behaviors; + internal readonly NamedBehavior Control; + internal readonly List Candidates = new List(); internal readonly Func Cleaner; internal readonly Func Comparator; internal readonly Func BeforeRun; @@ -27,18 +28,15 @@ internal class ExperimentInstance internal readonly Action Thrown; internal readonly bool ThrowOnMismatches; internal readonly IResultPublisher ResultPublisher; - + internal readonly bool EnsureControlRunsFirst; + static Random _random = new Random(DateTimeOffset.UtcNow.Millisecond); - + public ExperimentInstance(ExperimentSettings settings) { Name = settings.Name; - - Behaviors = new List - { - new NamedBehavior(ControlExperimentName, settings.Control), - }; - Behaviors.AddRange( + Control = new NamedBehavior(ControlExperimentName, settings.Control); + Candidates.AddRange( settings.Candidates.Select(c => new NamedBehavior(c.Key, c.Value))); BeforeRun = settings.BeforeRun; @@ -52,6 +50,7 @@ public ExperimentInstance(ExperimentSettings settings) Thrown = settings.Thrown; ThrowOnMismatches = settings.ThrowOnMismatches; ResultPublisher = settings.ResultPublisher; + EnsureControlRunsFirst = settings.EnsureControlRunsFirst; } public async Task Run() @@ -60,7 +59,7 @@ public async Task Run() if (!await ShouldExperimentRun().ConfigureAwait(false)) { // Run the control behavior. - return await Behaviors[0].Behavior().ConfigureAwait(false); + return await Control.Behavior().ConfigureAwait(false); } if (BeforeRun != null) @@ -68,19 +67,27 @@ public async Task Run() await BeforeRun().ConfigureAwait(false); } - // Randomize ordering... - NamedBehavior[] orderedBehaviors; - lock (_random) + + var behaviors = new NamedBehavior[0]; + if (EnsureControlRunsFirst) { - orderedBehaviors = Behaviors.OrderBy(b => _random.Next()).ToArray(); + + behaviors = RandomiseBehavioursOrder(Candidates); + behaviors = new[] { Control }.Concat(behaviors).ToArray(); } + else + { + Candidates.Add(Control); + behaviors = RandomiseBehavioursOrder(Candidates); + } + // Break tasks into batches of "ConcurrentTasks" size var observations = new List>(); - foreach (var behaviors in orderedBehaviors.Chunk(ConcurrentTasks)) + foreach (var behaviorsChunk in behaviors.Chunk(ConcurrentTasks)) { // Run batch of behaviors simultaneously - var tasks = behaviors.Select(b => + var tasks = behaviorsChunk.Select(b => { return Observation.New( b.Name, @@ -95,7 +102,7 @@ public async Task Run() } var controlObservation = observations.FirstOrDefault(o => o.Name == ControlExperimentName); - + var result = new Result(this, observations, controlObservation, Contexts); try @@ -115,7 +122,15 @@ public async Task Run() if (controlObservation.Thrown) throw controlObservation.Exception; return controlObservation.Value; } - + + private NamedBehavior[] RandomiseBehavioursOrder(List behaviors) + { + lock (_random) + { + return behaviors.OrderBy(b => _random.Next()).ToArray(); + } + } + /// /// Does allow the experiment to run? /// @@ -149,7 +164,7 @@ public async Task IgnoreMismatchedObservation(Observation contr return false; } } - + /// /// Determine whether or not the experiment should run. /// @@ -157,9 +172,8 @@ async Task ShouldExperimentRun() { try { - // Only let the experiment run if at least one candidate (> 1 behaviors) is - // included. The control is always included behaviors count. - return Behaviors.Count > 1 && await Enabled().ConfigureAwait(false) && await RunIfAllows().ConfigureAwait(false); + // Only let the experiment run if at least one candidate (>= 1 behaviors) + return Candidates.Count >= 1 && await Enabled().ConfigureAwait(false) && await RunIfAllows().ConfigureAwait(false); } catch (Exception ex) { diff --git a/src/Scientist/Internals/ExperimentSettings.cs b/src/Scientist/Internals/ExperimentSettings.cs index a824fa8..c4ce941 100644 --- a/src/Scientist/Internals/ExperimentSettings.cs +++ b/src/Scientist/Internals/ExperimentSettings.cs @@ -26,5 +26,6 @@ internal class ExperimentSettings public bool ThrowOnMismatches { get; set; } public Action Thrown { get; set; } public IResultPublisher ResultPublisher { get; set; } + public bool EnsureControlRunsFirst { get; set; } } } diff --git a/test/Scientist.Test/ExperimentTests/ExperimentAsyncTests.cs b/test/Scientist.Test/ExperimentTests/ExperimentAsyncTests.cs new file mode 100644 index 0000000..5f55d39 --- /dev/null +++ b/test/Scientist.Test/ExperimentTests/ExperimentAsyncTests.cs @@ -0,0 +1,43 @@ +using FluentAssertions; +using GitHub; +using GitHub.Internals; +using NSubstitute; +using System.Linq; +using System.Threading.Tasks; +using UnitTests; +using Xunit; + + +public class ExperimentAsyncTests +{ + [Fact] + public async Task ExperimentAsync_EnsureControlRunsFirst_ShouldRunControlFirst() + { + var mock = Substitute.For>(); + mock.Control().Returns(Task.FromResult(42)); + mock.Candidate().Returns(Task.FromResult(42)); + const string experimentName = nameof(ExperimentAsync_EnsureControlRunsFirst_ShouldRunControlFirst); + + var resultPublisher = new InMemoryResultPublisher(); + var scientist = new Scientist(resultPublisher); + + var result = await scientist.ExperimentAsync(experimentName, experiment => + { + experiment.ThrowOnMismatches = true; + experiment.EnsureControlRunsFirst(); + experiment.Use(mock.Control); + experiment.Try("candidate", mock.Candidate); + }); + + result.Should().Be(42); + + Received.InOrder(() => + { + mock.Received().Control(); + mock.Received().Candidate(); + }); + + resultPublisher.Results(experimentName).First().Matched.Should().BeTrue(); + } +} + diff --git a/test/Scientist.Test/ExperimentTests/ExperimentTests.cs b/test/Scientist.Test/ExperimentTests/ExperimentTests.cs new file mode 100644 index 0000000..919324f --- /dev/null +++ b/test/Scientist.Test/ExperimentTests/ExperimentTests.cs @@ -0,0 +1,40 @@ +using GitHub; +using GitHub.Internals; +using NSubstitute; +using System.Linq; +using UnitTests; +using Xunit; +using FluentAssertions; + + +public class ExperimentTests +{ + [Fact] + public void Experiment_EnsureControlRunsFirst_ShouldRunControlFirst() + { + var mock = Substitute.For>(); + mock.Control().Returns(42); + mock.Candidate().Returns(42); + const string experimentName = nameof(Experiment_EnsureControlRunsFirst_ShouldRunControlFirst); + + var resultPublisher = new InMemoryResultPublisher(); + var scientist = new Scientist(resultPublisher); + + var result = scientist.Experiment(experimentName, experiment => + { + experiment.ThrowOnMismatches = true; + experiment.EnsureControlRunsFirst(); + experiment.Use(mock.Control); + experiment.Try("candidate", mock.Candidate); + }); + + result.Should().Be(42); + Received.InOrder(() => + { + mock.Received().Control(); + mock.Received().Candidate(); + }); + resultPublisher.Results(experimentName).First().Matched.Should().BeTrue(); + } +} + diff --git a/test/Scientist.Test/Scientist.Test.csproj b/test/Scientist.Test/Scientist.Test.csproj index 530c363..efa114f 100644 --- a/test/Scientist.Test/Scientist.Test.csproj +++ b/test/Scientist.Test/Scientist.Test.csproj @@ -21,8 +21,9 @@ PreserveNewest - - + + + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/test/Scientist.Test/packages.lock.json b/test/Scientist.Test/packages.lock.json index f4d397b..0497fb2 100644 --- a/test/Scientist.Test/packages.lock.json +++ b/test/Scientist.Test/packages.lock.json @@ -8,13 +8,22 @@ "resolved": "5.2.1", "contentHash": "wHARzQA695jwwKreOzNsq54KiGqKP38tv8hi8e2FXDEC/sA6BtrX90tVPDkOfVu13PbEzr00TCV8coikl+D1Iw==" }, + "FluentAssertions": { + "type": "Direct", + "requested": "[8.8.0, )", + "resolved": "8.8.0", + "contentHash": "m0kwcqBwvVel03FuMa7Ozo/oTaxYbjeNlcOhQFkyQpwX/8wks6RNl/Jnn58DCZVs6c2oG1RsCZw7HfKSaxLm3w==", + "dependencies": { + "System.Threading.Tasks.Extensions": "4.5.4" + } + }, "Microsoft.NET.Test.Sdk": { "type": "Direct", - "requested": "[17.14.1, )", - "resolved": "17.14.1", - "contentHash": "HJKqKOE+vshXra2aEHpi2TlxYX7Z9VFYkr+E5rwEvHC8eIXiyO+K9kNm8vmNom3e2rA56WqxU+/N9NJlLGXsJQ==", + "requested": "[18.0.1, )", + "resolved": "18.0.1", + "contentHash": "WNpu6vI2rA0pXY4r7NKxCN16XRWl5uHu6qjuyVLoDo6oYEggIQefrMjkRuibQHm/NslIUNCcKftvoWAN80MSAg==", "dependencies": { - "Microsoft.CodeCoverage": "17.14.1" + "Microsoft.CodeCoverage": "18.0.1" } }, "NSubstitute": { @@ -40,17 +49,17 @@ }, "xunit.runner.visualstudio": { "type": "Direct", - "requested": "[3.1.4, )", - "resolved": "3.1.4", - "contentHash": "5mj99LvCqrq3CNi06xYdyIAXOEh+5b33F2nErCzI5zWiDdLHXiPXEWFSUAF8zlIv0ZWqjZNCwHTQeAPYbF3pCg==", + "requested": "[3.1.5, )", + "resolved": "3.1.5", + "contentHash": "tKi7dSTwP4m5m9eXPM2Ime4Kn7xNf4x4zT9sdLO/G4hZVnQCRiMTWoSZqI/pYTVeI27oPPqHBKYI/DjJ9GsYgA==", "dependencies": { "Microsoft.TestPlatform.ObjectModel": "17.13.0" } }, "Microsoft.CodeCoverage": { "type": "Transitive", - "resolved": "17.14.1", - "contentHash": "pmTrhfFIoplzFVbhVwUquT+77CbGH+h4/3mBpdmIlYtBi9nAB+kKI6dN3A/nV4DFi3wLLx/BlHIPK+MkbQ6Tpg==" + "resolved": "18.0.1", + "contentHash": "O+utSr97NAJowIQT/OVp3Lh9QgW/wALVTP4RG1m2AfFP4IyJmJz0ZBmFJUsRQiAPgq6IRC0t8AAzsiPIsaUDEA==" }, "Microsoft.TestPlatform.ObjectModel": { "type": "Transitive", @@ -73,10 +82,18 @@ "System.Collections.Immutable": "1.5.0" } }, + "System.Runtime.CompilerServices.Unsafe": { + "type": "Transitive", + "resolved": "4.5.3", + "contentHash": "3TIsJhD1EiiT0w2CcDMN/iSSwnNnsrnbzeVHSKkaEgV85txMprmuO+Yq2AdSbeVGcg28pdNDTPK87tJhX7VFHw==" + }, "System.Threading.Tasks.Extensions": { "type": "Transitive", - "resolved": "4.3.0", - "contentHash": "npvJkVKl5rKXrtl1Kkm6OhOUaYGEiF9wFbppFRWSMoApKzt2PiPHT2Bb8a5sAWxprvdOAtvaARS9QYMznEUtug==" + "resolved": "4.5.4", + "contentHash": "zteT+G8xuGu6mS+mzDzYXbzS7rd3K6Fjb9RiZlYlJPam2/hU7JCBZBVEcywNuR+oZ1ncTvc/cq0faRr3P01OVg==", + "dependencies": { + "System.Runtime.CompilerServices.Unsafe": "4.5.3" + } }, "xunit.abstractions": { "type": "Transitive", @@ -132,14 +149,20 @@ "System.Diagnostics.EventLog": "6.0.0" } }, + "FluentAssertions": { + "type": "Direct", + "requested": "[8.8.0, )", + "resolved": "8.8.0", + "contentHash": "m0kwcqBwvVel03FuMa7Ozo/oTaxYbjeNlcOhQFkyQpwX/8wks6RNl/Jnn58DCZVs6c2oG1RsCZw7HfKSaxLm3w==" + }, "Microsoft.NET.Test.Sdk": { "type": "Direct", - "requested": "[17.14.1, )", - "resolved": "17.14.1", - "contentHash": "HJKqKOE+vshXra2aEHpi2TlxYX7Z9VFYkr+E5rwEvHC8eIXiyO+K9kNm8vmNom3e2rA56WqxU+/N9NJlLGXsJQ==", + "requested": "[18.0.1, )", + "resolved": "18.0.1", + "contentHash": "WNpu6vI2rA0pXY4r7NKxCN16XRWl5uHu6qjuyVLoDo6oYEggIQefrMjkRuibQHm/NslIUNCcKftvoWAN80MSAg==", "dependencies": { - "Microsoft.CodeCoverage": "17.14.1", - "Microsoft.TestPlatform.TestHost": "17.14.1" + "Microsoft.CodeCoverage": "18.0.1", + "Microsoft.TestPlatform.TestHost": "18.0.1" } }, "NSubstitute": { @@ -164,29 +187,29 @@ }, "xunit.runner.visualstudio": { "type": "Direct", - "requested": "[3.1.4, )", - "resolved": "3.1.4", - "contentHash": "5mj99LvCqrq3CNi06xYdyIAXOEh+5b33F2nErCzI5zWiDdLHXiPXEWFSUAF8zlIv0ZWqjZNCwHTQeAPYbF3pCg==" + "requested": "[3.1.5, )", + "resolved": "3.1.5", + "contentHash": "tKi7dSTwP4m5m9eXPM2Ime4Kn7xNf4x4zT9sdLO/G4hZVnQCRiMTWoSZqI/pYTVeI27oPPqHBKYI/DjJ9GsYgA==" }, "Microsoft.CodeCoverage": { "type": "Transitive", - "resolved": "17.14.1", - "contentHash": "pmTrhfFIoplzFVbhVwUquT+77CbGH+h4/3mBpdmIlYtBi9nAB+kKI6dN3A/nV4DFi3wLLx/BlHIPK+MkbQ6Tpg==" + "resolved": "18.0.1", + "contentHash": "O+utSr97NAJowIQT/OVp3Lh9QgW/wALVTP4RG1m2AfFP4IyJmJz0ZBmFJUsRQiAPgq6IRC0t8AAzsiPIsaUDEA==" }, "Microsoft.TestPlatform.ObjectModel": { "type": "Transitive", - "resolved": "17.14.1", - "contentHash": "xTP1W6Mi6SWmuxd3a+jj9G9UoC850WGwZUps1Wah9r1ZxgXhdJfj1QqDLJkFjHDCvN42qDL2Ps5KjQYWUU0zcQ==", + "resolved": "18.0.1", + "contentHash": "qT/mwMcLF9BieRkzOBPL2qCopl8hQu6A1P7JWAoj/FMu5i9vds/7cjbJ/LLtaiwWevWLAeD5v5wjQJ/l6jvhWQ==", "dependencies": { "System.Reflection.Metadata": "8.0.0" } }, "Microsoft.TestPlatform.TestHost": { "type": "Transitive", - "resolved": "17.14.1", - "contentHash": "d78LPzGKkJwsJXAQwsbJJ7LE7D1wB+rAyhHHAaODF+RDSQ0NgMjDFkSA1Djw18VrxO76GlKAjRUhl+H8NL8Z+Q==", + "resolved": "18.0.1", + "contentHash": "uDJKAEjFTaa2wHdWlfo6ektyoh+WD4/Eesrwb4FpBFKsLGehhACVnwwTI4qD3FrIlIEPlxdXg3SyrYRIcO+RRQ==", "dependencies": { - "Microsoft.TestPlatform.ObjectModel": "17.14.1", + "Microsoft.TestPlatform.ObjectModel": "18.0.1", "Newtonsoft.Json": "13.0.3" } },