Additional tests against missing code coverage (#25)

* Additional tests against missing code coverage
* Updates logging within SyncWork to use structured logging syntax
* Removes private properties from `InvalidStateException`, as their values even when made public weren't serializing correctly.  This shouldn't impact the sem ver of the library, I don't think, since the properties were private anyway.

.github/workflows/ci.yml

@@ -14,7 +14,7 @@ jobs:
       matrix:
         target-framework: [ 'netcoreapp3.1', 'net5.0', 'net6.0' ]
 
-    name: dotnet build and test targetting ${{ matrix.target-framework }}
+    name: dotnet build and test targeting ${{ matrix.target-framework }}
     steps:
     - uses: actions/checkout@v2
       with:

samples/Orleans.SyncWork.Demo.Api/Program.cs

@@ -4,6 +4,7 @@
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using Orleans;
 using Orleans.SyncWork;
 using Orleans.SyncWork.Demo.Api;
@@ -40,8 +41,16 @@
 app
     .MapPost("/passwordVerifier", async (PasswordVerifierRequest request) =>
     {
-        var passwordVerifyGrain = grainFactory.GetGrain<ISyncWorker<PasswordVerifierRequest, PasswordVerifierResult>>(Guid.NewGuid());
-        return await passwordVerifyGrain.StartWorkAndPollUntilResult(request);
+        try
+        {
+            var passwordVerifyGrain = grainFactory.GetGrain<ISyncWorker<PasswordVerifierRequest, PasswordVerifierResult>>(Guid.NewGuid());
+            return await passwordVerifyGrain.StartWorkAndPollUntilResult(request);
+        }
+        catch (Exception e)
+        {
+            app.Logger.LogError(e, "Just demonstrating you can catch an exception thrown by the grain");
+            throw;
+        }
     })
     .WithName("GetPasswordVerify");
 

samples/Orleans.SyncWork.Demo.Services/TestGrains/GrainThatWaitsSetTimePriorToSuccessResultBecomingAvailable.cs

@@ -14,8 +14,6 @@ public class TestDelaySuccessResult
 {
     public DateTime Started { get; set; }
     public DateTime Ended { get; set; }
-
-    public TimeSpan Duration => Ended - Started;
 }
 
 public class GrainThatWaitsSetTimePriorToSuccessResultBecomingAvailable : SyncWorker<TestDelaySuccessRequest, TestDelaySuccessResult>

src/Orleans.SyncWork/Exceptions/InvalidStateException.cs

@@ -10,12 +10,8 @@ namespace Orleans.SyncWork.Exceptions;
 /// </summary>
 public class InvalidStateException : Exception
 {
-    SyncWorkStatus ActualStatus { get; }
-    SyncWorkStatus ExpectedStatus { get; }
-
-    public InvalidStateException(SyncWorkStatus actualStatus, SyncWorkStatus expectedStatus) : base()
+    public InvalidStateException(SyncWorkStatus actualStatus, SyncWorkStatus expectedStatus) : base(
+        $"Grain was in an invalid state for the requested grain method.  Expected status {expectedStatus}, got {actualStatus}.")
     {
-        ActualStatus = actualStatus;
-        ExpectedStatus = expectedStatus;
     }
 }

src/Orleans.SyncWork/ExtensionMethods/SiloHostBuilderExtensions.cs

@@ -1,7 +1,4 @@
-using System;
-using System.Linq;
-using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.DependencyInjection;
 using Orleans.Hosting;
 
 namespace Orleans.SyncWork.ExtensionMethods;
@@ -36,54 +33,4 @@ public static ISiloHostBuilder ConfigureSyncWorkAbstraction(this ISiloHostBuilde
 
         return builder;
     }
-
-    /// <summary>
-    /// Registers the required <see cref="LimitedConcurrencyLevelTaskScheduler"/>, and scans marker assemblies for the
-    /// automatic registration of implementations against <see cref="ISyncWorker{TRequest,TResult}"/>.
-    /// </summary>
-    /// <param name="builder">The <see cref="ISiloHostBuilder"/> instance.</param>
-    /// <param name="maxSyncWorkConcurrency">
-    ///     The maximum amount of concurrent work to perform at a time.  
-    ///     The CPU cannot be "tapped out" with concurrent work lest Orleans experience timeouts.
-    /// </param>
-    /// <param name="markers">
-    ///     The assemblies to scan to find implementations of <see cref="ISyncWorker{TRequest,TResult}"/> to register.
-    /// </param>
-    /// <remarks>
-    /// 
-    ///     A "general" rule of thumb that I've had success with is using "Environment.ProcessorCount - 2" as the max concurrency.
-    /// 
-    /// </remarks>
-    /// <returns>The <see cref="ISiloHostBuilder"/> to allow additional fluent API chaining.</returns>
-    public static ISiloHostBuilder ConfigureSyncWorkAbstraction(this ISiloHostBuilder builder,
-        int maxSyncWorkConcurrency = 4, params Type[] markers)
-    {
-        builder.ConfigureApplicationParts(parts => parts.AddApplicationPart(typeof(ISyncWorkAbstractionMarker).Assembly).WithReferences());
-
-        builder.ConfigureServices(services =>
-        {
-            services.AddSingleton(_ => new LimitedConcurrencyLevelTaskScheduler(maxSyncWorkConcurrency));
-        });
-
-        foreach (var marker in markers)
-        {
-            var assembly = marker.Assembly;
-            var grainImplementations = assembly.ExportedTypes
-                .Where(type =>
-                {
-                    var genericInterfaceTypes = type.GetInterfaces().Where(x => x.IsGenericType).ToList();
-                    var implementationSyncWorkType = genericInterfaceTypes
-                        .Any(x => x.GetGenericTypeDefinition() == typeof(ISyncWorker<,>));
-
-                    return !type.IsInterface && !type.IsAbstract && implementationSyncWorkType;
-                }).ToList();
-
-            var serviceDescriptors = grainImplementations.Select(grainImplementation =>
-                new ServiceDescriptor(grainImplementation, grainImplementation, ServiceLifetime.Transient));
-
-            builder.ConfigureServices(x => x.TryAdd(serviceDescriptors));
-        }
-
-        return builder;
-    }
 }

