Skip to content

Commit ff42d46

Browse files
authored
Release named recording state on dispose (#1898)
The named disposable only paused, so a scope left without an explicit Stop, for example because the test threw, left the State and every object it had recorded reachable forever through the static namedState dictionary. The unnamed variant is reclaimed with its execution context, but named state has nothing to reclaim it. It also blocked the identifier: identifiers are documented as statically unique, such as a fully qualified test name, so the next Start with the same one threw 'Recording already started', masking the original failure. Dispose now removes the entry, which stays a no-op after an explicit Stop.
1 parent a2d1a6c commit ff42d46

3 files changed

Lines changed: 48 additions & 14 deletions

File tree

docs/recording.md

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public Task Usage()
2525
return Verify("TheValue");
2626
}
2727
```
28-
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L54-L64' title='Snippet source file'>snippet source</a> | <a href='#snippet-Recording' title='Start of snippet'>anchor</a></sup>
28+
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L84-L94' title='Snippet source file'>snippet source</a> | <a href='#snippet-Recording' title='Start of snippet'>anchor</a></sup>
2929
<!-- endSnippet -->
3030

3131
Results in:
@@ -61,7 +61,7 @@ public Task TryAdd()
6161
return Verify("TheValue");
6262
}
6363
```
64-
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L90-L102' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingTryAdd' title='Start of snippet'>anchor</a></sup>
64+
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L120-L132' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingTryAdd' title='Start of snippet'>anchor</a></sup>
6565
<!-- endSnippet -->
6666

6767

@@ -85,7 +85,7 @@ public Task RecordingScoped()
8585
return Verify();
8686
}
8787
```
88-
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L113-L128' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingScoped' title='Start of snippet'>anchor</a></sup>
88+
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L143-L158' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingScoped' title='Start of snippet'>anchor</a></sup>
8989
<!-- endSnippet -->
9090

9191
Results in:
@@ -117,7 +117,7 @@ public Task SameKey()
117117
return Verify("TheValue");
118118
}
119119
```
120-
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L312-L323' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingSameKey' title='Start of snippet'>anchor</a></sup>
120+
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L342-L353' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingSameKey' title='Start of snippet'>anchor</a></sup>
121121
<!-- endSnippet -->
122122

123123
Results in:
@@ -156,7 +156,7 @@ public Task Identifier()
156156
return Verify(Recording.Stop("identifier"));
157157
}
158158
```
159-
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L130-L140' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingIdentifier' title='Start of snippet'>anchor</a></sup>
159+
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L160-L170' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingIdentifier' title='Start of snippet'>anchor</a></sup>
160160
<!-- endSnippet -->
161161

162162
Results in:
@@ -188,7 +188,7 @@ public Task Case()
188188
return Verify("TheValue");
189189
}
190190
```
191-
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L334-L345' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingIgnoreCase' title='Start of snippet'>anchor</a></sup>
191+
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L364-L375' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingIgnoreCase' title='Start of snippet'>anchor</a></sup>
192192
<!-- endSnippet -->
193193

194194
Results in:
@@ -223,7 +223,7 @@ public Task Stop()
223223
return Verify(appends.Where(_ => _.Name != "name1"));
224224
}
225225
```
226-
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L172-L184' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingStop' title='Start of snippet'>anchor</a></sup>
226+
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L202-L214' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingStop' title='Start of snippet'>anchor</a></sup>
227227
<!-- endSnippet -->
228228

229229
Results in:
@@ -255,7 +255,7 @@ public Task StopNotInResult()
255255
return Verify("other data");
256256
}
257257
```
258-
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L186-L198' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingStopNotInResult' title='Start of snippet'>anchor</a></sup>
258+
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L216-L228' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingStopNotInResult' title='Start of snippet'>anchor</a></sup>
259259
<!-- endSnippet -->
260260

261261
Results in:
@@ -284,7 +284,7 @@ public void IsRecording()
284284
Assert.True(Recording.IsRecording());
285285
}
286286
```
287-
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L142-L152' title='Snippet source file'>snippet source</a> | <a href='#snippet-IsRecording' title='Start of snippet'>anchor</a></sup>
287+
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L172-L182' title='Snippet source file'>snippet source</a> | <a href='#snippet-IsRecording' title='Start of snippet'>anchor</a></sup>
288288
<!-- endSnippet -->
289289

290290
This can be helpful if the cost of capturing data, to add to recording, is high.
@@ -307,7 +307,7 @@ public Task Clear()
307307
return Verify();
308308
}
309309
```
310-
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L232-L244' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingClear' title='Start of snippet'>anchor</a></sup>
310+
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L262-L274' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingClear' title='Start of snippet'>anchor</a></sup>
311311
<!-- endSnippet -->
312312

313313
Results in:
@@ -343,7 +343,7 @@ public Task PauseResume()
343343
return Verify();
344344
}
345345
```
346-
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L256-L271' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingPauseResume' title='Start of snippet'>anchor</a></sup>
346+
<sup><a href='/src/Verify.Tests/RecordingTests.cs#L286-L301' title='Snippet source file'>snippet source</a> | <a href='#snippet-RecordingPauseResume' title='Start of snippet'>anchor</a></sup>
347347
<!-- endSnippet -->
348348

349349
Results in:

src/Verify.Tests/RecordingTests.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,36 @@ public void DisposeNamedAfterStopDoesNotThrow()
4141
}
4242
}
4343

44+
// Named state lives in a static dictionary, so a scope left without an explicit Stop,
45+
// for example because the test threw, would otherwise hold it and its recorded objects
46+
// forever, and block the identifier for every later Start.
47+
[Fact]
48+
public void DisposeNamedReleasesTheIdentifier()
49+
{
50+
try
51+
{
52+
using (Recording.Start("DisposeNamedReleases"))
53+
{
54+
Recording.Add("DisposeNamedReleases", "name", "value");
55+
throw new("test failure");
56+
}
57+
}
58+
catch (Exception exception)
59+
when (exception.Message == "test failure")
60+
{
61+
}
62+
63+
Assert.False(Recording.IsRecording("DisposeNamedReleases"));
64+
65+
// The identifier is free again
66+
using (Recording.Start("DisposeNamedReleases"))
67+
{
68+
Recording.Add("DisposeNamedReleases", "name", "value2");
69+
var recorded = Recording.Stop("DisposeNamedReleases");
70+
Assert.Equal(["value2"], recorded.Select(_ => _.Data));
71+
}
72+
}
73+
4474
[Fact]
4575
public Task Dates()
4676
{

src/Verify/Recording/Recording_Named.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,14 @@ public static IDisposable Start(string identifier)
7575
class NamedDisposable(string identifier) :
7676
IDisposable
7777
{
78-
// TryPause (not Pause) so disposing after an explicit Stop is a no-op
79-
// rather than throwing from CurrentStateNamed.
78+
// Removed, not paused. Named state lives in a static dictionary, so unlike the
79+
// unnamed variant nothing else can reclaim it: leaving it there holds every
80+
// recorded object for the life of the process, and makes a later Start with the
81+
// same identifier throw "Recording already started". Identifiers are meant to be
82+
// statically unique, so that happens across theory cases or an in process re-run.
83+
// A no-op after an explicit Stop, which has already removed it.
8084
public void Dispose() =>
81-
TryPause(identifier);
85+
namedState.TryRemove(identifier, out _);
8286
}
8387

8488
public static void Pause(string identifier) =>

0 commit comments

Comments
 (0)