fix identifier recording shared state (#1257)

VerifyTests · Jul 26, 2024 · 3f8adb0 · 3f8adb0
1 parent c527a28
commit 3f8adb0
Show file tree

Hide file tree

Showing 8 changed files with 27 additions and 71 deletions.
diff --git a/docs/mdsource/recording.source.md b/docs/mdsource/recording.source.md
@@ -51,6 +51,8 @@ To avoid grouping use [Stop](#stop).
 
 Recording can be grouped by an identifier.
 
+The identifier should be statically unique. For example a fully qualified test name or a GUID.
+
 snippet: RecordingIdentifier
 
 Results in:

diff --git a/docs/recording.md b/docs/recording.md
@@ -117,7 +117,7 @@ public Task SameKey()
     return Verify("TheValue");
 }
 ```
-<sup><a href='/src/Verify.Tests/RecordingTests.cs#L288-L299' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingSameKey' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Verify.Tests/RecordingTests.cs#L281-L292' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingSameKey' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 Results in:
@@ -143,6 +143,8 @@ To avoid grouping use [Stop](#stop).
 
 Recording can be grouped by an identifier.
 
+The identifier should be statically unique. For example a fully qualified test name or a GUID.
+
 <!-- snippet: RecordingIdentifier -->
 <a id='snippet-RecordingIdentifier'></a>
 ```cs
@@ -186,7 +188,7 @@ public Task Case()
     return Verify("TheValue");
 }
 ```
-<sup><a href='/src/Verify.Tests/RecordingTests.cs#L310-L321' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingIgnoreCase' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Verify.Tests/RecordingTests.cs#L303-L314' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingIgnoreCase' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 Results in:
@@ -305,7 +307,7 @@ public Task Clear()
     return Verify();
 }
 ```
