Disable logging by default in Configuration.cs (#362)

- Replace `GeneralSettings.DisableLogging` with `GeneralSettings.EnableLogging` and set it to `false` in `Configuration.cs`
- Map it manually on startup to avoid dotnet/runtime#89732

This should avoid the issue of the binary crashing without `appsettings.json`.
**This doesn't mean it should be used without it**, it will swallow all errors and arnings, but yeah, you can. 

- Add test to ensure that `appsettings.json` are `Configuration.cs` are sync'ed
- Adjust `appsettings` values to Config ones

.github/workflows/ci.yml

@@ -147,7 +147,7 @@ jobs:
     - name: Build
       run: dotnet build -c ${{ matrix.configuration }} /p:DeterministicBuild=true
 
-    - name: Run CI tests
+    - name: Run ${{ matrix.category }} tests
       run: dotnet test -c ${{ matrix.configuration }} --no-build --collect:"XPlat Code Coverage" -v normal
 
     - name: '[Ubuntu] Generate test coverage report'
@@ -239,6 +239,38 @@ jobs:
       run: dotnet test -c Release --no-build --filter "TestCategory=${{ matrix.category }}" -v normal
         # --collect:"XPlat Code Coverage" https://github.com/coverlet-coverage/coverlet/issues/1192
 
+  other-tests:
+    runs-on: ${{ matrix.os }}
+
+    strategy:
+      matrix:
+        os: [ubuntu-latest, windows-latest, macOS-latest]
+        category: [Configuration]
+      fail-fast: false
+
+    steps:
+    - uses: actions/checkout@v3
+
+    - name: Setup .NET
+      uses: actions/setup-dotnet@v3
+      with:
+        dotnet-version: ${{ env.DOTNET_VERSION }}
+
+    - name: Nuget cache
+      uses: actions/cache@v3
+      with:
+        path:
+          ~/.nuget/packages
+        key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json') }}
+        restore-keys: |
+          ${{ runner.os }}-nuget-
+
+    - name: Build
+      run: dotnet build -c Release
+
+    - name: Run ${{ matrix.category }} tests
+      run: dotnet test -c Release --no-build --filter "TestCategory=${{ matrix.category }}" -v normal --collect:"XPlat Code Coverage"
+
   winning-at-chess:
     runs-on: ${{ matrix.os }}
 

src/Lynx.Cli/Program.cs

@@ -20,10 +20,17 @@
     .AddEnvironmentVariables()
     .Build();
 
-config.GetRequiredSection(nameof(EngineSettings)).Bind(Configuration.EngineSettings);
-config.GetRequiredSection(nameof(GeneralSettings)).Bind(Configuration.GeneralSettings);
+config.GetSection(nameof(EngineSettings)).Bind(Configuration.EngineSettings);
+config.GetSection(nameof(GeneralSettings)).Bind(Configuration.GeneralSettings);
 
-if (!Configuration.GeneralSettings.DisableLogging)
+// TODO remove when .NET sdk includes https://github.com/dotnet/runtime/issues/89732
+var generalConfig = config.GetSection(nameof(GeneralSettings));
+if (bool.TryParse(generalConfig[nameof(Configuration.GeneralSettings.EnableLogging)], out var enableLogging))
+{
+    Configuration.GeneralSettings.EnableLogging = enableLogging;
+}
+
+if (Configuration.GeneralSettings.EnableLogging)
 {
     LogManager.Configuration = new NLogLoggingConfiguration(config.GetSection("NLog"));
 }

src/Lynx.Cli/appsettings.json

@@ -1,23 +1,23 @@
 {
   // Settings that affect the executable behavior
   "GeneralSettings": {
-    "DisableLogging": false // Completely disables logging, both console and file. Alternatively, one or more "NLog.rules" can be removed/tweaked
+    "EnableLogging": true // logging can be completely disablesd, both console and file, setting this to false. Alternatively, one or more "NLog.rules" can be removed/tweaked
   },
 
   // Settings that affect the engine behavior
   "EngineSettings": {
     "DefaultMaxDepth": 5,
     "CoefficientBeforeKeyMovesBeforeMovesToGo": 1.5,
     "KeyMovesBeforeMovesToGo": 10,
-    "CoefficientAfterKeyMovesBeforeMovesToGo": 0.9,
+    "CoefficientAfterKeyMovesBeforeMovesToGo": 0.95,
     "TotalMovesWhenNoMovesToGoProvided": 100,
     "FixedMovesLeftWhenNoMovesToGoProvidedAndOverTotalMovesWhenNoMovesToGoProvided": 20,
     "FirstTimeLimitWhenNoMovesToGoProvided": 120000,
     "FirstCoefficientWhenNoMovesToGoProvided": 3,
     "SecondTimeLimitWhenNoMovesToGoProvided": 30000,
     "SecondCoefficientWhenNoMovesToGoProvided": 2,
     "MinSecurityTime": 1000,
-    "CoefficientSecurityTime": 0.95,
+    "CoefficientSecurityTime": 0.90,
     "MinDepth": 4,
     "MaxDepth": 128,
     //"MinMoveTime": 1000,

src/Lynx/Configuration.cs

@@ -75,7 +75,7 @@ public static int Hash
 
 public sealed class GeneralSettings
 {
-    public bool DisableLogging { get; set; } = false;
+    public bool EnableLogging { get; set; } = false;
 }
 
 public sealed class EngineSettings
@@ -133,7 +133,7 @@ public sealed class EngineSettings
     #endregion
 
     /// <summary>
-    /// Min. time left in the clock if all decision time is used befire <see cref="CoefficientSecurityTime"/> is used over that decision time
+    /// Min. time left in the clock if all decision time is used before <see cref="CoefficientSecurityTime"/> is used over that decision time
     /// </summary>
     public int MinSecurityTime { get; set; } = 1_000;
 

tests/Lynx.Test/Categories.cs

@@ -6,9 +6,23 @@ public static class Categories
 
     public const string LongRunning = "LongRunning";
 
+    /// <summary>
+    /// Need to run in isolation, since other tests might modify <see cref="Configuration"/> values
+    /// </summary>
+    public const string Configuration = "Configuration";
+
+    /// <summary>
+    /// Can't be run since it'd take way too long for regular CI
+    /// </summary>
     public const string TooLong = "TooLongToBeRun";
 
+    /// <summary>
+    /// Can't be run since no prunning is required
+    /// </summary>
     public const string NoPruning = "RequireNoPruning";
 
+    /// <summary>
+    /// Can't be run since our engine is simply not good enough yet
+    /// </summary>
     public const string NotGoodEnough = "NotGoodEnough";
 }

tests/Lynx.Test/ConfigurationTest.cs

@@ -0,0 +1,58 @@
+using Microsoft.Extensions.Configuration;
+using NUnit.Framework;
+using System.Text.Json;
+using System.Text.Json.Nodes;
+
+namespace Lynx.Test;
+
+[Explicit]
+[Category(Categories.Configuration)]
+[NonParallelizable]
+public class ConfigurationTest
+{
+    [Test]
+    public void SynchronizedAppSettings()
+    {
+        var config = new ConfigurationBuilder()
+            .AddJsonFile("appsettings.json", optional: true, reloadOnChange: false)
+            .Build();
+
+        var engineSettingsSection = config.GetRequiredSection(nameof(EngineSettings));
+        Assert.IsNotNull(engineSettingsSection);
+
+#pragma warning disable IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code
+        var serializedEngineSettingsConfig = JsonSerializer.Serialize(Configuration.EngineSettings);
+        var jsonNode = JsonSerializer.Deserialize<JsonNode>(serializedEngineSettingsConfig);
+#pragma warning restore IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code
+        Assert.IsNotNull(jsonNode);
+
+#pragma warning disable IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code - using sourcegenerator
+        engineSettingsSection.Bind(Configuration.EngineSettings);
+#pragma warning restore IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code
+
+        var reflectionProperties = Configuration.EngineSettings.GetType().GetProperties(
+            System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance);
+        Assert.GreaterOrEqual(reflectionProperties.Length, 35);
+
+        var originalCulture = Thread.CurrentThread.CurrentCulture;
+        Thread.CurrentThread.CurrentCulture = System.Globalization.CultureInfo.InvariantCulture;
+
+        Assert.Multiple(() =>
+        {
+            foreach (var property in reflectionProperties)
+            {
+                if (property.PropertyType == typeof(int[]))
+                {
+                    continue;
+                }
+
+                var sourceSetting = jsonNode![property.Name]!.ToString().ToLowerInvariant();
+                var configSetting = property.GetValue(Configuration.EngineSettings)!.ToString()!.ToLowerInvariant();
+
+                Assert.AreEqual(sourceSetting, configSetting, $"Error in {property.Name} ({property.PropertyType}): (Configuration.cs) {sourceSetting} != {configSetting} (appSettings.json)");
+            }
+
+            Thread.CurrentThread.CurrentCulture = originalCulture;
+        });
+    }
+}

tests/Lynx.Test/Lynx.Test.csproj

@@ -2,6 +2,7 @@
 
   <PropertyGroup>
     <IsPackable>false</IsPackable>
+    <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>
   </PropertyGroup>
 
   <ItemGroup>
@@ -16,7 +17,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <ProjectReference Include="..\..\src\Lynx\Lynx.csproj" />
+    <ProjectReference Include="..\..\src\Lynx.Cli\Lynx.Cli.csproj" />
   </ItemGroup>
 
 </Project>