#1655: Emit warning messages to the runner for config files when rele…

…vant (v2)
xunit · Jan 20, 2024 · 5ef7b75 · 5ef7b75
1 parent 462c943
commit 5ef7b75
Show file tree

Hide file tree

Showing 10 changed files with 228 additions and 67 deletions.
diff --git a/src/xunit.console/ConsoleRunner.cs b/src/xunit.console/ConsoleRunner.cs
@@ -353,6 +353,9 @@ void PrintUsage(IReadOnlyList<IRunnerReporter> reporters)
                                  XunitFilters filters,
                                  bool internalDiagnosticMessages)
         {
+            foreach (var warning in assembly.ConfigWarnings)
+                logger.LogWarning(warning);
+
             if (cancel)
                 return null;
 

diff --git a/src/xunit.runner.msbuild/xunit.cs b/src/xunit.runner.msbuild/xunit.cs
@@ -252,6 +252,9 @@ public override bool Execute()
 
         protected virtual XElement ExecuteAssembly(XunitProjectAssembly assembly, AppDomainSupport? appDomains)
         {
+            foreach (var warning in assembly.ConfigWarnings)
+                logger.LogWarning(warning);
+
             if (cancel)
                 return null;
 

diff --git a/src/xunit.runner.tdnet/TdNetRunnerHelper.cs b/src/xunit.runner.tdnet/TdNetRunnerHelper.cs
@@ -25,7 +25,12 @@ public TdNetRunnerHelper(Assembly assembly, ITestListener testListener)
             this.testListener = testListener;
 
             var assemblyFileName = assembly.GetLocalCodeBase();
-            configuration = ConfigReader.Load(assemblyFileName);
+            var warnings = new List<string>();
+            configuration = ConfigReader.Load(assemblyFileName, warnings: warnings);
+
+            foreach (var warning in warnings)
+                testListener.WriteLine(warning, Category.Warning);
+
             var diagnosticMessageSink = new DiagnosticMessageSink(testListener, Path.GetFileNameWithoutExtension(assemblyFileName), configuration.DiagnosticMessagesOrDefault);
             xunit = new Xunit2(configuration.AppDomainOrDefault, new NullSourceInformationProvider(), assemblyFileName, shadowCopy: false, diagnosticMessageSink: diagnosticMessageSink);
             toDispose.Push(xunit);

diff --git a/src/xunit.runner.utility/Configuration/ConfigReader.cs b/src/xunit.runner.utility/Configuration/ConfigReader.cs
@@ -1,3 +1,5 @@
+using System.Collections.Generic;
+using System.Globalization;
 using System.IO;
 
 namespace Xunit
@@ -12,24 +14,35 @@ public static class ConfigReader
         /// </summary>
         /// <param name="assemblyFileName">The test assembly.</param>
         /// <param name="configFileName">The test assembly configuration file.</param>
+        /// <param name="warnings">A container to receive loading warnings, if desired.</param>
         /// <returns>The test assembly configuration.</returns>
-        public static TestAssemblyConfiguration Load(string assemblyFileName, string configFileName = null)
+        public static TestAssemblyConfiguration Load(string assemblyFileName, string configFileName = null, List<string> warnings = null)
         {
-            return ConfigReader_Json.Load(assemblyFileName, configFileName)
+            var result = ConfigReader_Json.Load(assemblyFileName, configFileName, warnings);
+            if (result != null)
+                return result;
+
 #if NETFRAMEWORK
-                ?? ConfigReader_Configuration.Load(assemblyFileName, configFileName)
+            result = ConfigReader_Configuration.Load(assemblyFileName, configFileName, warnings);
+            if (result != null)
+                return result;
 #endif
-                ?? new TestAssemblyConfiguration();
+
+            if (configFileName != null && warnings != null && warnings.Count == 0)
+                warnings.Add(string.Format(CultureInfo.CurrentCulture, "Couldn't load config file '{0}': unknown file type", configFileName));
+
+            return new TestAssemblyConfiguration();
         }
 
         /// <summary>
         /// Loads the test assembly configuration for the given test assembly from a JSON stream. Caller is responsible for opening the stream.
         /// </summary>
         /// <param name="configStream">Stream containing config for an assembly</param>
+        /// <param name="warnings">A container to receive loading warnings, if desired.</param>
         /// <returns>The test assembly configuration.</returns>
-        public static TestAssemblyConfiguration Load(Stream configStream)
+        public static TestAssemblyConfiguration Load(Stream configStream, List<string> warnings = null)
         {
-            return ConfigReader_Json.Load(configStream);
+            return ConfigReader_Json.Load(configStream, warnings);
         }
     }
 }
