Merge branch 'Cleanup_refactor' into '2018.2'

Cleanup refactor

See merge request seans/PerformanceBenchmarkReporter!2

Author: seanstolberg-unity

UnityPerformanceBenchmarkReporter/Entities/PerformanceTestRun.cs

@@ -73,6 +73,7 @@ public class QualitySettings
     public class PlayerSettings
     {
         public string ScriptingBackend;
+        public string ScriptingRuntimeVersion;
         public bool VrSupported;
         public bool MtRendering;
         public bool GraphicsJobs;

UnityPerformanceBenchmarkReporter/Entities/PerformanceTestRunResult.cs

@@ -17,5 +17,15 @@ public class PerformanceTestRunResult
         public List<TestResult> TestResults  = new List<TestResult>();
         public bool IsBaseline;
         public string ResultName;
+
+        public bool TestRunMetadataExists()
+        {
+            return PlayerSystemInfo != null
+                   || PlayerSettings != null
+                   || QualitySettings != null
+                   || ScreenSettings != null
+                   || BuildSettings != null
+                   || EditorVersion != null;
+        }
     }
 }

UnityPerformanceBenchmarkReporter/MetadataValidator.cs

@@ -65,12 +65,12 @@ private OptionSet GetOptions(PerformanceBenchmark performanceBenchmark)
                 .Add("results|testresultsxmlsource=", "REQUIRED - Path to a test result XML filename OR directory. Directories are searched resursively. You can repeat this option with multiple result file or directory paths.",
                     xmlsource =>
                     {
-                        performanceBenchmark.AddXmlSourcePath(xmlsource, "testresultsxmlsource", ResultType.Test);
+                        performanceBenchmark.AddXmlSourcePath(xmlsource, "results", ResultType.Test);
                     })
                 .Add("baseline|baselinexmlsource:", "OPTIONAL - Path to a baseline XML filename.",
                         xmlsource =>
                         {
-                            performanceBenchmark.AddXmlSourcePath(xmlsource, "baselinexmlsource", ResultType.Baseline);
+                            performanceBenchmark.AddXmlSourcePath(xmlsource, "baseline", ResultType.Baseline);
                         })
                 .Add("report|reportdirpath:", "OPTIONAL - Path to where the report will be written. Default is current working directory.",
                     performanceBenchmark.AddReportDirPath);

UnityPerformanceBenchmarkReporter/OptionsParser.cs

