Added tests to #242

tfsaggregator · Aug 24, 2021 · 09c6e41 · 09c6e41
1 parent 1f43b80
commit 09c6e41
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 15 deletions.
diff --git a/Next-Release-ChangeLog.md b/Next-Release-ChangeLog.md
@@ -1,9 +1,9 @@
-This release fixes a number of issues and introduce an additional CLI command.
+This release TODO.
 
 
 CLI Commands and Options
 ========================
-- Fixes spurious upgrade message noted in #225.
+No changes.
 
 
 Docker and Azure Function Hosting
@@ -13,20 +13,17 @@ No changes.
 
 Rule Language
 ========================
-- Added support for `.bypassrules` directive (#83, #228).
-- Fixes #231 (Directive check revision false missing after `update.rule`).
+No changes.
 
 
 Rule Interpreter Engine
 ========================
-- Fixes #229 (Updating a work item field with impersonation enabled fails with the message: _Remove requires Value to be null_).
-- Fixes #234 (_Object reference not set_ when removing a work item link).
+Improved performance and memory profile through caching compiled rules (#242).
 
 
 Build, Test, Documentation
 ========================
-- SonarCloud now requires Java 11.
-
+No changes.
 
 File Hashes
 ------------------------

diff --git a/src/aggregator-ruleng/ScriptedRuleWrapper.cs b/src/aggregator-ruleng/ScriptedRuleWrapper.cs
@@ -37,7 +37,7 @@ public class ScriptedRuleWrapper : IRule
 
         private ScriptedRuleWrapper(string ruleName, IAggregatorLogger logger)
         {
-            _logger = logger;
+            _logger = logger ?? new NullLogger();
             Name = ruleName;
         }
 
@@ -46,7 +46,7 @@ private ScriptedRuleWrapper(string ruleName, IAggregatorLogger logger)
         /// </summary>
         /// <param name="ruleName"></param>
         /// <param name="ruleCode"></param>
-        internal ScriptedRuleWrapper(string ruleName, string[] ruleCode) : this(ruleName, new NullLogger())
+        internal ScriptedRuleWrapper(string ruleName, string[] ruleCode, IAggregatorLogger logger = null) : this(ruleName, logger)
         {
             (IPreprocessedRule preprocessedRule, bool parseSuccess) = RuleFileParser.Read(ruleCode);
             _ruleFileParseSuccess = parseSuccess;
@@ -85,7 +85,11 @@ private void Initialize(IPreprocessedRule preprocessedRule)
 
             if (RuleDirectives.IsCSharp())
             {
-                string ruleKey = string.Join("/n", RuleFileParser.Write(RuleDirectives));
+                string ruleKey = string.Join('\n', RuleFileParser.Write(RuleDirectives));
+#if DEBUG
+                bool cached = _scriptCache.ContainsKey(ruleKey);                
+                _logger.WriteVerbose(cached ? $"Rule {Name} found in cache": $"Rule {Name} was not in cache: compiling");
+#endif
                 _roslynScript = _scriptCache.GetOrAdd(ruleKey, CreateRoslynScript, RuleDirectives);
             }
             else

diff --git a/src/unittests-ruleng/RuleCachingTests.cs b/src/unittests-ruleng/RuleCachingTests.cs
@@ -0,0 +1,51 @@
+using aggregator;
+using aggregator.Engine;
+using NSubstitute;
+using Xunit;
+using Xunit.Priority;
+
+namespace unittests_ruleng
+{
+    [TestCaseOrderer(PriorityOrderer.Name, PriorityOrderer.Assembly)]
+    public class RuleCachingTests
+    {
+#if DEBUG
+        [Fact, Priority(0)]
+        public void GivenTheSameRule_WhenTheRuleIsParsedThrice_ThenTheLogger_LogsThatWasFoundInCache()
+        {
+            //Given
+            var logger = Substitute.For<IAggregatorLogger>();
+            string ruleName = "dummy";
+            var ruleCode = new[] { "" };
+
+            //When
+            _ = new ScriptedRuleWrapper(ruleName, ruleCode, logger);
+            _ = new ScriptedRuleWrapper(ruleName, ruleCode, logger);
+            _ = new ScriptedRuleWrapper(ruleName, ruleCode, logger);
+
+            //Then
+            logger.Received(1).WriteVerbose("Rule dummy was not in cache: compiling");
+            logger.Received(2).WriteVerbose("Rule dummy found in cache");
+        }
+
+        [Fact, Priority(1)]
+        public void GivenARule_WhenTheRuleChanges_ThenTheLogger_LogsThatWasNotFoundInCache()
+        {
+            //Given
+            var logger = Substitute.For<IAggregatorLogger>();
+            string ruleName = "dummy";
+            var ruleCode1 = new[] { "/* v1 */" };
+            var ruleCode2 = new[] { "/* v2 */" };
+
+            //When
+            _ = new ScriptedRuleWrapper(ruleName, ruleCode1, logger);
+            _ = new ScriptedRuleWrapper(ruleName, ruleCode2, logger);
+            _ = new ScriptedRuleWrapper(ruleName, ruleCode2, logger);
+
+            //Then
+            logger.Received(2).WriteVerbose("Rule dummy was not in cache: compiling");
+            logger.Received(1).WriteVerbose("Rule dummy found in cache");
+        }
+#endif
+    }
+}
diff --git a/src/unittests-ruleng/ScriptedRuleWrapperTests.cs b/src/unittests-ruleng/ScriptedRuleWrapperTests.cs
@@ -6,12 +6,11 @@ namespace unittests_ruleng
     public class ScriptedRuleWrapperTests
     {
         [Fact]
-        public void GivenAnValidRule_WhenTheRuleIsVerified_ThenTheResult_ShouldBeSuccessfull()
+        public void GivenAValidRule_WhenTheRuleIsVerified_ThenTheResult_ShouldBeSuccessfull()
         {
             //Given
             var rule = new ScriptedRuleWrapper("dummy", new[] { "" });
 
-
             //When
             var (success, diagnostics) = rule.Verify();
 
@@ -26,7 +25,6 @@ public void GivenAnNotCompilableRule_WhenTheRuleIsVerified_ThenTheResult_ShouldN
             //Given
             var rule = new ScriptedRuleWrapper("dummy", new[] { "(" });
 
-
             //When
             var (success, diagnostics) = rule.Verify();
 
@@ -41,7 +39,6 @@ public void GivenAnNotParsableRule_WhenTheRuleIsVerified_ThenTheResult_ShouldNot
             //Given
             var rule = new ScriptedRuleWrapper("dummy", new[] { ".invalid=directive" });
 
-
             //When
             var (success, diagnostics) = rule.Verify();
 

diff --git a/src/unittests-ruleng/unittests-ruleng.csproj b/src/unittests-ruleng/unittests-ruleng.csproj
@@ -38,6 +38,7 @@
     <PackageReference Include="NSubstitute" Version="4.2.2" />
     <PackageReference Include="NSubstitute.Analyzers.CSharp" Version="1.0.14" />
     <PackageReference Include="xunit" Version="2.4.1" />
+    <PackageReference Include="Xunit.Priority" Version="1.1.6" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>