Skip to content

Commit b99e554

Browse files
JusterZhuclaude
andcommitted
fix: address Copilot review suggestions
- Remove unused ConcurrentBag variable - Cap Environment.ProcessorCount in tests for stable CI - Strengthen DiffProgress assertions (check Total > 0 and Completed == Total) - Clarify BsdiffDiffer thread-safety doc to mention compression provider contract Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 5c30265 commit b99e554

2 files changed

Lines changed: 12 additions & 9 deletions

File tree

src/c#/GeneralUpdate.Differential/Differ/BsdiffDiffer.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,11 @@ namespace GeneralUpdate.Differential.Differ
1515
/// Default: <see cref="BZip2CompressionProvider"/> (backward compatible).
1616
/// Use <see cref="DeflateCompressionProvider"/> (0x01) for faster decompression.
1717
///
18-
/// Thread-safety: this class is stateless beyond the compression provider.
19-
/// A single instance is safe for concurrent calls.
18+
/// Thread-safety: this class is safe for concurrent calls provided the
19+
/// configured <see cref="ICompressionProvider"/> is thread-safe.
20+
/// The built-in providers (<see cref="BZip2CompressionProvider"/>,
21+
/// <see cref="DeflateCompressionProvider"/>, <see cref="BrotliCompressionProvider"/>)
22+
/// are all safe for concurrent use.
2023
/// </remarks>
2124
public class BsdiffDiffer : IBinaryDiffer
2225
{

tests/DifferentialTest/ComprehensiveDifferentialTests.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System;
2-
using System.Collections.Concurrent;
32
using System.Collections.Generic;
43
using System.Diagnostics;
54
using System.IO;
@@ -395,10 +394,9 @@ public async Task DiffPipeline_ParallelClean_AllFilesGeneratePatches()
395394
// Also add some new files in target (no old version)
396395
File.WriteAllBytes(Path.Combine(tgt, "new_features.dll"), [1, 2, 3, 4, 5]);
397396

398-
var completedFiles = new ConcurrentBag<string>();
399397
var progress = new SyncProgress<DiffProgress>();
400398
var pipeline = new DiffPipelineBuilder()
401-
.WithParallelism(Environment.ProcessorCount)
399+
.WithParallelism(Math.Min(Environment.ProcessorCount, 4))
402400
.Build();
403401

404402
// Act
@@ -407,7 +405,8 @@ public async Task DiffPipeline_ParallelClean_AllFilesGeneratePatches()
407405
_output.WriteLine($"Parallel Clean (30 files): {sw.ElapsedMilliseconds}ms");
408406

409407
// Assert
410-
Assert.True(progress.LastValue.IsComplete);
408+
Assert.True(progress.LastValue.Total > 0, "Progress Total should be > 0");
409+
Assert.Equal(progress.LastValue.Total, progress.LastValue.Completed);
411410
_output.WriteLine($"Progress: {progress.LastValue.Completed}/{progress.LastValue.Total}");
412411

413412
// Each modified file should have a .patch
@@ -807,17 +806,18 @@ public async Task MultiFile_HighParallelism_AllFilesProcessed()
807806
File.WriteAllBytes(Path.Combine(app, $"h_{i:D3}.dll"), oldContent);
808807
}
809808

809+
var parallelism = Math.Min(Environment.ProcessorCount, 4);
810810
var pipeline = new DiffPipelineBuilder()
811-
.WithParallelism(Environment.ProcessorCount)
811+
.WithParallelism(parallelism)
812812
.Build();
813813

814814
var sw = Stopwatch.StartNew();
815815
await pipeline.CleanAsync(src, tgt, patchDir);
816-
_output.WriteLine($"Clean {fileCount} files (x{Environment.ProcessorCount}): {sw.ElapsedMilliseconds}ms");
816+
_output.WriteLine($"Clean {fileCount} files (x{parallelism}): {sw.ElapsedMilliseconds}ms");
817817

818818
sw.Restart();
819819
await pipeline.DirtyAsync(app, patchDir);
820-
_output.WriteLine($"Dirty {fileCount} files (x{Environment.ProcessorCount}): {sw.ElapsedMilliseconds}ms");
820+
_output.WriteLine($"Dirty {fileCount} files (x{parallelism}): {sw.ElapsedMilliseconds}ms");
821821

822822
for (var i = 0; i < fileCount; i++)
823823
{

0 commit comments

Comments
 (0)