diff --git a/src/Cake.Issues.Testing/FakeFailingIssueProvider.cs b/src/Cake.Issues.Testing/FakeFailingIssueProvider.cs new file mode 100644 index 000000000..1e33d9302 --- /dev/null +++ b/src/Cake.Issues.Testing/FakeFailingIssueProvider.cs @@ -0,0 +1,20 @@ +namespace Cake.Issues.Testing; + +using System.Collections.Generic; +using Cake.Core.Diagnostics; + +/// +/// Implementation of a that fails during initialization for use in test cases. +/// +/// The Cake log instance. +public class FakeFailingIssueProvider(ICakeLog log) : BaseIssueProvider(log) +{ + /// + public override string ProviderName => "Fake Failing Issue Provider"; + + /// + public override bool Initialize(IReadIssuesSettings settings) => false; + + /// + protected override IEnumerable InternalReadIssues() => []; +} \ No newline at end of file diff --git a/src/Cake.Issues.Testing/FakeSlowIssueProvider.cs b/src/Cake.Issues.Testing/FakeSlowIssueProvider.cs new file mode 100644 index 000000000..8d6f2692e --- /dev/null +++ b/src/Cake.Issues.Testing/FakeSlowIssueProvider.cs @@ -0,0 +1,40 @@ +namespace Cake.Issues.Testing; + +using System.Collections.Generic; +using System.Threading; +using Cake.Core.Diagnostics; + +/// +/// Implementation of a that simulates slow processing for performance testing. +/// +public class FakeSlowIssueProvider : BaseIssueProvider +{ + private readonly List issues = []; + private readonly int delayMs; + + /// + /// Initializes a new instance of the class. + /// + /// The Cake log instance. + /// Issues which should be returned by the issue provider. + /// Simulated processing delay in milliseconds. + public FakeSlowIssueProvider(ICakeLog log, IEnumerable issues, int delayMs) + : base(log) + { + issues.NotNull(); + + this.issues.AddRange(issues); + this.delayMs = delayMs; + } + + /// + public override string ProviderName => "Fake Slow Issue Provider"; + + /// + protected override IEnumerable InternalReadIssues() + { + // Simulate slow processing + Thread.Sleep(this.delayMs); + return this.issues; + } +} \ No newline at end of file diff --git a/src/Cake.Issues.Tests/IssueReaderTests.cs b/src/Cake.Issues.Tests/IssueReaderTests.cs index 83a6018b0..055391ddf 100644 --- a/src/Cake.Issues.Tests/IssueReaderTests.cs +++ b/src/Cake.Issues.Tests/IssueReaderTests.cs @@ -94,7 +94,7 @@ public void Should_Initialize_Issue_Provider() _ = fixture.ReadIssues(); // Then - fixture.IssueProviders.ShouldAllBe(x => x.Settings == fixture.Settings); + fixture.IssueProviders.OfType().ShouldAllBe(x => x.Settings == fixture.Settings); } [Fact] @@ -142,7 +142,7 @@ public void Should_Initialize_All_Issue_Provider() _ = fixture.ReadIssues(); // Then - fixture.IssueProviders.ShouldAllBe(x => x.Settings == fixture.Settings); + fixture.IssueProviders.OfType().ShouldAllBe(x => x.Settings == fixture.Settings); } [Fact] @@ -349,5 +349,132 @@ public void Should_Set_FileLink_Property() issue2.FileLink.ToString() .ShouldBe($"{repoUrl}/blob/{branch}/{filePath2.Replace(@"\", "/")}#L{line2}"); } + + [Fact] + public void Should_Read_Issues_From_Multiple_Providers_Concurrently() + { + // Given + const int providerCount = 10; + const int issuesPerProvider = 100; + var fixture = new IssuesFixture(); + fixture.IssueProviders.Clear(); + + // Create multiple providers with different issues + for (var i = 0; i < providerCount; i++) + { + var providerIssues = new List(); + for (var j = 0; j < issuesPerProvider; j++) + { + var issue = IssueBuilder + .NewIssue($"Issue{i}-{j}", $"ProviderType{i}", $"ProviderName{i}") + .InFile($@"src\Provider{i}\File{j}.cs", j + 1) + .OfRule($"Rule{j}") + .WithPriority(IssuePriority.Warning) + .Create(); + providerIssues.Add(issue); + } + fixture.IssueProviders.Add(new FakeIssueProvider(fixture.Log, providerIssues)); + } + + // When + var stopwatch = System.Diagnostics.Stopwatch.StartNew(); + var issues = fixture.ReadIssues().ToList(); + stopwatch.Stop(); + + // Then + issues.Count.ShouldBe(providerCount * issuesPerProvider); + + // Verify all providers contributed issues + for (var i = 0; i < providerCount; i++) + { + var providerIssues = issues.Where(x => x.ProviderType == $"ProviderType{i}").ToList(); + providerIssues.Count.ShouldBe(issuesPerProvider); + } + + // Verify all Run properties are set + issues.ShouldAllBe(x => x.Run == fixture.Settings.Run); + + // Log timing for performance verification + System.Console.WriteLine($"Reading {issues.Count} issues from {providerCount} providers took {stopwatch.ElapsedMilliseconds}ms"); + } + + [Fact] + public void Should_Handle_Provider_Initialization_Failures_Concurrently() + { + // Given + var fixture = new IssuesFixture(); + fixture.IssueProviders.Clear(); + + // Add mix of successful and failing providers + fixture.IssueProviders.Add(new FakeIssueProvider(fixture.Log, [ + IssueBuilder.NewIssue("Success1", "ProviderType1", "ProviderName1") + .InFile(@"src\File1.cs", 1) + .OfRule("Rule1") + .WithPriority(IssuePriority.Warning) + .Create() + ])); + + // Create a failing provider by passing null settings later + var failingProvider = new FakeFailingIssueProvider(fixture.Log); + fixture.IssueProviders.Add(failingProvider); + fixture.IssueProviders.Add(new FakeIssueProvider(fixture.Log, [ + IssueBuilder.NewIssue("Success2", "ProviderType2", "ProviderName2") + .InFile(@"src\File2.cs", 2) + .OfRule("Rule2") + .WithPriority(IssuePriority.Warning) + .Create() + ])); + + // When + var issues = fixture.ReadIssues().ToList(); + + // Then + // Should get issues from successful providers only + issues.Count.ShouldBe(2); + issues.ShouldContain(x => x.MessageText == "Success1"); + issues.ShouldContain(x => x.MessageText == "Success2"); + } + + [Fact] + public void Should_Demonstrate_Parallel_Processing_Benefits_With_Simulated_Delays() + { + // Given - Create providers that simulate processing delays + const int providerCount = 5; + const int delayPerProviderMs = 50; // Simulate 50ms delay per provider + var fixture = new IssuesFixture(); + fixture.IssueProviders.Clear(); + + for (var i = 0; i < providerCount; i++) + { + var issue = IssueBuilder + .NewIssue($"SlowIssue{i}", $"SlowProviderType{i}", $"SlowProviderName{i}") + .InFile($@"src\SlowFile{i}.cs", i + 1) + .OfRule($"SlowRule{i}") + .WithPriority(IssuePriority.Warning) + .Create(); + fixture.IssueProviders.Add(new FakeSlowIssueProvider(fixture.Log, [issue], delayPerProviderMs)); + } + + // When + var stopwatch = System.Diagnostics.Stopwatch.StartNew(); + var issues = fixture.ReadIssues().ToList(); + stopwatch.Stop(); + + // Then + issues.Count.ShouldBe(providerCount); + + // With parallel processing, total time should be significantly less than + // sum of all delays (providerCount * delayPerProviderMs) + var expectedSequentialTime = providerCount * delayPerProviderMs; + var actualTime = stopwatch.ElapsedMilliseconds; + + Console.WriteLine($"Sequential time would be ~{expectedSequentialTime}ms, parallel time was {actualTime}ms"); + actualTime.ShouldBeLessThan(expectedSequentialTime, "Parallel reading of issue providers is slower than sequential processing would be"); + + // This assertion may be flaky in CI environments, so we'll use a generous threshold + // Should be much faster than 40% of sequential time + //var maxExpectedParallelTime = expectedSequentialTime * 0.4; + //Convert.ToDouble(actualTime).ShouldBeLessThan(maxExpectedParallelTime, "Parallel reading of issue providers is more than 40% of time it took for sequential processing"); + } } } \ No newline at end of file diff --git a/src/Cake.Issues.Tests/IssuesFixture.cs b/src/Cake.Issues.Tests/IssuesFixture.cs index 53dae27f4..f71ef2413 100644 --- a/src/Cake.Issues.Tests/IssuesFixture.cs +++ b/src/Cake.Issues.Tests/IssuesFixture.cs @@ -7,7 +7,7 @@ public class IssuesFixture public IssuesFixture() { this.Log = new FakeLog { Verbosity = Verbosity.Normal }; - this.IssueProviders = [new(this.Log)]; + this.IssueProviders = [new FakeIssueProvider(this.Log)]; this.Settings = new ReadIssuesSettings( new Core.IO.DirectoryPath(@"c:\Source\Cake.Issues")); @@ -15,7 +15,7 @@ public IssuesFixture() public FakeLog Log { get; init; } - public IList IssueProviders { get; init; } + public IList IssueProviders { get; init; } public ReadIssuesSettings Settings { get; init; } diff --git a/src/Cake.Issues/IssuesReader.cs b/src/Cake.Issues/IssuesReader.cs index 714c2f09a..8b72d509e 100644 --- a/src/Cake.Issues/IssuesReader.cs +++ b/src/Cake.Issues/IssuesReader.cs @@ -1,7 +1,9 @@ namespace Cake.Issues; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; +using System.Threading.Tasks; using Cake.Core.Diagnostics; /// @@ -41,40 +43,59 @@ public IssuesReader( /// List of issues. public IEnumerable ReadIssues() { - // Initialize issue providers and read issues. - var issues = new List(); - foreach (var issueProvider in this.issueProviders) + var stopwatch = Stopwatch.StartNew(); + + var results = new List[this.issueProviders.Count]; + + // Process providers in parallel + _ = Parallel.For(0, this.issueProviders.Count, i => { - var providerName = issueProvider.GetType().Name; - this.log.Verbose("Initialize issue provider {0}...", providerName); - if (issueProvider.Initialize(this.settings)) - { - this.log.Verbose("Reading issues from {0}...", providerName); - var currentIssues = issueProvider.ReadIssues().ToList(); + results[i] = this.ReadIssuesFromProvider(this.issueProviders[i]); + }); - this.log.Verbose( - "Found {0} issues using issue provider {1}...", - currentIssues.Count, - providerName); + stopwatch.Stop(); - currentIssues.ForEach(x => - { - x.Run = this.settings.Run; + var issuesList = results.SelectMany(r => r).ToList(); + this.log.Verbose( + "Reading {0} issues from {1} providers took {2} ms", + issuesList.Count, + this.issueProviders.Count, + stopwatch.ElapsedMilliseconds); - if (this.settings.FileLinkSettings != null) - { - x.FileLink = this.settings.FileLinkSettings.GetFileLink(x); - } - }); + return issuesList; + } + + private List ReadIssuesFromProvider(IIssueProvider issueProvider) + { + var providerName = issueProvider.GetType().Name; + this.log.Verbose("Initialize issue provider {0}...", providerName); - issues.AddRange(currentIssues); - } - else + if (issueProvider.Initialize(this.settings)) + { + this.log.Verbose("Reading issues from {0}...", providerName); + var currentIssues = issueProvider.ReadIssues().ToList(); + + this.log.Verbose( + "Found {0} issues using issue provider {1}...", + currentIssues.Count, + providerName); + + // Post-process issues - this is thread-safe as each provider gets its own issues + currentIssues.ForEach(x => { - this.log.Warning("Error initializing issue provider {0}.", providerName); - } - } + x.Run = this.settings.Run; - return issues; + if (this.settings.FileLinkSettings != null) + { + x.FileLink = this.settings.FileLinkSettings.GetFileLink(x); + } + }); + return currentIssues; + } + else + { + this.log.Warning("Error initializing issue provider {0}.", providerName); + return []; + } } }