-<sup><a href='/src/Verify.Tests/RecordingTests.cs#L208-L220' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingClear' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Verify.Tests/RecordingTests.cs#L201-L213' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingClear' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 Results in:
@@ -341,7 +343,7 @@ public Task PauseResume()
     return Verify();
 }
 ```
-<sup><a href='/src/Verify.Tests/RecordingTests.cs#L232-L247' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingPauseResume' title='Start of snippet'>anchor</a></sup>
+<sup><a href='/src/Verify.Tests/RecordingTests.cs#L225-L240' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingPauseResume' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 Results in:

diff --git a/src/Directory.Build.props b/src/Directory.Build.props
@@ -2,7 +2,7 @@
 <Project>
   <PropertyGroup>
     <NoWarn>CS1591;CS0649;xUnit1026;xUnit1013;CS1573;VerifyTestsProjectDir;VerifySetParameters;PolyFillTargetsForNuget</NoWarn>
-    <Version>26.1.1</Version>
+    <Version>26.1.2</Version>
     <ImplicitUsings>enable</ImplicitUsings>
     <LangVersion>preview</LangVersion>
     <AssemblyVersion>1.0.0</AssemblyVersion>

diff --git a/src/Verify.Tests/RecordingTests.StartIdentifier1.verified.txt b/src/Verify.Tests/RecordingTests.StartIdentifier1.verified.txt
diff --git a/src/Verify.Tests/RecordingTests.StopIdentifier2.verified.txt b/src/Verify.Tests/RecordingTests.StopIdentifier2.verified.txt
diff --git a/src/Verify.Tests/RecordingTests.StopIdentifierOverlap.verified.txt b/src/Verify.Tests/RecordingTests.StopIdentifierOverlap.verified.txt
diff --git a/src/Verify.Tests/RecordingTests.cs b/src/Verify.Tests/RecordingTests.cs
@@ -167,18 +167,11 @@ public Task StopNotInResult()
     #endregion
 
     [Fact]
-    public void StartIdentifierThatDoesntFinish()
+    public Task StopIdentifier()
     {
-        Recording.Start("identifierOverlap");
-        Recording.Add("identifierOverlap", "name", "value");
-    }
-
-    [Fact]
-    public Task StopIdentifierOverlap()
-    {
-        Recording.Start("identifierOverlap");
-        Recording.Add("identifierOverlap", "name", "value");
-        return Verify(Recording.Stop("identifierOverlap"));
+        Recording.Start("identifier");
+        Recording.Add("identifier", "name", "value");
+        return Verify(Recording.Stop("identifier"));
     }
 
     [Fact]

diff --git a/src/Verify/Recording/Recording_Named.cs b/src/Verify/Recording/Recording_Named.cs
@@ -2,33 +2,25 @@
 
 public static partial class Recording
 {
-    static AsyncLocal<ConcurrentDictionary<string, State>?> asyncLocalNamed = new();
+    // The identifier should be statically unique. For example a fully qualified test name or a GUID.
+    // so it is ok for this to be shared state
+    static ConcurrentDictionary<string, State> namedState = new(StringComparer.OrdinalIgnoreCase);
 
     public static void Add(string identifier, string name, object item) =>
         CurrentStateNamed(identifier)
             .Add(name, item);
 
     public static void TryAdd(string identifier, string name, object item)
     {
-        var states = asyncLocalNamed.Value;
-        if (states != null)
+        if (namedState.TryGetValue(identifier, out var state))
         {
-            if (states.TryGetValue(identifier, out var state))
-            {
-                state.Add(name, item);
-            }
+            state.Add(name, item);
         }
     }
 
     public static bool IsRecording(string identifier)
     {
-        var states = asyncLocalNamed.Value;
-        if (states == null)
-        {
-            return false;
-        }
-
-        if (!states.TryGetValue(identifier, out var state))
+        if (!namedState.TryGetValue(identifier, out var state))
         {
             return false;
         }
@@ -50,14 +42,10 @@ public static bool TryStop(
         string identifier,
         [NotNullWhen(true)] out IReadOnlyCollection<ToAppend>? recorded)
     {
-        var states = asyncLocalNamed.Value;
-        if (states != null)
+        if (namedState.TryRemove(identifier, out var state))
         {
-            if (states.TryRemove(identifier, out var state))
-            {
-                recorded = state.Items;
-                return true;
-            }
+            recorded = state.Items;
+            return true;
         }
 
         recorded = null;
@@ -66,9 +54,7 @@ public static bool TryStop(
 
     static State CurrentStateNamed(string identifier, [CallerMemberName] string caller = "")
     {
-        var states = asyncLocalNamed.Value;
-        if (states != null &&
-            states.TryGetValue(identifier, out var state))
+        if (namedState.TryGetValue(identifier, out var state))
         {
             return state;
         }
@@ -78,9 +64,7 @@ static State CurrentStateNamed(string identifier, [CallerMemberName] string call
 
     public static IDisposable Start(string identifier)
     {
-        var states = asyncLocalNamed.Value ??= new(StringComparer.OrdinalIgnoreCase);
-
-        if (!states.TryAdd(identifier, new()))
+        if (!namedState.TryAdd(identifier, new()))
         {
             throw new("Recording already started");
         }
@@ -101,13 +85,7 @@ public static void Pause(string identifier) =>
 
     public static void TryPause(string identifier)
     {
-        var states = asyncLocalNamed.Value;
-        if (states == null)
-        {
-            return;
-        }
-
-        if (states.TryGetValue(identifier, out var state))
+        if (namedState.TryGetValue(identifier, out var state))
         {
             state.Pause();
         }
@@ -119,13 +97,7 @@ public static void Resume(string identifier) =>
 
     public static void TryResume(string identifier)
     {
-        var states = asyncLocalNamed.Value;
-        if (states == null)
-        {
-            return;
-        }
-
-        if (states.TryGetValue(identifier, out var state))
+        if (namedState.TryGetValue(identifier, out var state))
         {
             state.Resume();
         }
@@ -137,13 +109,7 @@ public static void Clear(string identifier) =>
 
     public static void TryClear(string identifier)
     {
-        var states = asyncLocalNamed.Value;
-        if (states == null)
-        {
-            return;
-        }
-
-        if (states.TryGetValue(identifier, out var state))
+        if (namedState.TryGetValue(identifier, out var state))
         {
             state.Clear();
         }