From 11eca6f78c006ad1da5c338b81d680eb85b7124c Mon Sep 17 00:00:00 2001 From: Andrew Bell <115623869+andybharness@users.noreply.github.com> Date: Tue, 29 Nov 2022 10:53:03 +0000 Subject: [PATCH] [FFM-5307] - .NET SDK - Fix for prereq identifier + update JSON parser to use latest tests (#47) What When a feature depends on another prereq feature being true, the evaluation fails if the value and identifier are different values in the prereq. Update code to use latest ff-test-cases JSON format. Some additional fixes were needed including Null reference exception and incorrect OR handling in groups when latest JSON was used. Why The checkPreRequisite() method incorrectly uses the value of the prereq instead of the identifier. Testing Tested locally with new ff-test-cases JSON + sanity checks with example program --- client/api/Evaluator.cs | 13 +- tests/ff-server-sdk-test/EvaluatorTest.cs | 159 ++++++++++++++---- tests/ff-server-sdk-test/TestModel.cs | 29 +++- .../ff-server-sdk-test.csproj | 25 +-- tests/ff-server-sdk-test/ff-test-cases | 2 +- 5 files changed, 159 insertions(+), 69 deletions(-) diff --git a/client/api/Evaluator.cs b/client/api/Evaluator.cs index 39c5758..e28734d 100644 --- a/client/api/Evaluator.cs +++ b/client/api/Evaluator.cs @@ -86,9 +86,10 @@ public string StringVariation(string key, dto.Target target, string defaultValue private bool checkPreRequisite(FeatureConfig parentFeatureConfig, dto.Target target) { bool result = true; - List prerequisites = parentFeatureConfig.Prerequisites.ToList(); - if ( prerequisites != null && prerequisites.Count > 0) + if (parentFeatureConfig.Prerequisites != null && parentFeatureConfig.Prerequisites.Count > 0) { + List prerequisites = parentFeatureConfig.Prerequisites.ToList(); + foreach (Prerequisite pqs in prerequisites) { FeatureConfig preReqFeatureConfig = this.repository.GetFlag(pqs.Feature); @@ -105,7 +106,7 @@ private bool checkPreRequisite(FeatureConfig parentFeatureConfig, dto.Target tar } List validPreReqVariations = pqs.Variations.ToList(); - if (!validPreReqVariations.Contains(preReqEvaluatedVariation.Value)) + if (!validPreReqVariations.Contains(preReqEvaluatedVariation.Identifier)) { return false; } @@ -227,11 +228,11 @@ private bool IsTargetIncludedOrExcludedInSegment(List segmentList, dto.T return true; } - // if we have rules, all should pass + // if we have rules, at least one should pass if (segment.Rules != null) { - Clause firstFailure = segment.Rules.FirstOrDefault(r => EvaluateClause(r, target) == false); - return firstFailure == null; + Clause firstSuccess = segment.Rules.FirstOrDefault(r => EvaluateClause(r, target)); + return firstSuccess != null; } } } diff --git a/tests/ff-server-sdk-test/EvaluatorTest.cs b/tests/ff-server-sdk-test/EvaluatorTest.cs index c233388..d6344c0 100644 --- a/tests/ff-server-sdk-test/EvaluatorTest.cs +++ b/tests/ff-server-sdk-test/EvaluatorTest.cs @@ -1,29 +1,47 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; -using System.Linq; using io.harness.cfsdk.client.api; using io.harness.cfsdk.client.cache; -using io.harness.cfsdk.client.dto; using io.harness.cfsdk.HarnessOpenAPIService; using Newtonsoft.Json; using Newtonsoft.Json.Linq; using NUnit.Framework; +[SetUpFixture] +public class SetupTracing +{ + [OneTimeSetUp] + public void StartTest() + { + Trace.Listeners.Add(new ConsoleTraceListener()); + } + + [OneTimeTearDown] + public void EndTest() + { + Trace.Flush(); + } +} + namespace ff_server_sdk_test { public class EvaluatorListener : IEvaluatorCallback { - public void evaluationProcessed(FeatureConfig featureConfig, io.harness.cfsdk.client.dto.Target target, Variation variation) + public void evaluationProcessed(FeatureConfig featureConfig, io.harness.cfsdk.client.dto.Target target, + Variation variation) { var targetName = target != null ? target.Name : "_no_target"; Serilog.Log.Information($"processEvaluation {featureConfig.Feature}, {targetName}, {variation.Value} "); } } + + + [TestFixture] public class EvaluatorTest { - private static List testData = new List(); private static IRepository repository; private static ICache cache; @@ -37,73 +55,140 @@ static EvaluatorTest() cache = new FeatureSegmentCache(); repository = new StorageRepository(cache, null, null); evaluator = new Evaluator(repository, listener); + } + + private static void LoadSegments(List segments) + { + if (segments != null) + { + segments.ForEach(segment => { repository.SetSegment(segment.Identifier, segment); }); + } + } - Assert.DoesNotThrow(() => + private static void LoadFlags(List flags) + { + if (flags != null) { - foreach (string fileName in Directory.GetFiles("./ff-test-cases/tests", "*.json")) + flags.ForEach(flag => { repository.SetFlag(flag.Feature, flag); }); + } + } + + + private static FeatureConfig FindFeatureConfig(string flagName, List flags) + { + foreach (FeatureConfig nextFlag in flags) + { + if (nextFlag.Feature.Equals(flagName)) { - var testModel = JsonConvert.DeserializeObject(File.ReadAllText(fileName)); - Assert.NotNull(testModel); + return nextFlag; + } + } + Assert.Fail("Could not find feature flag " + flagName); + return null; + } - string name = Path.GetFileName(fileName); - string feature = testModel.flag.Feature + name; - testModel.flag.Feature = feature; - testModel.testFile = name; - testData.Add(testModel); + private static List GetTree(string path, string searchPattern) + { + List found = new List(); + foreach (string file in Directory.GetFiles(path, searchPattern)) + { + found.Add(file); + } - repository.SetFlag(testModel.flag.Feature, testModel.flag); - if (testModel.segments != null) - { - testModel.segments.ForEach(s => - { - repository.SetSegment(s.Identifier, s); - }); - } - } - }); + foreach (string dir in Directory.GetDirectories(path, "*", SearchOption.TopDirectoryOnly)) + { + List subFound = GetTree(dir, searchPattern); + found.AddRange(subFound); + } + + return found; } private static IEnumerable GenerateTestCases() { + string baseTestPath = Path.GetFullPath("./ff-test-cases/tests/"); - foreach (var test in testData) + foreach (string fileName in GetTree(baseTestPath, "*")) { - foreach (var item in test.expected) + Console.WriteLine("Processing " + fileName); + + var testModel = JsonConvert.DeserializeObject(File.ReadAllText(fileName), + new JsonSerializerSettings + { + ContractResolver = new LenientContractResolver() + }); + + Assert.NotNull(testModel); + + foreach (Dictionary nextTest in testModel.tests) { - yield return new TestCaseData(test.flag.Feature, item.Key, item.Value, test); + object expected = nextTest.GetValueOrDefault("expected"); + string target = (string)nextTest.GetValueOrDefault("target", null); // May be null + string flag = (string)nextTest.GetValueOrDefault("flag"); + FeatureConfig feature = FindFeatureConfig(flag, testModel.flags); + + LoadSegments(testModel.segments); + LoadFlags(testModel.flags); + + string nUnitTestName = fileName.Replace(baseTestPath, "ff-test-cases ").Replace(".json", ""); + + nUnitTestName += "__with_flag_" + flag; + if (target != null) + { + nUnitTestName += "__with_target_" + target; + } + + yield return new TestCaseData(nUnitTestName, target, expected, flag, feature.Kind, testModel); } } } [Test, Category("Evaluation Testing"), TestCaseSource("GenerateTestCases")] - public void ExecuteTestCases(string f, string identifier, bool result, TestModel test) + public void ExecuteTestCases(string testName, string targetIdentifier, object expected, string featureFlag, FeatureConfigKind kind, TestModel testModel) { io.harness.cfsdk.client.dto.Target target = null; - if (!identifier.Equals(noTarget)) + if (!noTarget.Equals(targetIdentifier)) { - if (test.targets != null) + if (testModel.targets != null) { - target = test.targets.Find(t => { return t.Identifier == identifier; }); + target = testModel.targets.Find(t => { return t.Identifier == targetIdentifier; }); } } - string feature = test.flag.Feature; - switch (test.flag.Kind) + + object got = null; + + switch (kind) { case FeatureConfigKind.Boolean: - bool res = evaluator.BoolVariation(feature, target, false); - Assert.AreEqual(res, result, $"Expected result for {feature} was {result}"); + got = evaluator.BoolVariation(featureFlag, target, false); break; case FeatureConfigKind.Int: - double resInt = evaluator.NumberVariation(feature, target, 0); + got = evaluator.NumberVariation(featureFlag, target, 0); break; case FeatureConfigKind.String: - string resStr = evaluator.StringVariation(feature, target, ""); + got = evaluator.StringVariation(featureFlag, target, ""); break; case FeatureConfigKind.Json: - JObject resObj = evaluator.JsonVariation(feature, target, JObject.Parse("{val: 'default value'}")); + got = evaluator.JsonVariation(featureFlag, target, JObject.Parse("{val: 'default value'}")); break; } + + Debug.Print(" TEST : {0}", testName); + Debug.Print(" FLAG : {0}", featureFlag); + Debug.Print(" TARGET : {0} ", targetIdentifier ?? "(none)"); + Debug.Print("EXPECTED : {0} ({1})", expected, expected.GetType().Name); + Debug.Print(" GOT : {0} ({1})", got.ToString().Replace("\n", ""), got.GetType().Name); + + if (kind == FeatureConfigKind.Json) + { + var expectedJson = JObject.Parse((string)expected); + Assert.AreEqual(expectedJson, got, $"Expected result for {featureFlag} was {expected}"); + } + else + { + Assert.AreEqual(expected, got, $"Expected result for {featureFlag} was {expected}"); + } } } diff --git a/tests/ff-server-sdk-test/TestModel.cs b/tests/ff-server-sdk-test/TestModel.cs index 7e94241..fd36490 100644 --- a/tests/ff-server-sdk-test/TestModel.cs +++ b/tests/ff-server-sdk-test/TestModel.cs @@ -1,18 +1,43 @@ using System; using System.Collections.Generic; +using System.Reflection; using io.harness.cfsdk.HarnessOpenAPIService; +using Newtonsoft.Json; +using Newtonsoft.Json.Serialization; namespace ff_server_sdk_test { + public class LenientContractResolver : DefaultContractResolver + { + protected override JsonProperty CreateProperty(System.Reflection.MemberInfo member, MemberSerialization memberSerialization) + { + var property = base.CreateProperty(member, memberSerialization); + property.Required = Required.Default; + return property; + } + + protected override JsonObjectContract CreateObjectContract(Type objectType) + { + var contract = base.CreateObjectContract(objectType); + contract.ItemRequired = Required.Default; + return contract; + } + protected override JsonProperty CreatePropertyFromConstructorParameter(JsonProperty matchingMemberProperty, ParameterInfo parameterInfo) + { + var property = base.CreatePropertyFromConstructorParameter(matchingMemberProperty, parameterInfo); + property.Required = Required.Default; + return property; + } + } public class TestModel { public string testFile; - public FeatureConfig flag; + public List flags; public List targets; public List segments; - public Dictionary expected; + public List> tests; public TestModel() { } diff --git a/tests/ff-server-sdk-test/ff-server-sdk-test.csproj b/tests/ff-server-sdk-test/ff-server-sdk-test.csproj index 9c2abbd..1548879 100644 --- a/tests/ff-server-sdk-test/ff-server-sdk-test.csproj +++ b/tests/ff-server-sdk-test/ff-server-sdk-test.csproj @@ -25,30 +25,9 @@ - + PreserveNewest - - - PreserveNewest - - - PreserveNewest - - - PreserveNewest - - - PreserveNewest - - - PreserveNewest - - - PreserveNewest - - - PreserveNewest - + diff --git a/tests/ff-server-sdk-test/ff-test-cases b/tests/ff-server-sdk-test/ff-test-cases index 561740a..beed9ac 160000 --- a/tests/ff-server-sdk-test/ff-test-cases +++ b/tests/ff-server-sdk-test/ff-test-cases @@ -1 +1 @@ -Subproject commit 561740a9c62f84cf87a58f4a69e26e5f38ed64b6 +Subproject commit beed9acd839fb7324caea74e9d19f2154329d7a9