feat: add FakeXrmEasy mocked tests for BulkOperationExecutor (Phase 3)

.claude/commands/review-bot-comments.md

@@ -13,6 +13,18 @@ Systematically address PR review comments from Copilot, Gemini, and CodeQL.
 gh api repos/joshsmithxrm/ppds-sdk/pulls/[PR]/comments
 ```
 
+### Bot Usernames
+
+Look for comments from these `user.login` values:
+
+| Bot | Username |
+|-----|----------|
+| Gemini | `gemini-code-assist[bot]` |
+| Copilot | `Copilot` |
+| CodeQL/GHAS | `github-advanced-security[bot]` |
+
+**Note:** CodeQL and Copilot frequently report **duplicate findings** (same file, same line, same issue). Group comments by file+line to identify duplicates before triaging.
+
 ### 2. Triage Each Comment
 
 For each bot comment, determine verdict and rationale:
@@ -79,6 +91,7 @@ gh api repos/joshsmithxrm/ppds-sdk/pulls/{pr}/comments \
 | Bot Claim | Why It's Often Wrong |
 |-----------|---------------------|
 | "Use .Where() instead of foreach+if" | Preference, not correctness |
+| "Use .Select() instead of foreach" | Using Select for side effects is an anti-pattern |
 | "Volatile needed with Interlocked" | Interlocked provides barriers |
 | "OR should be AND" | Logic may be intentionally inverted (DeMorgan) |
 | "Static field not thread-safe" | May be set once at startup |

tests/PPDS.Dataverse.IntegrationTests/BulkOperations/BatchingBehaviorTests.cs

@@ -0,0 +1,251 @@
+using FluentAssertions;
+using Microsoft.Xrm.Sdk;
+using PPDS.Dataverse.BulkOperations;
+using Xunit;
+
+namespace PPDS.Dataverse.IntegrationTests.BulkOperations;
+
+/// <summary>
+/// Tests for verifying correct batching behavior in bulk operations.
+/// </summary>
+public class BatchingBehaviorTests : BulkOperationExecutorTestsBase
+{
+    private const string EntityName = "account";
+
+    [Theory]
+    [InlineData(1, 10)]    // 1 entity, batch size 10 = 1 batch
+    [InlineData(10, 10)]   // 10 entities, batch size 10 = 1 batch
+    [InlineData(11, 10)]   // 11 entities, batch size 10 = 2 batches
+    [InlineData(100, 10)]  // 100 entities, batch size 10 = 10 batches
+    [InlineData(105, 10)]  // 105 entities, batch size 10 = 11 batches
+    public async Task CreateMultiple_WithVariousBatchSizes_ProcessesAllEntities(int entityCount, int batchSize)
+    {
+        // Arrange
+        var entities = CreateTestEntities(EntityName, entityCount);
+        var options = new BulkOperationOptions
+        {
+            BatchSize = batchSize,
+            MaxParallelBatches = 1
+        };
+
+        // Act
+        var result = await Executor.CreateMultipleAsync(EntityName, entities, options);
+
+        // Assert
+        result.IsSuccess.Should().BeTrue();
+        result.SuccessCount.Should().Be(entityCount);
+        result.CreatedIds.Should().HaveCount(entityCount);
+    }
+
+    [Fact]
+    public async Task CreateMultiple_WithBatchSizeOne_ProcessesEachEntitySeparately()
+    {
+        // Arrange
+        var entities = CreateTestEntities(EntityName, 5);
+        var options = new BulkOperationOptions
+        {
+            BatchSize = 1,
+            MaxParallelBatches = 1
+        };
+
+        // Act
+        var result = await Executor.CreateMultipleAsync(EntityName, entities, options);
+
+        // Assert
+        result.IsSuccess.Should().BeTrue();
+        result.SuccessCount.Should().Be(5);
+    }
+
+    [Fact]
+    public async Task CreateMultiple_WithLargeBatchSize_ProcessesInSingleBatch()
+    {
+        // Arrange
+        var entities = CreateTestEntities(EntityName, 50);
+        var options = new BulkOperationOptions
+        {
+            BatchSize = 1000, // Much larger than entity count
+            MaxParallelBatches = 1
+        };
+
+        // Act
+        var result = await Executor.CreateMultipleAsync(EntityName, entities, options);
+
+        // Assert
+        result.IsSuccess.Should().BeTrue();
+        result.SuccessCount.Should().Be(50);
+    }
+
+    [Fact]
+    public async Task CreateMultiple_ProgressReports_MatchBatching()
+    {
+        // Arrange
+        var entities = CreateTestEntities(EntityName, 25);
+        var options = new BulkOperationOptions
+        {
+            BatchSize = 10,
+            MaxParallelBatches = 1 // Sequential for deterministic progress
+        };
+        var progress = CreateProgressReporter();
+
+        // Act
+        await Executor.CreateMultipleAsync(EntityName, entities, options, progress);
+
+        // Assert - Should have 3 batches: 10, 10, 5
+        progress.Reports.Should().HaveCount(3);
+        progress.Reports[0].Processed.Should().Be(10);
+        progress.Reports[1].Processed.Should().Be(20);
+        progress.Reports[2].Processed.Should().Be(25);
+    }
+
+    [Fact]
+    public async Task UpdateMultiple_ProgressReports_MatchBatching()
+    {
+        // Arrange
+        var createResult = await Executor.CreateMultipleAsync(EntityName, CreateTestEntities(EntityName, 25));
+        var updateEntities = CreateTestEntitiesWithIds(EntityName, createResult.CreatedIds!);
+        var options = new BulkOperationOptions
+        {
+            BatchSize = 10,
+            MaxParallelBatches = 1
+        };
+        var progress = CreateProgressReporter();
+
+        // Act
+        await Executor.UpdateMultipleAsync(EntityName, updateEntities, options, progress);
+
+        // Assert - Should have 3 batches: 10, 10, 5
+        progress.Reports.Should().HaveCount(3);
+        progress.LastReport!.Processed.Should().Be(25);
+    }
+
+    [Fact]
+    public async Task DeleteMultiple_ProgressReports_MatchBatching()
+    {
+        // Arrange
+        var createResult = await Executor.CreateMultipleAsync(EntityName, CreateTestEntities(EntityName, 25));
+        var options = new BulkOperationOptions
+        {
+            BatchSize = 10,
+            MaxParallelBatches = 1
+        };
+        var progress = CreateProgressReporter();
+
+        // Act
+        await Executor.DeleteMultipleAsync(EntityName, createResult.CreatedIds!.ToList(), options, progress);
+
+        // Assert - Should have 3 batches: 10, 10, 5
+        progress.Reports.Should().HaveCount(3);
+        progress.LastReport!.Processed.Should().Be(25);
+    }
+
+    [Fact]
+    public async Task ProgressReporter_TracksTotalCorrectly()
+    {
+        // Arrange
+        var entities = CreateTestEntities(EntityName, 100);
+        var options = new BulkOperationOptions
+        {
+            BatchSize = 25,
+            MaxParallelBatches = 1
+        };
+        var progress = CreateProgressReporter();
+
+        // Act
+        await Executor.CreateMultipleAsync(EntityName, entities, options, progress);
+
+        // Assert
+        progress.LastReport!.Total.Should().Be(100);
+        progress.LastReport.Processed.Should().Be(100);
+        progress.LastReport.PercentComplete.Should().Be(100);
+    }
+
+    [Theory]
+    [InlineData(5)]
+    [InlineData(10)]
+    [InlineData(50)]
+    [InlineData(100)]
+    public async Task DefaultBatchSize_ProcessesAllEntities(int entityCount)
+    {
+        // Arrange - Use default options (batch size 100)
+        var entities = CreateTestEntities(EntityName, entityCount);
+
+        // Act
+        var result = await Executor.CreateMultipleAsync(EntityName, entities);
+
+        // Assert
+        result.IsSuccess.Should().BeTrue();
+        result.SuccessCount.Should().Be(entityCount);
+    }
+
+    [Fact]
+    public async Task BypassCustomLogic_IsApplied()
+    {
+        // Arrange
+        var entities = CreateTestEntities(EntityName, 5);
+        var options = new BulkOperationOptions
+        {
+            BypassCustomLogic = CustomLogicBypass.All,
+            MaxParallelBatches = 1
+        };
+
+        // Act - Should not throw
+        var result = await Executor.CreateMultipleAsync(EntityName, entities, options);
+
+        // Assert
+        result.IsSuccess.Should().BeTrue();
+    }
+
+    [Fact]
+    public async Task BypassPowerAutomateFlows_IsApplied()
+    {
+        // Arrange
+        var entities = CreateTestEntities(EntityName, 5);
+        var options = new BulkOperationOptions
+        {
+            BypassPowerAutomateFlows = true,
+            MaxParallelBatches = 1
+        };
+
+        // Act - Should not throw
+        var result = await Executor.CreateMultipleAsync(EntityName, entities, options);
+
+        // Assert
+        result.IsSuccess.Should().BeTrue();
+    }
+
+    [Fact]
+    public async Task SuppressDuplicateDetection_IsApplied()
+    {
+        // Arrange
+        var entities = CreateTestEntities(EntityName, 5);
+        var options = new BulkOperationOptions
+        {
+            SuppressDuplicateDetection = true,
+            MaxParallelBatches = 1
+        };
+
+        // Act - Should not throw
+        var result = await Executor.CreateMultipleAsync(EntityName, entities, options);
+
+        // Assert
+        result.IsSuccess.Should().BeTrue();
+    }
+
+    [Fact]
+    public async Task Tag_IsApplied()
+    {
+        // Arrange
+        var entities = CreateTestEntities(EntityName, 5);
+        var options = new BulkOperationOptions
+        {
+            Tag = "TestBulkOperation",
+            MaxParallelBatches = 1
+        };
+
+        // Act - Should not throw
+        var result = await Executor.CreateMultipleAsync(EntityName, entities, options);
+
+        // Assert
+        result.IsSuccess.Should().BeTrue();
+    }
+}

tests/PPDS.Dataverse.IntegrationTests/BulkOperations/BulkOperationExecutorTestsBase.cs

@@ -0,0 +1,120 @@
+using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.Extensions.Options;
+using Microsoft.Xrm.Sdk;
+using PPDS.Dataverse.BulkOperations;
+using PPDS.Dataverse.DependencyInjection;
+using PPDS.Dataverse.IntegrationTests.Mocks;
+using PPDS.Dataverse.Pooling;
+using PPDS.Dataverse.Progress;
+using PPDS.Dataverse.Resilience;
+
+namespace PPDS.Dataverse.IntegrationTests.BulkOperations;
+
+/// <summary>
+/// Base class for BulkOperationExecutor tests using FakeXrmEasy.
+/// Provides pre-configured BulkOperationExecutor with mocked dependencies.
+/// </summary>
+public abstract class BulkOperationExecutorTestsBase : FakeXrmEasyTestsBase
+{
+    /// <summary>
+    /// The BulkOperationExecutor under test.
+    /// </summary>
+    protected IBulkOperationExecutor Executor { get; }
+
+    /// <summary>
+    /// The fake connection pool for controlling connection behavior.
+    /// </summary>
+    protected FakeConnectionPool ConnectionPool { get; }
+
+    /// <summary>
+    /// The fake throttle tracker for controlling throttle behavior.
+    /// </summary>
+    protected FakeThrottleTracker ThrottleTracker { get; }
+
+    /// <summary>
+    /// The Dataverse options for configuring bulk operation behavior.
+    /// </summary>
+    protected DataverseOptions Options { get; }
+
+    /// <summary>
+    /// Initializes a new instance with mocked BulkOperationExecutor dependencies.
+    /// </summary>
+    protected BulkOperationExecutorTestsBase()
+    {
+        ConnectionPool = new FakeConnectionPool(Service);
+        ThrottleTracker = new FakeThrottleTracker();
+        Options = new DataverseOptions
+        {
+            BulkOperations = new BulkOperationOptions
+            {
+                BatchSize = 100,
+                MaxParallelBatches = 1 // Sequential for deterministic testing
+            },
+            Pool = new ConnectionPoolOptions
+            {
+                MaxConnectionRetries = 2
+            }
+        };
+
+        var optionsWrapper = Microsoft.Extensions.Options.Options.Create(Options);
+        var logger = NullLogger<BulkOperationExecutor>.Instance;
+
+        Executor = new BulkOperationExecutor(
+            ConnectionPool,
+            ThrottleTracker,
+            optionsWrapper,
+            logger);
+    }
+
+    /// <summary>
+    /// Creates a collection of test entities with the specified count.
+    /// </summary>
+    protected static List<Entity> CreateTestEntities(string entityName, int count)
+    {
+        var entities = new List<Entity>();
+        for (int i = 0; i < count; i++)
+        {
+            entities.Add(new Entity(entityName)
+            {
+                ["name"] = $"Test Entity {i}"
+            });
+        }
+        return entities;
+    }
+
+    /// <summary>
+    /// Creates a collection of test entities with IDs for update/upsert operations.
+    /// </summary>
+    protected static List<Entity> CreateTestEntitiesWithIds(string entityName, IEnumerable<Guid> ids)
+    {
+        return ids.Select((id, i) => new Entity(entityName, id)
+        {
+            ["name"] = $"Updated Entity {i}"
+        }).ToList();
+    }
+
+    /// <summary>
+    /// Creates a progress tracker for testing progress reporting.
+    /// </summary>
+    protected static TestProgressReporter CreateProgressReporter()
+    {
+        return new TestProgressReporter();
+    }
+
+    /// <summary>
+    /// Helper class for capturing progress reports during tests.
+    /// </summary>
+    protected class TestProgressReporter : IProgress<ProgressSnapshot>
+    {
+        private readonly List<ProgressSnapshot> _reports = new();
+
+        public IReadOnlyList<ProgressSnapshot> Reports => _reports;
+
+        public ProgressSnapshot? LastReport => _reports.Count > 0 ? _reports[^1] : null;
+
+        public void Report(ProgressSnapshot value)
+        {
+            _reports.Add(value);
+        }
+    }
+}