Skip to content

Commit caeaeed

Browse files
committed
Fix correctness and perf issues in compare, recording, naming and checks
- StreamComparer: compare both streams' read counts and return NotEqual on a length difference, closing a silent false-pass for non-seekable/empty received streams - Callbacks: await every registered OnFirstVerify/OnDelete/OnVerifyMismatch handler via GetInvocationList instead of only the last (static and instance) - Recording.TryStop: snapshot the items then Clear and Pause the shared State, so a stop consumed inside the engine's async flow is observable to the test (no re-appending on a second verify, IsRecording reflects it) - Recording: Disposable/NamedDisposable use TryPause so disposing after an explicit Stop is a no-op instead of throwing - Recording.IgnoreNames: copy-on-write HashSet read via Volatile.Read, so IsIgnored on Recording.Add cannot race a concurrent mutation - MatchingFileFinder: account for a trailing separator on the directory so UseDirectory("snapshots/") still finds verified files and cleans received - InnerVerifyChecks: look for .gitignore (was .gitIgnore), so the check runs on case-sensitive file systems; message updated to match - Naming: pass pathFriendly through to collection items so pathFriendly:false callers keep path chars raw in snapshot content - DanglingSnapshotsCheck: materialize tracked files into HashSets once instead of LINQ Contains over a ConcurrentBag per file (O(files*tracked) -> O(files)) - Add tests for the above; update the two VerifyChecksTests snapshots for the .gitignore casing
1 parent 72503b3 commit caeaeed

16 files changed

Lines changed: 260 additions & 36 deletions

src/Verify.Tests/CallbackTests.cs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
public class CallbackTests
2+
{
3+
[Fact]
4+
public async Task OnFirstVerifyAwaitsAllHandlers()
5+
{
6+
var settings = new VerifySettings();
7+
var ran = new List<int>();
8+
// The slower handler is registered first: if only the last handler is
9+
// awaited (the bug), it will not have completed by the assert.
10+
settings.OnFirstVerify(
11+
async (_, _, _) =>
12+
{
13+
await Task.Delay(200);
14+
ran.Add(1);
15+
});
16+
settings.OnFirstVerify(
17+
async (_, _, _) =>
18+
{
19+
await Task.Delay(1);
20+
ran.Add(2);
21+
});
22+
23+
var filePair = new FilePair("txt", "received.txt", "verified.txt");
24+
var result = new NewResult(filePair, null);
25+
await settings.RunOnFirstVerify(result, false);
26+
27+
Assert.Equal(2, ran.Count);
28+
}
29+
}

