The multiool, core sarif, and validation test binaries now all pass (#…

…1215)

* The multiool, core sarif, and validation test binaries now all pass completely.

* Remove unwanted assert that may fire during unit testing.

* Merge from files-array

* PR feedback.

* PR feedback tweaks

src/Sarif.Converters.UnitTests/AndroidStudioConverterTests.cs

@@ -61,23 +61,23 @@ public void AndroidStudioConverter_Convert_NoResults()
         {
             string androidStudioLog = @"<?xml version=""1.0"" encoding=""UTF-8""?><problems></problems>";
             string actualJson = Utilities.GetConverterJson(_converter, androidStudioLog);
-            actualJson.Should().BeCrossPlatformEquivalent(emptyResult);
+            actualJson.Should().BeCrossPlatformEquivalent<SarifLog>(emptyResult);
         }
 
         [Fact]
         public void AndroidStudioConverter_Convert_EmptyResult()
         {
             string androidStudioLog = @"<?xml version=""1.0"" encoding=""UTF-8""?><problems><problem></problem></problems>";
             string actualJson = Utilities.GetConverterJson(_converter, androidStudioLog);
-            actualJson.Should().BeCrossPlatformEquivalent(emptyResult);
+            actualJson.Should().BeCrossPlatformEquivalent<SarifLog>(emptyResult);
         }
 
         [Fact]
         public void AndroidStudioConverter_Convert_EmptyResultSingleTag()
         {
             string androidStudioLog = @"<?xml version=""1.0"" encoding=""UTF-8""?><problems><problem /></problems>";
             string actualJson = Utilities.GetConverterJson(_converter, androidStudioLog);
-            actualJson.Should().BeCrossPlatformEquivalent(emptyResult);
+            actualJson.Should().BeCrossPlatformEquivalent<SarifLog>(emptyResult);
         }
 
         [Fact]

src/Sarif.Converters.UnitTests/ConverterTestsBase.cs

@@ -39,7 +39,7 @@ public SarifLog RunTestCase(string inputData, string expectedResult, bool pretty
             // are fragile. It would be better for our testing to have a dedicated set of data-driven
             // tests that flag changes and for the unit-tests to work exclusively against the 
             // object model.
-            actualJson.Should().BeCrossPlatformEquivalent(expectedResult);
+            actualJson.Should().BeCrossPlatformEquivalent<SarifLog>(expectedResult);
 
             return log;
         }

src/Sarif.Converters.UnitTests/FxCopConverterTests.cs

@@ -103,7 +103,7 @@ public void FxCopLogReader_Context_RefineStackTrace()
             var context = TestHelper.CreateProjectContext();
 
             context.RefineStackTrace(@"trace\n trace");
-            context.StackTrace.Should().BeCrossPlatformEquivalent(@"trace\n trace");
+            context.StackTrace.Should().BeCrossPlatformEquivalentStrings(@"trace\n trace");
         }
 
         [Fact]

src/Sarif.Driver/Sdk/SkimmerBase.cs