@@ -9,68 +9,61 @@ namespace UnityPerformanceBenchmarkReporter
 {
     public class PerformanceBenchmark
     {
-        public HashSet<string> ResultXmlFilePaths { get; } = new HashSet<string>();
-        public HashSet<string> ResultXmlDirectoryPaths { get; } = new HashSet<string>();
-        public HashSet<string> BaselineXmlFilePaths { get; } = new HashSet<string>();
-        public HashSet<string> BaselineXmlDirectoryPaths { get; } = new HashSet<string>();
-        public uint SigFig { get; private set; }
-        public string ReportDirPath { get; private set; }
-
-        public readonly MetadataValidator MetadataValidator = new MetadataValidator();
-
-        private bool firstResult = true;
-        private string firstTestRunResultPath;
-        private PerformanceTestRun firstTestRun = new PerformanceTestRun();
         private readonly PerformanceTestRunProcessor performanceTestRunProcessor = new PerformanceTestRunProcessor();
-        private readonly string xmlFileExtension = ".xml";
-        private readonly Dictionary<string, string[]> excludedConfigFieldNames = new Dictionary<string, string[]>();
-
-
-        public bool BaselineResultFilesExist => BaselineXmlFilePaths.Any() || BaselineXmlDirectoryPaths.Any();
 
-        public bool ResultFilesExist => ResultXmlFilePaths.Any() || ResultXmlDirectoryPaths.Any();
+        public readonly TestRunMetadataProcessor TestRunMetadataProcessor;
+        private readonly string xmlFileExtension = ".xml";
 
-        public PerformanceBenchmark(Dictionary<string, string[]> configFieldNames = null)
+        public PerformanceBenchmark(Dictionary<Type, string[]> configFieldNames = null)
         {
             // Default significant figures to use for non-integer metrics if user doesn't specify another value.
             // Most values are in milliseconds or a count of something, so using more often creates an artificial baseline
             // failure based on insignificant digits equating to a microsecond, or less, time difference. The Unity Profiler only shows
             // up to 2 significant figures for milliseconds as well, so this is what folks are used to working with.
             SigFig = 2;
-
-            if (configFieldNames != null)
-            {
-                excludedConfigFieldNames = configFieldNames;
-            }
+            TestRunMetadataProcessor = new TestRunMetadataProcessor(configFieldNames);
         }
 
+        public HashSet<string> ResultXmlFilePaths { get; } = new HashSet<string>();
+        public HashSet<string> ResultXmlDirectoryPaths { get; } = new HashSet<string>();
+        public HashSet<string> BaselineXmlFilePaths { get; } = new HashSet<string>();
+        public uint SigFig { get; }
+        public string ReportDirPath { get; private set; }
+
+
+        public bool BaselineResultFilesExist => BaselineXmlFilePaths.Any();
+
+        public bool ResultFilesExist => ResultXmlFilePaths.Any() || ResultXmlDirectoryPaths.Any();
+
         public void AddPerformanceTestRunResults(
-            TestResultXmlParser testResultXmlParser, 
-            List<PerformanceTestRunResult> performanceTestRunResults, 
-            List<TestResult> testResults, 
+            TestResultXmlParser testResultXmlParser,
+            List<PerformanceTestRunResult> performanceTestRunResults,
+            List<TestResult> testResults,
             List<TestResult> baselineTestResults)
         {
-            AddTestResults(testResultXmlParser, performanceTestRunResults, testResults, baselineTestResults, ResultXmlDirectoryPaths, ResultXmlFilePaths);
+            AddTestResults(testResultXmlParser, performanceTestRunResults, testResults, baselineTestResults,
+                ResultXmlDirectoryPaths, ResultXmlFilePaths);
         }
 
         public void AddBaselinePerformanceTestRunResults(
-            TestResultXmlParser testResultXmlParser, 
-            List<PerformanceTestRunResult> baselinePerformanceTestRunResults, 
+            TestResultXmlParser testResultXmlParser,
+            List<PerformanceTestRunResult> baselinePerformanceTestRunResults,
             List<TestResult> baselineTestResults)
         {
-            AddTestResults(testResultXmlParser, baselinePerformanceTestRunResults, baselineTestResults, baselineTestResults, BaselineXmlDirectoryPaths, BaselineXmlFilePaths, true);
+            AddTestResults(testResultXmlParser, baselinePerformanceTestRunResults, baselineTestResults,
+                baselineTestResults, null, BaselineXmlFilePaths, true);
         }
 
         private void AddTestResults(
-            TestResultXmlParser testResultXmlParser,  
-            List<PerformanceTestRunResult> runResults, 
-            List<TestResult> testResults, 
+            TestResultXmlParser testResultXmlParser,
+            List<PerformanceTestRunResult> testRunResults,
+            List<TestResult> testResults,
             List<TestResult> baselineTestResults,
-            HashSet<string> xmlDirectoryPaths, 
+            HashSet<string> xmlDirectoryPaths,
             HashSet<string> xmlFileNamePaths,
             bool isBaseline = false)
         {
-            if (!isBaseline && xmlDirectoryPaths.Any())
+            if (!isBaseline && xmlDirectoryPaths != null && xmlDirectoryPaths.Any())
             {
                 foreach (var xmlDirectory in xmlDirectoryPaths)
                 {
@@ -92,36 +85,44 @@ private void AddTestResults(
                     var performanceTestRun = testResultXmlParser.GetPerformanceTestRunFromXml(xmlFileNamePath);
                     if (performanceTestRun != null && performanceTestRun.Results.Any())
                     {
-                        perfTestRuns.Add( new KeyValuePair<string, PerformanceTestRun>(xmlFileNamePath, performanceTestRun));
+                        perfTestRuns.Add(
+                            new KeyValuePair<string, PerformanceTestRun>(xmlFileNamePath, performanceTestRun));
                     }
                 }
 
-                perfTestRuns.Sort((run1, run2) => run1.Value.StartTime.CompareTo(run2.Value.StartTime));
-                var resultFilesOrderByStartTime = perfTestRuns.ToArray();
+                perfTestRuns.Sort((run1, run2) => string.Compare(run1.Key, run2.Key, StringComparison.Ordinal));
+                var resultFilesOrderedByResultName = perfTestRuns.ToArray();
 
-                for (var i = 0; i < resultFilesOrderByStartTime.Length; i++)
+                for (var i = 0; i < resultFilesOrderedByResultName.Length; i++)
                 {
-                    var performanceTestRun = testResultXmlParser.GetPerformanceTestRunFromXml(resultFilesOrderByStartTime[i].Key);
+                    var performanceTestRun =
+                        testResultXmlParser.GetPerformanceTestRunFromXml(resultFilesOrderedByResultName[i].Key);
 
                     if (performanceTestRun != null && performanceTestRun.Results.Any())
                     {
                         var results = performanceTestRunProcessor.GetTestResults(performanceTestRun);
                         if (!results.Any())
                         {
                             Console.ForegroundColor = ConsoleColor.Yellow;
-                            Console.WriteLine("No performance test data found to report in: {0}", resultFilesOrderByStartTime[i].Key);
+                            Console.WriteLine("No performance test data found to report in: {0}",
+                                resultFilesOrderedByResultName[i].Key);
                             Console.ResetColor();
+                            continue;
                         }
+
                         testResults.AddRange(results);
 
-                        performanceTestRunProcessor.UpdateTestResultsBasedOnBaselineResults(baselineTestResults, testResults, SigFig);
+                        performanceTestRunProcessor.UpdateTestResultsBasedOnBaselineResults(baselineTestResults,
+                            testResults, SigFig);
 
-                        ValidateMetadata(performanceTestRun, resultFilesOrderByStartTime[i].Key);
-                        runResults.Add(performanceTestRunProcessor.CreateTestRunResult
+                        TestRunMetadataProcessor.ProcessMetadata(performanceTestRun,
+                            resultFilesOrderedByResultName[i].Key);
+
+                        testRunResults.Add(performanceTestRunProcessor.CreateTestRunResult
                             (
                                 performanceTestRun,
                                 results,
-                                Path.GetFileNameWithoutExtension(resultFilesOrderByStartTime[i].Key),
+                                Path.GetFileNameWithoutExtension(resultFilesOrderedByResultName[i].Key),
                                 isBaseline)
                         );
                     }
@@ -132,36 +133,11 @@ private void AddTestResults(
         private IEnumerable<string> GetAllXmlFileNames(string xmlDirectory)
         {
             var dir = new DirectoryInfo(xmlDirectory);
-            var xmlFileNames = dir.GetFiles("*" + xmlFileExtension, SearchOption.AllDirectories).Select(f => f.FullName);
+            var xmlFileNames = dir.GetFiles("*" + xmlFileExtension, SearchOption.AllDirectories)
+                .Select(f => f.FullName);
             return xmlFileNames;
         }
 
-        private void ValidateMetadata(PerformanceTestRun performanceTestRun, string xmlFileNamePath)
-        {
-            if (firstResult)
-            {
-                firstTestRunResultPath = xmlFileNamePath;
-                firstTestRun = performanceTestRun;
-                firstResult = false;
-            }
-            else
-            {
-                MetadataValidator.ValidatePlayerSystemInfo(firstTestRun, performanceTestRun, firstTestRunResultPath, xmlFileNamePath, ExcludedFieldNames<PlayerSystemInfo>());
-                MetadataValidator.ValidatePlayerSettings(firstTestRun, performanceTestRun, firstTestRunResultPath, xmlFileNamePath, ExcludedFieldNames<PlayerSettings>());
-                MetadataValidator.ValidateQualitySettings(firstTestRun, performanceTestRun, firstTestRunResultPath, xmlFileNamePath, ExcludedFieldNames<QualitySettings>());
-                MetadataValidator.ValidateScreenSettings(firstTestRun, performanceTestRun, firstTestRunResultPath, xmlFileNamePath, ExcludedFieldNames<ScreenSettings>());
-                MetadataValidator.ValidateBuildSettings(firstTestRun, performanceTestRun, firstTestRunResultPath, xmlFileNamePath, ExcludedFieldNames<BuildSettings>());
-                MetadataValidator.ValidateEditorVersion(firstTestRun, performanceTestRun, firstTestRunResultPath, xmlFileNamePath, ExcludedFieldNames<EditorVersion>());
-            }
-        }
-
-        private string[] ExcludedFieldNames<T>()
-        {
-            return excludedConfigFieldNames.ContainsKey(typeof(T).Name)
-                ? excludedConfigFieldNames[typeof(T).Name]
-                : null;
-        }
-
         public void AddXmlSourcePath(string xmlSourcePath, string optionName, OptionsParser.ResultType resultType)
         {
             if (string.IsNullOrEmpty(xmlSourcePath))
@@ -196,21 +172,12 @@ private void ProcessAsXmlDirectory(string xmlSourcePath, string optionName, Opti
             var xmlFileNames = GetAllXmlFileNames(xmlSourcePath).ToArray();
             if (!xmlFileNames.Any())
             {
-                throw new ArgumentException(string.Format("{0} directory `{1}` doesn't contain any .xml files.", optionName,
+                throw new ArgumentException(string.Format("{0} directory `{1}` doesn't contain any .xml files.",
+                    optionName,
                     xmlSourcePath));
             }
 
-            switch (resultType)
-            {
-                case OptionsParser.ResultType.Test:
-                    ResultXmlDirectoryPaths.Add(xmlSourcePath);
-                    break;
-                case OptionsParser.ResultType.Baseline:
-                    BaselineXmlDirectoryPaths.Add(xmlSourcePath);
-                    break;
-                default:
-                    throw new InvalidEnumArgumentException(resultType.ToString());
-            }
+            ResultXmlDirectoryPaths.Add(xmlSourcePath);
         }
 
         private void ProcessAsXmlFile(string xmlSourcePath, string optionName, OptionsParser.ResultType resultType)
@@ -237,10 +204,5 @@ public void AddReportDirPath(string reportDirectoryPath)
         {
             ReportDirPath = reportDirectoryPath;
         }
-
-        public void AddSigFig(uint sigFig)
-        {
-            SigFig = sigFig;
-        }
     }
-}
+}

UnityPerformanceBenchmarkReporter/PerformanceBenchmark.cs

@@ -8,10 +8,20 @@ namespace UnityPerformanceBenchmarkReporter
 {
     internal class Program
     {
-        private static readonly Dictionary<string, string[]> ExcludedConfigFieldNames = new Dictionary<string, string[]>
+        private static readonly Dictionary<Type, string[]> ExcludedConfigFieldNames = new Dictionary<Type, string[]>
         {
-            {typeof(EditorVersion).Name, new []{"DateSeconds", "RevisionValue", "Branch"}},
-            {typeof(PlayerSettings).Name, new []{"MtRendering", "GraphicsJobs"}}
+            {typeof(EditorVersion), new []
+            {
+                "DateSeconds", 
+                "RevisionValue", 
+                "Branch"
+            }},
+            {typeof(PlayerSettings), new []
+            {
+                "MtRendering", // Hidden because we have a calculated field, RenderThreadingMode, that provides a more succinct value (SingleThreaded, MultiThreaded, GfxJobs)
+                "GraphicsJobs", // Hidden because we have a calculated field, RenderThreadingMode, that provides a more succinct value (SingleThreaded, MultiThreaded, GfxJobs)
+                "VrSupported" // Hidden because this value doesn't seem to be coming through as 'True' when it should be true.
+            }}
         };
 
         private static void Main(string[] args)
@@ -55,10 +65,29 @@ private static void Main(string[] args)
                 }
             }
 
-            var reportWriter = new ReportWriter(ExcludedConfigFieldNames);
+            var performanceTestResults = new PerformanceTestRunResult[0]; 
+
+            if (aggregateTestRunResults.Any(a => a.IsBaseline))
+            {
+                Array.Resize(ref performanceTestResults, 1);
+                performanceTestResults[0] = aggregateTestRunResults.First(a => a.IsBaseline);
+            }
+
+            var nonBaselineTestRunResults = aggregateTestRunResults.Where(a => !a.IsBaseline).ToList();
+
+            nonBaselineTestRunResults.Sort((run1, run2) => string.Compare(run1.ResultName, run2.ResultName, StringComparison.Ordinal));
+
+
+            foreach (var performanceTestRunResult in nonBaselineTestRunResults)
+            {
+                Array.Resize(ref performanceTestResults, performanceTestResults.Length + 1);
+                performanceTestResults[performanceTestResults.Length - 1] = performanceTestRunResult;
+            }
+
+
+            var reportWriter = new ReportWriter(performanceBenchmark.TestRunMetadataProcessor);
             reportWriter.WriteReport(
-                aggregateTestRunResults, 
-                performanceBenchmark.MetadataValidator, 
+                performanceTestResults, 
                 performanceBenchmark.SigFig, 
                 performanceBenchmark.ReportDirPath, 
                 performanceBenchmark.BaselineResultFilesExist);