src/Verify.Tests/InnerVerifyChecksTests/VerifyChecksTests.GitIgnore.verified.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
{
22
Type: VerifyCheckException,
33
Message:
4-
Expected .gitIgnore to contain settings for Verify.
5-
Path: file:///{ProjectDirectory}InnerVerifyChecksTests/Invalid/.gitIgnore
4+
Expected .gitignore to contain settings for Verify.
5+
Path: file:///{ProjectDirectory}InnerVerifyChecksTests/Invalid/.gitignore
66
Recommended settings:
77

88
# Verify

src/Verify.Tests/InnerVerifyChecksTests/VerifyChecksTests.Invalid.verified.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
{
22
Type: VerifyCheckException,
33
Message:
4-
Expected .gitIgnore to contain settings for Verify.
5-
Path: file:///{ProjectDirectory}InnerVerifyChecksTests/Invalid/.gitIgnore
4+
Expected .gitignore to contain settings for Verify.
5+
Path: file:///{ProjectDirectory}InnerVerifyChecksTests/Invalid/.gitignore
66
Recommended settings:
77

88
# Verify
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
public class MatchingFileFinderTests
2+
{
3+
[Fact]
4+
public void FindVerifiedWithTrailingSeparator()
5+
{
6+
var directory = CreateTempDirectory();
7+
try
8+
{
9+
File.WriteAllText(Path.Combine(directory, "SomeTest.verified.txt"), "a");
10+
File.WriteAllText(Path.Combine(directory, "Other.verified.txt"), "b");
11+
12+
var withSeparator = directory + Path.DirectorySeparatorChar;
13+
var found = MatchingFileFinder.FindVerified("SomeTest", withSeparator)
14+
.ToList();
15+
16+
Assert.Single(found);
17+
Assert.EndsWith("SomeTest.verified.txt", found[0]);
18+
}
19+
finally
20+
{
21+
Directory.Delete(directory, true);
22+
}
23+
}
24+
25+
[Fact]
26+
public void FindVerifiedWithoutTrailingSeparator()
27+
{
28+
var directory = CreateTempDirectory();
29+
try
30+
{
31+
File.WriteAllText(Path.Combine(directory, "SomeTest.verified.txt"), "a");
32+
File.WriteAllText(Path.Combine(directory, "Other.verified.txt"), "b");
33+
34+
var found = MatchingFileFinder.FindVerified("SomeTest", directory)
35+
.ToList();
36+
37+
Assert.Single(found);
38+
}
39+
finally
40+
{
41+
Directory.Delete(directory, true);
42+
}
43+
}
44+
45+
static string CreateTempDirectory()
46+
{
47+
var directory = Path.Combine(Path.GetTempPath(), $"MatchingFileFinder{Guid.NewGuid():N}");
48+
Directory.CreateDirectory(directory);
49+
return directory;
50+
}
51+
}

src/Verify.Tests/Naming/NameForParameterTests.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,21 @@ public Task StringEmpty() =>
1212
public Task StringInvalidPathChar() =>
1313
Verify(VerifierSettings.GetNameForParameter("a/a", counter: CounterBuilder.Empty()));
1414

15+
[Fact]
16+
public void CollectionItemPathFriendlyFalseNotCleaned()
17+
{
18+
// pathFriendly must flow to collection items, so path chars are kept
19+
// raw when the caller does not want file-name cleaning applied.
20+
var name = VerifierSettings.GetNameForParameter(
21+
new List<string>
22+
{
23+
"a/b"
24+
},
25+
counter: CounterBuilder.Empty(),
26+
pathFriendly: false);
27+
Assert.Contains("a/b", name);
28+
}
29+
1530
[Fact]
1631
public Task Int() =>
1732
Verify(VerifierSettings.GetNameForParameter(10, counter: CounterBuilder.Empty()));

src/Verify.Tests/RecordingTests.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,37 @@ public Task IgnoreNames()
1010
return Verify();
1111
}
1212

13+
[Fact]
14+
public async Task StoppedInChildContextIsNotRecording()
15+
{
16+
Recording.Start();
17+
Recording.Add("name", "value");
18+
// Consuming inside a child execution context (as the verify engine does)
19+
// must not leave the recording active in this context.
20+
await Task.Run(() => Recording.TryStop(out _));
21+
Assert.False(Recording.IsRecording());
22+
}
23+
24+
[Fact]
25+
public void DisposeAfterStopDoesNotThrow()
26+
{
27+
using (Recording.Start())
28+
{
29+
Recording.Add("name", "value");
30+
Recording.Stop();
31+
}
32+
}
33+
34+
[Fact]
35+
public void DisposeNamedAfterStopDoesNotThrow()
36+
{
37+
using (Recording.Start("DisposeNamedAfterStop"))
38+
{
39+
Recording.Add("DisposeNamedAfterStop", "name", "value");
40+
Recording.Stop("DisposeNamedAfterStop");
41+
}
42+
}
43+
1344
[Fact]
1445
public Task Dates()
1546
{

src/Verify.Tests/StreamComparerTests.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,34 @@ public async Task BinaryNotEquals()
3535
Assert.False(result.IsEqual);
3636
}
3737

38+
[Fact]
39+
public async Task PrefixIsNotEqual()
40+
{
41+
// received is an 8-byte-aligned prefix of verified; must not be Equal.
42+
using var received = new MemoryStream(new byte[16]);
43+
using var verified = new MemoryStream(new byte[32]);
44+
var result = await StreamComparer.AreEqual(received, verified);
45+
Assert.False(result.IsEqual);
46+
}
47+
48+
[Fact]
49+
public async Task EmptyReceivedIsNotEqual()
50+
{
51+
using var received = new MemoryStream();
52+
using var verified = new MemoryStream(new byte[16]);
53+
var result = await StreamComparer.AreEqual(received, verified);
54+
Assert.False(result.IsEqual);
55+
}
56+
57+
[Fact]
58+
public async Task LongerReceivedIsNotEqual()
59+
{
60+
using var received = new MemoryStream(new byte[32]);
61+
using var verified = new MemoryStream(new byte[16]);
62+
var result = await StreamComparer.AreEqual(received, verified);
63+
Assert.False(result.IsEqual);
64+
}
65+
3866
[Fact]
3967
public async Task ShouldNotLock()
4068
{

src/Verify/Callbacks/VerifierSettings.cs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,20 @@ public static void OnFirstVerify(FirstVerify firstVerify)
3535
handleOnFirstVerify += firstVerify;
3636
}
3737

38-
internal static Task RunOnFirstVerify(NewResult item, bool autoVerify)
38+
internal static async Task RunOnFirstVerify(NewResult item, bool autoVerify)
3939
{
4040
if (handleOnFirstVerify is null)
4141
{
42-
return Task.CompletedTask;
42+
return;
4343
}
4444

45-
return handleOnFirstVerify(item.File, item.ReceivedText?.ToString(), autoVerify);
45+
var receivedText = item.ReceivedText?.ToString();
46+
// Await every registered handler; invoking the multicast delegate
47+
// directly would only await the last handler's Task.
48+
foreach (var handler in handleOnFirstVerify.GetInvocationList())
49+
{
50+
await ((FirstVerify) handler)(item.File, receivedText, autoVerify);
51+
}
4652
}
4753

4854
static VerifyDelete? handleOnVerifyDelete;
@@ -55,24 +61,30 @@ public static void OnDelete(VerifyDelete verifyDelete)
5561

5662
static VerifyMismatch? handleOnVerifyMismatch;
5763

58-
internal static Task RunOnVerifyDelete(string file, bool autoVerify)
64+
internal static async Task RunOnVerifyDelete(string file, bool autoVerify)
5965
{
6066
if (handleOnVerifyDelete is null)
6167
{
62-
return Task.CompletedTask;
68+
return;
6369
}
6470

65-
return handleOnVerifyDelete(file, autoVerify);
71+
foreach (var handler in handleOnVerifyDelete.GetInvocationList())
72+
{
73+
await ((VerifyDelete) handler)(file, autoVerify);
74+
}
6675
}
6776

68-
internal static Task RunOnVerifyMismatch(FilePair item, string? message, bool autoVerify)
77+
internal static async Task RunOnVerifyMismatch(FilePair item, string? message, bool autoVerify)
6978
{
7079
if (handleOnVerifyMismatch is null)
7180
{
72-
return Task.CompletedTask;
81+
return;
7382
}
7483

75-
return handleOnVerifyMismatch(item, message, autoVerify);
84+
foreach (var handler in handleOnVerifyMismatch.GetInvocationList())
85+
{
86+
await ((VerifyMismatch) handler)(item, message, autoVerify);
87+
}
7688
}
7789

7890
public static void OnVerifyMismatch(VerifyMismatch verifyMismatch)

src/Verify/Callbacks/VerifySettings.cs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ internal async Task RunOnFirstVerify(NewResult item, bool autoVerify)
1010
{
1111
if (handleOnFirstVerify is not null)
1212
{
13-
await handleOnFirstVerify(item.File, item.ReceivedText?.ToString(), autoVerify);
13+
var receivedText = item.ReceivedText?.ToString();
14+
foreach (var handler in handleOnFirstVerify.GetInvocationList())
15+
{
16+
await ((FirstVerify) handler)(item.File, receivedText, autoVerify);
17+
}
1418
}
1519

1620
await VerifierSettings.RunOnFirstVerify(item, autoVerify);
@@ -26,7 +30,10 @@ internal async Task RunOnVerifyDelete(string file, bool autoVerify)
2630
{
2731
if (handleOnVerifyDelete is not null)
2832
{
29-
await handleOnVerifyDelete(file, autoVerify);
33+
foreach (var handler in handleOnVerifyDelete.GetInvocationList())
34+
{
35+
await ((VerifyDelete) handler)(file, autoVerify);
36+
}
3037
}
3138

3239
await VerifierSettings.RunOnVerifyDelete(file, autoVerify);
@@ -38,7 +45,10 @@ internal async Task RunOnVerifyMismatch(FilePair item, string? message, bool aut
3845
{
3946
if (handleOnVerifyMismatch is not null)
4047
{
41-
await handleOnVerifyMismatch(item, message, autoVerify);
48+
foreach (var handler in handleOnVerifyMismatch.GetInvocationList())
49+
{
50+
await ((VerifyMismatch) handler)(item, message, autoVerify);
51+
}
4252
}
4353

4454
await VerifierSettings.RunOnVerifyMismatch(item, message, autoVerify);

src/Verify/Compare/StreamComparer.cs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,23 @@ public static async Task<CompareResult> AreEqual(Stream stream1, Stream stream2)
1414

1515
while (true)
1616
{
17-
var count = await ReadBufferAsync(stream1, buffer1);
17+
var count1 = await ReadBufferAsync(stream1, buffer1);
18+
var count2 = await ReadBufferAsync(stream2, buffer2);
1819

19-
//no need to compare size here since only enter on files being same size
20+
// Callers do not always guarantee the streams are the same length
21+
// (e.g. a non-seekable received stream), so a length difference must
22+
// be treated as not-equal instead of a short-circuit to equal.
23+
if (count1 != count2)
24+
{
25+
return CompareResult.NotEqual();
26+
}
2027

21-
if (count == 0)
28+
if (count1 == 0)
2229
{
2330
return CompareResult.Equal;
2431
}
2532

26-
await ReadBufferAsync(stream2, buffer2);
27-
28-
for (var i = 0; i < count; i += sizeof(long))
33+
for (var i = 0; i < count1; i += sizeof(long))
2934
{
3035
if (BitConverter.ToInt64(buffer1, i) != BitConverter.ToInt64(buffer2, i))
3136
{

0 commit comments

Comments
 (0)