From 0abda2d8554133276b220640c45f324b78791b76 Mon Sep 17 00:00:00 2001 From: Chris Hannon Date: Sun, 15 Dec 2024 17:21:17 -0700 Subject: [PATCH] Fixed JSON tests for existing and sticky bucket --- .../GrowthBookTests/StickyBucketTests.cs | 40 ++++++++++++++++++- GrowthBook/GrowthBook.cs | 34 +++++++++------- GrowthBook/Services/IStickyBucketService.cs | 2 +- .../Services/InMemoryStickyBucketService.cs | 6 +-- GrowthBook/StickyAssignmentsDocument.cs | 4 +- GrowthBook/Utilities/ExperimentUtilities.cs | 13 +++--- 6 files changed, 69 insertions(+), 30 deletions(-) diff --git a/GrowthBook.Tests/StandardTests/GrowthBookTests/StickyBucketTests.cs b/GrowthBook.Tests/StandardTests/GrowthBookTests/StickyBucketTests.cs index 7b5dc00..56fc0d3 100644 --- a/GrowthBook.Tests/StandardTests/GrowthBookTests/StickyBucketTests.cs +++ b/GrowthBook.Tests/StandardTests/GrowthBookTests/StickyBucketTests.cs @@ -3,6 +3,9 @@ using System.Linq; using System.Text; using System.Threading.Tasks; +using FluentAssertions; +using GrowthBook.Services; +using Newtonsoft.Json; using Newtonsoft.Json.Linq; using Xunit; @@ -24,14 +27,47 @@ public class StickyBucketTestCase [TestPropertyIndex(4)] public JToken ExpectedResult { get; set; } [TestPropertyIndex(5)] - public StickyAssignmentsDocument[] ExpectedAssignmentDocs { get; set; } = []; + public Dictionary ExpectedAssignmentDocs { get; set; } = []; } [Theory] [MemberData(nameof(GetMappedTestsInCategory), typeof(StickyBucketTestCase))] public void Run(StickyBucketTestCase testCase) { + var service = new InMemoryStickyBucketService(); + + testCase.Context.StickyBucketService = service; + testCase.Context.StickyBucketAssignmentDocs = testCase.PreExistingAssignmentDocs.ToDictionary(x => x.FormattedAttribute); + + // NOTE: Existing sticky bucket JSON tests in the JS SDK load this into the service up front + // but I wonder if that's correct because without that any assignment doc that exists + // other than those will not be stored and some of these test cases will fail. + + foreach (var document in testCase.PreExistingAssignmentDocs) + { + service.SaveAssignments(document); + } + var gb = new GrowthBook(testCase.Context); -#warning Complete this. + + var result = gb.EvalFeature(testCase.FeatureName); + + var actualResult = JToken.Parse(JsonConvert.SerializeObject(result.ExperimentResult)); + + if (testCase.ExpectedResult is JObject obj) + { + foreach (var property in obj.Properties()) + { + actualResult[property.Name].ToString().Should().Be(property.Value.ToString()); + } + } + else + { + actualResult.ToString().Should().Be(testCase.ExpectedResult.ToString()); + } + + var storedDocuments = service.GetAllAssignments(testCase.ExpectedAssignmentDocs.Keys); + + storedDocuments.Should().BeEquivalentTo(testCase.ExpectedAssignmentDocs, "because those should have been stored correctly"); } } diff --git a/GrowthBook/GrowthBook.cs b/GrowthBook/GrowthBook.cs index 75ca6b9..6ffc7c2 100644 --- a/GrowthBook/GrowthBook.cs +++ b/GrowthBook/GrowthBook.cs @@ -486,7 +486,7 @@ private ExperimentResult RunExperiment(Experiment experiment, string featureId) if (hasFallback) { - (_, hashValue) = Attributes.GetHashAttributeAndValue(experiment.FallbackAttribute); + (hashAttribute, hashValue) = Attributes.GetHashAttributeAndValue(experiment.FallbackAttribute); } else { @@ -548,24 +548,24 @@ private ExperimentResult RunExperiment(Experiment experiment, string featureId) _logger.LogDebug("Aborting experiment, associated conditions have prohibited participation"); return GetExperimentResult(experiment, featureId: featureId); } + } - if (experiment.ParentConditions != null) + if (experiment.ParentConditions != null) + { + foreach (var parentCondition in experiment.ParentConditions) { - foreach (var parentCondition in experiment.ParentConditions) - { - var parentResult = EvaluateFeature(featureId); + var parentResult = EvaluateFeature(parentCondition.Id); - if (parentResult.Source == FeatureResult.SourceId.CyclicPrerequisite) - { - return GetExperimentResult(experiment, featureId: featureId); - } + if (parentResult.Source == FeatureResult.SourceId.CyclicPrerequisite) + { + return GetExperimentResult(experiment, featureId: featureId); + } - var evaluationObject = new JObject { ["value"] = parentResult.Value }; + var evaluationObject = new JObject { ["value"] = parentResult.Value }; - if (!_conditionEvaluator.EvalCondition(evaluationObject, parentCondition.Condition ?? new JObject(), _savedGroups)) - { - return GetExperimentResult(experiment, featureId: featureId); - } + if (!_conditionEvaluator.EvalCondition(evaluationObject, parentCondition.Condition ?? new JObject(), _savedGroups)) + { + return GetExperimentResult(experiment, featureId: featureId); } } } @@ -753,7 +753,11 @@ private ExperimentResult GetExperimentResult(Experiment experiment, int variatio HashUsed = hashUsed, HashValue = hashValue, Value = experiment.Variations is null ? null : experiment.Variations[variationIndex], - VariationId = variationIndex + VariationId = variationIndex, + Name = meta?.Name, + Passthrough = meta?.Passthrough ?? false, + Bucket = bucketHash ?? 0d, + StickyBucketUsed = wasStickyBucketUsed }; result.Name = meta?.Name; diff --git a/GrowthBook/Services/IStickyBucketService.cs b/GrowthBook/Services/IStickyBucketService.cs index 99c3284..d3e3a7f 100644 --- a/GrowthBook/Services/IStickyBucketService.cs +++ b/GrowthBook/Services/IStickyBucketService.cs @@ -8,6 +8,6 @@ public interface IStickyBucketService { StickyAssignmentsDocument GetAssignments(string attributeName, string attributeValue); void SaveAssignments(StickyAssignmentsDocument document); - IDictionary GetAllAssignments(IDictionary attributes); + IDictionary GetAllAssignments(IEnumerable attributes); } } diff --git a/GrowthBook/Services/InMemoryStickyBucketService.cs b/GrowthBook/Services/InMemoryStickyBucketService.cs index aa4402d..aa199f1 100644 --- a/GrowthBook/Services/InMemoryStickyBucketService.cs +++ b/GrowthBook/Services/InMemoryStickyBucketService.cs @@ -9,10 +9,10 @@ public class InMemoryStickyBucketService : IStickyBucketService { private readonly IDictionary _cachedDocuments = new Dictionary(); - public IDictionary GetAllAssignments(IDictionary attributes) + public IDictionary GetAllAssignments(IEnumerable attributes) { - var assignments = from pair in attributes - let existingDoc = _cachedDocuments.TryGetValue(pair.Key, out var doc) ? doc : null + var assignments = from name in attributes + let existingDoc = _cachedDocuments.TryGetValue(name, out var doc) ? doc : null where existingDoc != null select (Attribute: existingDoc.FormattedAttribute, Document: existingDoc); diff --git a/GrowthBook/StickyAssignmentsDocument.cs b/GrowthBook/StickyAssignmentsDocument.cs index 036e155..adb9397 100644 --- a/GrowthBook/StickyAssignmentsDocument.cs +++ b/GrowthBook/StickyAssignmentsDocument.cs @@ -12,7 +12,7 @@ public class StickyAssignmentsDocument public string AttributeName { get; set; } public string AttributeValue { get; set; } - public IDictionary StickyAssignments { get; set; } + public IDictionary Assignments { get; set; } = new Dictionary(); public StickyAssignmentsDocument() { } @@ -20,7 +20,7 @@ public StickyAssignmentsDocument(string attributeName, string attributeValue, ID { AttributeName = attributeName; AttributeValue = attributeValue; - StickyAssignments = stickyAssignments ?? new Dictionary(); + Assignments = stickyAssignments ?? new Dictionary(); } } } diff --git a/GrowthBook/Utilities/ExperimentUtilities.cs b/GrowthBook/Utilities/ExperimentUtilities.cs index b7659c8..a678395 100644 --- a/GrowthBook/Utilities/ExperimentUtilities.cs +++ b/GrowthBook/Utilities/ExperimentUtilities.cs @@ -253,15 +253,14 @@ private static Regex GetUrlRegex(UrlPattern pattern) public static (StickyAssignmentsDocument Document, bool IsChanged) GenerateStickyBucketAssignment(IStickyBucketService stickyBucketService, string attributeName, string attributeValue, IDictionary assignments) { var existingDocument = stickyBucketService is null ? new StickyAssignmentsDocument(attributeName, attributeValue) : stickyBucketService.GetAssignments(attributeName, attributeValue); - var newAssignments = new Dictionary(existingDocument.StickyAssignments); + var newAssignments = new Dictionary(existingDocument?.Assignments ?? new Dictionary()); newAssignments.MergeWith(new[] { assignments }); var isChanged = JsonConvert.SerializeObject(existingDocument) != JsonConvert.SerializeObject(newAssignments); + var document = new StickyAssignmentsDocument(attributeName, attributeValue, newAssignments); - existingDocument.StickyAssignments = newAssignments; - - return (existingDocument, isChanged); + return (document, isChanged); } public static StickyBucketVariation GetStickyBucketVariation(Experiment experiment, int bucketVersion, int minBucketVersion, IList meta, JObject attributes, IDictionary document) @@ -304,7 +303,7 @@ private static IDictionary GetStickyBucketAssignments(JObject at (var hashAttributeWithoutFallback, var hashValueWithoutFallback) = attributes.GetHashAttributeAndValue(hashAttribute, default); var hashKey = new StickyAssignmentsDocument(hashAttributeWithoutFallback, hashValueWithoutFallback); - (var hashAttributeWithFallback, var hashValueWithFallback) = attributes.GetHashAttributeAndValue(default, fallbackAttribute); + (var hashAttributeWithFallback, var hashValueWithFallback) = attributes.GetHashAttributeAndValue(fallbackAttribute, default); var fallbackKey = new StickyAssignmentsDocument(hashAttributeWithFallback, hashValueWithFallback); var pendingAssignments = new List>(); @@ -313,12 +312,12 @@ private static IDictionary GetStickyBucketAssignments(JObject at if (fallbackKey.HasValue && stickyAssignmentDocs.TryGetValue(fallbackKey.FormattedAttribute, out var fallbackDocument)) { - pendingAssignments.Add(fallbackDocument.StickyAssignments); + pendingAssignments.Add(fallbackDocument.Assignments); } if (stickyAssignmentDocs.TryGetValue(hashKey.FormattedAttribute, out var document)) { - pendingAssignments.Add(document.StickyAssignments); + pendingAssignments.Add(document.Assignments); } return mergedAssignments.MergeWith(pendingAssignments);