Merge pull request #1 from jolexxa/test/gc-scenarios

test: gc scenarios

Chickensoft.AutoInject.Tests/coverage.sh

@@ -26,7 +26,7 @@ dotnet build --no-restore
 
 coverlet \
   "./.godot/mono/temp/bin/Debug" --verbosity detailed \
-  --target $GODOT \
+  --target "$GODOT" \
   --targetargs "--headless --run-tests --coverage --quit-on-finish" \
   --format "opencover" \
   --output "./coverage/coverage.xml" \

Chickensoft.AutoInject.Tests/src/auto_inject/dependent/DependencyResolver.cs

@@ -286,42 +286,44 @@ void onProviderInitialized(IBaseProvider provider) {
   }
 
   public class DefaultProvider<TValue> : IBaseProvider {
-    private object _value;
-    private readonly Func<TValue>? _fallback;
+    internal object _value;
+    private readonly Func<TValue?> _fallback;
     public ProviderState ProviderState { get; }
 
-    // When working with reference types, we must wrap the value in a WeakReference<>()
-    // to allow the garbage collection to work when the assembly is being unloaded or reloaded;
-    // such as in the case of rebuilding within the Godot Editor if you've instantiated a node
+    // When working with reference types, we must wrap the value in a
+    // WeakReference() to allow the garbage collection to work when the
+    // assembly is being unloaded or reloaded; such as in the case of
+    // rebuilding within the Godot Editor if you've instantiated a node
     // and run it as a tool script.
-    public DefaultProvider(object value, Func<TValue>? fallback = default) {
-      _fallback = fallback;
+    public DefaultProvider(object value, Func<TValue?>? fallback = default) {
+      _fallback = fallback ?? (() => (TValue?)value);
+
+      _value = value.GetType().IsValueType
+        ? value
+        : new WeakReference(value);
 
-      _value = value.GetType().IsValueType ? value : new WeakReference<object>(value);
       ProviderState = new() { IsInitialized = true };
     }
 
     public TValue Value() {
-      if (_value is WeakReference<object> weakReference) {
+      if (_value is WeakReference weakReference) {
         // Try to return a reference type.
-        if (weakReference.TryGetTarget(out var target)) {
-          return (TValue)target;
+        if (weakReference.Target is TValue target) {
+          return target;
         }
-        else {
-          // If value was garbage collected, make a new one.
-          if (_fallback == null) {
-            throw new InvalidOperationException("Fallback method is null.");
-          }
 
-          var value = _fallback() ?? throw new InvalidOperationException("Fallback cannot create a null value");
-          _value = new WeakReference<object>(value);
-          return value;
-        }
-      }
-      else {
-        // Return a value type.
-        return (TValue)_value;
+        var value = _fallback() ??
+          throw new InvalidOperationException(
+            "Fallback cannot create a null value"
+          );
+
+        _value = new WeakReference(value);
+
+        return value;
+
       }
+      // Return a value type.
+      return (TValue)_value;
     }
   }
 }

Chickensoft.AutoInject.Tests/test/fixtures/Dependents.cs

@@ -1,5 +1,6 @@
 namespace Chickensoft.AutoInject.Tests.Subjects;
 
+using System;
 using Chickensoft.Introspection;
 using Godot;
 
@@ -56,6 +57,21 @@ public void OnResolved() {
   }
 }
 
+[Meta(typeof(IAutoOn), typeof(IDependent))]
+public partial class WeakReferenceDependent : Node {
+  public override void _Notification(int what) => this.Notify(what);
+
+  [Dependency]
+  public object MyDependency => this.DependOn(Fallback);
+
+  public Func<object>? Fallback { get; set; }
+  public bool OnResolvedCalled { get; private set; }
+
+  public void OnReady() { }
+
+  public void OnResolved() => OnResolvedCalled = true;
+}
+
 [Meta(typeof(IAutoOn), typeof(IDependent))]
 public partial class ReferenceDependentFallback : Node {
   public override void _Notification(int what) => this.Notify(what);
@@ -80,10 +96,11 @@ public partial class IntDependent : Node {
   public override void _Notification(int what) => this.Notify(what);
 
   [Dependency]
-  public int MyDependency => this.DependOn<int>();
+  public int MyDependency => this.DependOn(FallbackValue);
 
   public bool OnResolvedCalled { get; private set; }
   public int ResolvedValue { get; set; }
+  public Func<int>? FallbackValue { get; set; }
 
   public void OnReady() { }
 

Chickensoft.AutoInject.Tests/test/src/ResolutionTest.cs

@@ -1,5 +1,7 @@
 namespace Chickensoft.AutoInject.Tests;
 
+using System;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using Chickensoft.AutoInject.Tests.Subjects;
 using Chickensoft.GoDotTest;
@@ -131,9 +133,9 @@ public void ThrowsWhenNoProviderFound() {
   }
 
   [Test]
-  public void UsesValueFallbackValueWhenNoProviderFound() {
-    var fallback = "Hello, world!";
-    var dependent = new StringDependentFallback {
+  public void UsesReferenceFallbackValueWhenNoProviderFound() {
+    var fallback = new Resource();
+    var dependent = new ReferenceDependentFallback {
       FallbackValue = fallback
     };
 
@@ -144,16 +146,73 @@ public void UsesValueFallbackValueWhenNoProviderFound() {
   }
 
   [Test]
-  public void UsesReferenceFallbackValueWhenNoProviderFound() {
-    var fallback = new Resource();
-    var dependent = new ReferenceDependentFallback {
-      FallbackValue = fallback
+  public void DependsOnValueType() {
+    var value = 10;
+    var depObj = new IntDependent() { FallbackValue = () => value };
+    var dependent = depObj as IDependent;
+
+    depObj._Notification((int)Node.NotificationReady);
+
+
+    depObj.OnResolvedCalled.ShouldBeTrue();
+    depObj.ResolvedValue.ShouldBe(value);
+
+    depObj._Notification((int)Node.NotificationExitTree);
+
+    dependent.DependentState.Pending.ShouldBeEmpty();
+
+    depObj.QueueFree();
+  }
+
+  [Test]
+  public void ThrowsIfFallbackProducesNullAfterPreviousValueIsGarbageCollected(
+  ) {
+    var currentFallback = 0;
+    var replacementValue = new object();
+    var fallbacks = new List<object?>() { new(), null, replacementValue };
+
+    var value = Utils.CreateWeakReference();
+
+    // Fallback will be called 3 times in this test. First will be non-null,
+    // second will be null, third will be non-null and different from the first.
+    var depObj = new WeakReferenceDependent() {
+      Fallback = () => fallbacks[currentFallback++]!
     };
 
-    dependent._Notification((int)Node.NotificationReady);
+    var dependent = depObj as IDependent;
 
-    dependent.ResolvedValue.ShouldBe(fallback);
-    dependent.MyDependency.ShouldBe(fallback);
+    depObj._Notification((int)Node.NotificationReady);
+
+    // Let's access the fallback value to ensure the default provider is setup.
+    depObj.MyDependency.ShouldNotBeNull();
+
+    // Simulate a garbage collected object. We support weak references to
+    // dependencies to avoid causing build issues when reloading the scene.
+    Utils.ClearWeakReference(value);
+
+    // To test this highly specific scenario, we have to clear ALL
+    // weak references to the object, including the one in the default provider
+    // that's generated behind-the-scenes for us.
+
+    // Let's dig out the weak ref used in the default provider from the
+    // dependent's internal state...
+    var underlyingDefaultProvider =
+      (DependencyResolver.DefaultProvider<object>)
+        depObj.MixinState.Get<DependentState>().Dependencies[typeof(object)];
+
+    var actualWeakRef = (WeakReference)underlyingDefaultProvider._value;
+
+    Utils.ClearWeakReference(actualWeakRef);
+
+    var e = Should.Throw<InvalidOperationException>(
+      () => depObj.MyDependency
+    );
+
+    e.Message.ShouldContain("cannot create a null value");
+
+    // Now that the fallback returns a valid value, the dependency should
+    // be resolved once again.
+    depObj.MyDependency.ShouldBeSameAs(replacementValue);
   }
 
   [Test]
@@ -246,4 +305,17 @@ public BadProvider() {
       };
     }
   }
+
+  public static class Utils {
+    public static WeakReference CreateWeakReference() => new(new object());
+
+    public static void ClearWeakReference(WeakReference weakReference) {
+      weakReference.Target = null;
+
+      while (weakReference.Target is not null) {
+        GC.Collect();
+        GC.WaitForPendingFinalizers();
+      }
+    }
+  }
 }

cspell.json

@@ -13,9 +13,6 @@
     "Chickensoft.AutoInject/nupkg/**/*.*"
   ],
   "words": [
-    "devbuild",
-    "mktemp",
-    "skipautoprops",
     "assemblyfilters",
     "automerge",
     "branchcoverage",
@@ -25,7 +22,9 @@
     "Chickensoft",
     "classfilters",
     "CYGWIN",
+    "devbuild",
     "endregion",
+    "Finalizers",
     "globaltool",
     "godotengine",
     "godotpackage",
@@ -35,6 +34,7 @@
     "Metatype",
     "methodcoverage",
     "missingall",
+    "mktemp",
     "msbuild",
     "MSYS",
     "nameof",
@@ -52,6 +52,7 @@
     "reportgenerator",
     "reporttypes",
     "Shouldly",
+    "skipautoprops",
     "subfolders",
     "targetargs",
     "targetdir",