src/Orleans.SyncWork/SyncWorker.cs

@@ -58,7 +58,7 @@ public Task<Exception> GetException()
     {
         if (_status != SyncWorkStatus.Faulted)
         {
-            _logger.LogError($"{nameof(this.GetException)}: Attempting to retrieve exception from grain when grain not in a faulted state ({_status}).");
+            _logger.LogError("{nameof(this.GetException)}: Attempting to retrieve exception from grain when grain not in a faulted state ({_status}).", nameof(this.GetException), _status);
             throw new InvalidStateException(_status, SyncWorkStatus.Faulted);
         }
 
@@ -73,7 +73,7 @@ public Task<TResult> GetResult()
     {
         if (_status != SyncWorkStatus.Completed)
         {
-            _logger.LogError($"{nameof(this.GetResult)}: Attempting to retrieve result from grain when grain not in a completed state ({_status}).");
+            _logger.LogError("{nameof(this.GetResult)}: Attempting to retrieve result from grain when grain not in a completed state ({_status}).", nameof(this.GetResult), _status);
             throw new InvalidStateException(_status, SyncWorkStatus.Completed);
         }
 

test/Orleans.SyncWork.Tests/ExtensionMethods/SiloHostBuilderExtensionsTests.cs

@@ -0,0 +1,27 @@
+using FluentAssertions;
+using Microsoft.Extensions.Hosting;
+using Orleans.Hosting;
+using Xunit;
+
+namespace Orleans.SyncWork.Tests.ExtensionMethods;
+
+public class SiloHostBuilderExtensionsTests
+{
+    [Theory]
+    [InlineData(4)]
+    [InlineData(8)]
+    public void WhenCallingConfigure_ShouldRegisterLimitedConcurrencyScheduler(int maxSyncWorkConcurrency)
+    {
+        var builder = new SiloHostBuilder();
+        Orleans.SyncWork.ExtensionMethods.SiloHostBuilderExtensions.ConfigureSyncWorkAbstraction(builder, maxSyncWorkConcurrency);
+
+        builder.UseLocalhostClustering();
+
+        var host = builder.Build();
+        var scheduler = (LimitedConcurrencyLevelTaskScheduler)host.Services.GetService(typeof(LimitedConcurrencyLevelTaskScheduler));
+
+        scheduler.Should().NotBeNull("the extension method was to registered the scheduler");
+        scheduler?.MaximumConcurrencyLevel.Should().Be(maxSyncWorkConcurrency,
+            "the scheduler should have the registered level of maximum concurrency");
+    }
+}

test/Orleans.SyncWork.Tests/LimitedConcurrencyLevelTaskSchedulerTests.cs

@@ -0,0 +1,31 @@
+using System;
+using FluentAssertions;
+using Xunit;
+
+namespace Orleans.SyncWork.Tests;
+
+public class LimitedConcurrencyLevelTaskSchedulerTests
+{
+    [Theory]
+    [InlineData(1)]
+    [InlineData(4)]
+    public void WhenProvidedConcurrencyValueAtConstruction_ShouldContainThatLevelOfMaxConcurrency(int maxDegreeOfParallelism)
+    {
+        var subject = new LimitedConcurrencyLevelTaskScheduler(maxDegreeOfParallelism);
+
+        subject.MaximumConcurrencyLevel.Should().Be(maxDegreeOfParallelism);
+    }
+
+    [Theory]
+    [InlineData(0)]
+    [InlineData(-42)]
+    public void WhenProvidedConcurrencyAtOrBelowZero_ShouldThrow(int maxDegreeOfParallelism)
+    {
+        Action action = () =>
+        {
+            _ = new LimitedConcurrencyLevelTaskScheduler(maxDegreeOfParallelism);
+        };
+
+        action.Should().Throw<ArgumentOutOfRangeException>();
+    }
+}