@@ -7,17 +7,12 @@
 
 namespace Microsoft.CodeAnalysis.Sarif.Driver
 {
-    public abstract class SkimmerBase<TContext>  : PropertyBagHolder, ISkimmer<TContext>
+    public abstract class SkimmerBase<TContext>  : Rule, ISkimmer<TContext>
     {
         public SkimmerBase()
         {
             this.Options = new Dictionary<string, string>();
         }
-
-        abstract public Uri HelpUri { get;  }
-
-        abstract public Message Help { get; }
-
         private IDictionary<string, string> messageStrings;
         private IDictionary<string, string> richMessageStrings;
 
@@ -27,11 +22,10 @@ public SkimmerBase()
 
         virtual protected IEnumerable<string> RichMessageResourceNames => new List<string>();
 
-        virtual public RuleConfiguration Configuration {  get; }
 
         virtual public ResultLevel DefaultLevel { get { return ResultLevel.Warning; } }
 
-        virtual public IDictionary<string, string> MessageStrings
+        override public IDictionary<string, string> MessageStrings
         {
             get
             {
@@ -43,7 +37,7 @@ virtual public IDictionary<string, string> MessageStrings
             }
         }
 
-        virtual public IDictionary<string, string> RichMessageStrings
+        override public IDictionary<string, string> RichMessageStrings
         {
             get
             {
@@ -65,14 +59,11 @@ private Dictionary<string, string> InitializeRichMessageStrings()
             return RuleUtilities.BuildDictionary(ResourceManager, RichMessageResourceNames,ruleId: Id, prefix: "Rich");
         }
 
-        abstract public string Id { get; }
-
-        // This one isn't abstract because there's no point in forcing every skimmer to implement it.
-        public virtual IList<string> DeprecatedIds => null;
+        public override string Id => throw new InvalidOperationException($"The {nameof(Id)} property must be overridden in the SkimmerBase-derived class.");
 
-        abstract public Message FullDescription { get; }
+        public override Message FullDescription => throw new InvalidOperationException($"The {nameof(FullDescription)} property must be overridden in the SkimmerBase-derived class.");
 
-        public virtual Message ShortDescription
+        public override Message ShortDescription
         {
             get { return new Message { Text = FirstSentence(FullDescription.Text) }; }
         }
@@ -105,7 +96,7 @@ internal static string FirstSentence(string fullDescription)
             return fullDescription.Substring(0, length) + (truncated ? "..." : "");
         }
 
-        public virtual Message Name {  get { return new Message { Text = this.GetType().Name }; } }
+        public override Message Name {  get { return new Message { Text = this.GetType().Name }; } }
 
         public IDictionary<string, string> Options { get; }
 

src/Sarif.FunctionalTests/SarifConverterTests.cs

@@ -155,7 +155,7 @@ private string RunConverter(StringBuilder sb, string toolFormat, string inputFil
 
             string actualSarif = File.ReadAllText(generatedFileName);
 
-            if (!AreEquivalentSarifLogs<SarifLog>(actualSarif, expectedSarif))
+            if (!AreEquivalent<SarifLog>(actualSarif, expectedSarif))
             {
                 File.WriteAllText(expectedFileName, expectedSarif);
                 File.WriteAllText(generatedFileName, actualSarif);

src/Sarif.Multitool.FunctionalTests/Rules/ValidationSkimmerTestsBase.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using FluentAssertions;
@@ -87,15 +88,31 @@ protected void Verify(string testFileName, bool disablePrereleaseCompatibilityTr
         // Verify that those detected result locations match the expected locations.
         private void Verify(Run run, ExpectedValidationResults expectedResults)
         {
-            string[] detectedResultLocations = run.Results.Select(r => r.Message.Arguments[0]).OrderBy(loc => loc).ToArray();
-            string[] expectedResultLocations = expectedResults.ResultLocationPointers.OrderBy(loc => loc).ToArray();
+            HashSet<string> actualResultLocations = new HashSet<string>(run.Results.Select(r => r.Message.Arguments[0]));
+            HashSet<string> unexpectedlyAbsentResultLocations = new HashSet<string>(expectedResults.ResultLocationPointers);
 
-            // We could make this assertion at the start of the method. We delay it until here
-            // so that, during debugging, you can set a breakpoint here, and you'll have the
-            // detected and expected result location arrays available to compare.
-            run.Results.Count.Should().Be(expectedResults.ResultCount);
+            List<string> unexpectedNewIssues = new List<string>();
 
-            detectedResultLocations.Should().ContainInOrder(expectedResultLocations);
+            foreach(string location in actualResultLocations)
+            {
+                if (unexpectedlyAbsentResultLocations.Contains(location))
+                {
+                    // Happy path. Actual location found in expected
+                    unexpectedlyAbsentResultLocations.Remove(location);
+                }
+                else
+                {
+                    // Bad new unexpected issue 
+                    unexpectedNewIssues.Add(location);
+                }
+            }
+
+            // No new issues
+            unexpectedNewIssues.Count.Should().Be(0);
+
+            // No unexpectedly absent issues. If we found everything we expected,
+            // then our expected results should be empty
+            unexpectedlyAbsentResultLocations.Count.Should().Be(0);
         }
     }
 }