diff --git a/src/xunit.runner.utility/Configuration/ConfigReader_Configuration.cs b/src/xunit.runner.utility/Configuration/ConfigReader_Configuration.cs
@@ -1,7 +1,10 @@
 #if NETFRAMEWORK
 
 using System;
+using System.Collections.Generic;
 using System.Configuration;
+using System.Globalization;
+using System.IO;
 
 namespace Xunit
 {
@@ -15,40 +18,57 @@ public static class ConfigReader_Configuration
         /// </summary>
         /// <param name="assemblyFileName">The test assembly.</param>
         /// <param name="configFileName">The test assembly configuration file.</param>
+        /// <param name="warnings">A container to receive loading warnings, if desired.</param>
         /// <returns>The test assembly configuration.</returns>
-        public static TestAssemblyConfiguration Load(string assemblyFileName, string configFileName = null)
+        public static TestAssemblyConfiguration Load(string assemblyFileName, string configFileName = null, List<string> warnings = null)
         {
-            if (configFileName == null)
+            // If they provide a configuration file, we only read that, success or failure
+            if (configFileName != null)
+            {
+                if (!configFileName.EndsWith(".config", StringComparison.Ordinal))
+                    return null;
+
+                if (!File.Exists(configFileName))
+                {
+                    warnings?.Add(string.Format(CultureInfo.CurrentCulture, "Couldn't load config file '{0}': file not found", configFileName));
+                    return null;
+                }
+            }
+            else
+            {
                 configFileName = assemblyFileName + ".config";
+                if (!File.Exists(configFileName))
+                    return null;
+            }
 
-            if (configFileName.EndsWith(".config", StringComparison.Ordinal))
+            try
             {
-                try
+                var map = new ExeConfigurationFileMap { ExeConfigFilename = configFileName };
+                var config = ConfigurationManager.OpenMappedExeConfiguration(map, ConfigurationUserLevel.None);
+                if (config != null && config.AppSettings != null)
                 {
-                    var map = new ExeConfigurationFileMap { ExeConfigFilename = configFileName };
-                    var config = ConfigurationManager.OpenMappedExeConfiguration(map, ConfigurationUserLevel.None);
-                    if (config != null && config.AppSettings != null)
-                    {
-                        var result = new TestAssemblyConfiguration();
-                        var settings = config.AppSettings.Settings;
+                    var result = new TestAssemblyConfiguration();
+                    var settings = config.AppSettings.Settings;
 
-                        result.AppDomain = GetEnum<AppDomainSupport>(settings, Configuration.AppDomain) ?? result.AppDomain;
-                        result.DiagnosticMessages = GetBoolean(settings, Configuration.DiagnosticMessages) ?? result.DiagnosticMessages;
-                        result.InternalDiagnosticMessages = GetBoolean(settings, Configuration.InternalDiagnosticMessages) ?? result.InternalDiagnosticMessages;
-                        result.MaxParallelThreads = GetInt(settings, Configuration.MaxParallelThreads) ?? result.MaxParallelThreads;
-                        result.MethodDisplay = GetEnum<TestMethodDisplay>(settings, Configuration.MethodDisplay) ?? result.MethodDisplay;
-                        result.MethodDisplayOptions = GetEnum<TestMethodDisplayOptions>(settings, Configuration.MethodDisplayOptions) ?? result.MethodDisplayOptions;
-                        result.ParallelizeAssembly = GetBoolean(settings, Configuration.ParallelizeAssembly) ?? result.ParallelizeAssembly;
-                        result.ParallelizeTestCollections = GetBoolean(settings, Configuration.ParallelizeTestCollections) ?? result.ParallelizeTestCollections;
-                        result.PreEnumerateTheories = GetBoolean(settings, Configuration.PreEnumerateTheories) ?? result.PreEnumerateTheories;
-                        result.ShadowCopy = GetBoolean(settings, Configuration.ShadowCopy) ?? result.ShadowCopy;
-                        result.StopOnFail = GetBoolean(settings, Configuration.StopOnFail) ?? result.StopOnFail;
-                        result.LongRunningTestSeconds = GetInt(settings, Configuration.LongRunningTestSeconds) ?? result.LongRunningTestSeconds;
+                    result.AppDomain = GetEnum<AppDomainSupport>(settings, Configuration.AppDomain) ?? result.AppDomain;
+                    result.DiagnosticMessages = GetBoolean(settings, Configuration.DiagnosticMessages) ?? result.DiagnosticMessages;
+                    result.InternalDiagnosticMessages = GetBoolean(settings, Configuration.InternalDiagnosticMessages) ?? result.InternalDiagnosticMessages;
+                    result.MaxParallelThreads = GetInt(settings, Configuration.MaxParallelThreads) ?? result.MaxParallelThreads;
+                    result.MethodDisplay = GetEnum<TestMethodDisplay>(settings, Configuration.MethodDisplay) ?? result.MethodDisplay;
+                    result.MethodDisplayOptions = GetEnum<TestMethodDisplayOptions>(settings, Configuration.MethodDisplayOptions) ?? result.MethodDisplayOptions;
+                    result.ParallelizeAssembly = GetBoolean(settings, Configuration.ParallelizeAssembly) ?? result.ParallelizeAssembly;
+                    result.ParallelizeTestCollections = GetBoolean(settings, Configuration.ParallelizeTestCollections) ?? result.ParallelizeTestCollections;
+                    result.PreEnumerateTheories = GetBoolean(settings, Configuration.PreEnumerateTheories) ?? result.PreEnumerateTheories;
+                    result.ShadowCopy = GetBoolean(settings, Configuration.ShadowCopy) ?? result.ShadowCopy;
+                    result.StopOnFail = GetBoolean(settings, Configuration.StopOnFail) ?? result.StopOnFail;
+                    result.LongRunningTestSeconds = GetInt(settings, Configuration.LongRunningTestSeconds) ?? result.LongRunningTestSeconds;
 
-                        return result;
-                    }
+                    return result;
                 }
-                catch { }
+            }
+            catch (Exception ex)
+            {
+                warnings?.Add(string.Format(CultureInfo.CurrentCulture, "Exception loading config file '{0}': {1}", configFileName, ex.Message));
             }
 
             return null;

diff --git a/src/xunit.runner.utility/Configuration/ConfigReader_Json.cs b/src/xunit.runner.utility/Configuration/ConfigReader_Json.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
 
@@ -17,9 +18,53 @@ public static class ConfigReader_Json
         /// Loads the test assembly configuration for the given test assembly from a JSON stream. Caller is responsible for opening the stream.
         /// </summary>
         /// <param name="configStream">Stream containing config for an assembly</param>
+        /// <param name="warnings">A container to receive loading warnings, if desired.</param>
         /// <returns>The test assembly configuration.</returns>
-        public static TestAssemblyConfiguration Load(Stream configStream)
+        public static TestAssemblyConfiguration Load(Stream configStream, List<string> warnings = null) =>
+            LoadConfiguration(configStream, null, warnings);
+
+        /// <summary>
+        /// Loads the test assembly configuration for the given test assembly.
+        /// </summary>
+        /// <param name="assemblyFileName">The test assembly.</param>
+        /// <param name="configFileName">The test assembly configuration file.</param>
+        /// <param name="warnings">A container to receive loading warnings, if desired.</param>
+        /// <returns>The test assembly configuration.</returns>
+        public static TestAssemblyConfiguration Load(string assemblyFileName, string configFileName = null, List<string> warnings = null)
+        {
+            // If they provide a configuration file, we only read that, success or failure
+            if (configFileName != null)
+            {
+                if (!configFileName.EndsWith(".json", StringComparison.Ordinal))
+                    return null;
+
+#if !NETSTANDARD1_1
+                if (!File.Exists(configFileName))
+                {
+                    warnings?.Add(string.Format(CultureInfo.CurrentCulture, "Couldn't load config file '{0}': file not found", configFileName));
+                    return null;
+                }
+#endif
+
+                return LoadFile(configFileName, warnings);
+            }
+
+            var assemblyName = Path.GetFileNameWithoutExtension(assemblyFileName);
+            var directoryName = Path.GetDirectoryName(assemblyFileName);
+
+            return LoadFile(Path.Combine(directoryName, string.Format(CultureInfo.InvariantCulture, "{0}.xunit.runner.json", assemblyName)), warnings)
+                ?? LoadFile(Path.Combine(directoryName, "xunit.runner.json"), warnings);
+        }
+
+        static TestAssemblyConfiguration LoadConfiguration(Stream configStream, string configFileName, List<string> warnings)
         {
+            Guard.ArgumentNotNull(nameof(configStream), configStream);
+
+            string ConfigDescription() =>
+                configFileName == null
+                    ? "configuration"
+                    : string.Format(CultureInfo.CurrentCulture, "config file '{0}'", configFileName);
+
             var result = new TestAssemblyConfiguration();
 
             try
@@ -28,6 +73,12 @@ public static TestAssemblyConfiguration Load(Stream configStream)
                 {
                     var config = JsonDeserializer.Deserialize(reader) as JsonObject;
 
+                    if (config == null)
+                    {
+                        warnings?.Add(string.Format(CultureInfo.CurrentCulture, "Couldn't parse {0}: the root must be a JSON object", ConfigDescription()));
+                        return null;
+                    }
+
                     foreach (var propertyName in config.Keys)
                     {
                         var propertyValue = config.Value(propertyName);
@@ -114,41 +165,24 @@ public static TestAssemblyConfiguration Load(Stream configStream)
                     }
                 }
             }
-            catch { }
+            catch (Exception ex)
+            {
+                warnings?.Add(string.Format(CultureInfo.CurrentCulture, "Exception loading {0}: {1}", ConfigDescription(), ex.Message));
+            }
 
             return result;
         }
 
-        /// <summary>
-        /// Loads the test assembly configuration for the given test assembly.
-        /// </summary>
-        /// <param name="assemblyFileName">The test assembly.</param>
-        /// <param name="configFileName">The test assembly configuration file.</param>
-        /// <returns>The test assembly configuration.</returns>
-        public static TestAssemblyConfiguration Load(string assemblyFileName, string configFileName = null)
-        {
-            if (configFileName != null)
-                return configFileName.EndsWith(".json", StringComparison.Ordinal) ? LoadFile(configFileName) : null;
-
-            var assemblyName = Path.GetFileNameWithoutExtension(assemblyFileName);
-            var directoryName = Path.GetDirectoryName(assemblyFileName);
-
-            return LoadFile(Path.Combine(directoryName, string.Format(CultureInfo.InvariantCulture, "{0}.xunit.runner.json", assemblyName)))
-                ?? LoadFile(Path.Combine(directoryName, "xunit.runner.json"));
-        }
-
-        static TestAssemblyConfiguration LoadFile(string configFileName)
+        static TestAssemblyConfiguration LoadFile(string configFileName, List<string> warnings = null)
         {
-            try
-            {
 #if !NETSTANDARD1_1
-                if (!File.Exists(configFileName))
-                {
-                    return null;
-                }
+            if (!File.Exists(configFileName))
+                return null;
 #endif
+            try
+            {
                 using (var stream = File_OpenRead(configFileName))
-                    return Load(stream);
+                    return LoadConfiguration(stream, configFileName, warnings);
             }
             catch { }
 

diff --git a/src/xunit.runner.utility/Project/XunitProjectAssembly.cs b/src/xunit.runner.utility/Project/XunitProjectAssembly.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 
 namespace Xunit
 {
@@ -27,12 +28,17 @@ public TestAssemblyConfiguration Configuration
             get
             {
                 if (configuration == null)
-                    configuration = ConfigReader.Load(AssemblyFilename, ConfigFilename);
+                    configuration = ConfigReader.Load(AssemblyFilename, ConfigFilename, ConfigWarnings);
 
                 return configuration;
             }
         }
 
+        /// <summary>
+        /// Gets the list of warnings that resulting from reading the configuration.
+        /// </summary>
+        public List<string> ConfigWarnings { get; } = new();
+
         /// <summary>
         /// Gets or sets a value indicating whether to shadow copy the assembly
         /// when running the tests.