Skip to content

Make IIssue immutable by removing mutable Run and FileLink properties #1269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

Make IIssue immutable by removing mutable Run and FileLink properties #1269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c294d73
Initial plan
Copilot Sep 23, 2025
1b03cdf
Make IIssue immutable with Run and FileLink properties
Copilot Sep 24, 2025
04429ec
Fix all PullRequests tests to use identifier comparison instead of re…
Copilot Sep 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/Cake.Issues.PullRequests.Tests/ExceptionAssertExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
namespace Cake.Issues.PullRequests.Tests;

using System.Collections.Generic;
using System.Linq;
using Shouldly;

public static class ExceptionAssertExtensions
{
public static void IsPullRequestIssuesException(this Exception exception, string message)
Expand All @@ -8,3 +12,21 @@ public static void IsPullRequestIssuesException(this Exception exception, string
Assert.Equal(message, ex.Message);
}
}

/// <summary>
/// Extensions to help with issue comparisons in tests after making IIssue immutable.
/// Since issues are now cloned with enhanced properties, we need to compare by content rather than reference.
/// </summary>
public static class IssueAssertExtensions
{
/// <summary>
/// Verifies that the collection contains an issue with the same identifier as the expected issue.
/// This is needed because IIssue is now immutable and IssuesReader returns new objects.
/// </summary>
public static void ShouldContainIssueWithSameIdentifier(
this IEnumerable<IIssue> actualIssues,
IIssue expectedIssue) =>
actualIssues
.Any(x => x.Identifier == expectedIssue.Identifier)
.ShouldBeTrue($"Expected to find issue with identifier '{expectedIssue.Identifier}' but collection only contained: [{string.Join(", ", actualIssues.Select(x => $"'{x.Identifier}'"))}]");
}
62 changes: 31 additions & 31 deletions src/Cake.Issues.PullRequests.Tests/IssueFiltererTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public void Should_Apply_Custom_Filters()

// Then
issues.Count.ShouldBe(1);
issues.ShouldContain(issue1);
issues.ShouldContainIssueWithSameIdentifier(issue1);
}

public sealed class TheFilterIssuesByPathMethod
Expand Down Expand Up @@ -198,7 +198,7 @@ public void Should_Filter_Issues_If_File_Is_Not_Modified()

// Then
issues.Count.ShouldBe(1);
issues.ShouldContain(issue1);
issues.ShouldContainIssueWithSameIdentifier(issue1);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered because they do not belong to files that were changed in this pull request");
}
}
Expand Down Expand Up @@ -254,7 +254,7 @@ public void Should_Filter_Issues_With_Existing_Active_Comment()

// Then
issues.Count.ShouldBe(1);
issues.ShouldContain(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue2);
}

[Fact]
Expand Down Expand Up @@ -303,7 +303,7 @@ public void Should_Filter_Issues_With_Existing_WontFix_Comment()

// Then
issues.Count.ShouldBe(1);
issues.ShouldContain(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue2);
}

[Fact]
Expand Down Expand Up @@ -352,7 +352,7 @@ public void Should_Filter_Issues_With_Existing_Resolved_Comment()

// Then
issues.Count.ShouldBe(1);
issues.ShouldContain(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue2);
}
}

Expand Down Expand Up @@ -398,7 +398,7 @@ public void Should_Limit_Messages_To_Maximum()

// Then
issues.Count.ShouldBe(1);
issues.ShouldContain(issue1);
issues.ShouldContainIssueWithSameIdentifier(issue1);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global issue limit of 1");
}

Expand Down Expand Up @@ -440,7 +440,7 @@ public void Should_Limit_Messages_To_Maximum_By_Priority()

// Then
issues.Count.ShouldBe(1);
issues.ShouldContain(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue2);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global issue limit of 1");
}

Expand Down Expand Up @@ -481,7 +481,7 @@ public void Should_Limit_Messages_To_Maximum_By_FilePath()

// Then
issues.Count.ShouldBe(1);
issues.ShouldContain(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue2);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global issue limit of 1");
}
}
Expand Down Expand Up @@ -753,8 +753,8 @@ public void Should_Limit_Messages_To_Maximum_By_Priority()

// Then
issues.Count.ShouldBe(2);
issues.ShouldContain(issue2);
issues.ShouldContain(issue3);
issues.ShouldContainIssueWithSameIdentifier(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue3);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global issue limit of 2 across all runs for provider 'ProviderType Foo' (1 issues already posted in previous runs)");
}

Expand Down Expand Up @@ -824,8 +824,8 @@ public void Should_Limit_Messages_To_Maximum_By_FilePath()

// Then
issues.Count.ShouldBe(2);
issues.ShouldContain(issue2);
issues.ShouldContain(issue4);
issues.ShouldContainIssueWithSameIdentifier(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue4);
fixture.Log.Entries.ShouldContain(x => x.Message == "2 issue(s) were filtered to match the global issue limit of 2 across all runs for provider 'ProviderType Foo' (1 issues already posted in previous runs)");
}
}
Expand Down Expand Up @@ -883,8 +883,8 @@ public void Should_Limit_Messages_To_Maximum()

// Then
issues.Count.ShouldBe(2);
issues.ShouldContain(issue1);
issues.ShouldContain(issue3);
issues.ShouldContainIssueWithSameIdentifier(issue1);
issues.ShouldContainIssueWithSameIdentifier(issue3);

fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global limit of 1 issues which should be reported for issue provider 'ProviderTypeA'");
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global limit of 1 issues which should be reported for issue provider 'ProviderTypeB'");
Expand Down Expand Up @@ -997,10 +997,10 @@ public void Should_Ignore_Limit_When_Negative()

// Then
issues.Count.ShouldBe(4);
issues.ShouldContain(issue1);
issues.ShouldContain(issue2);
issues.ShouldContain(issue3);
issues.ShouldContain(issue4);
issues.ShouldContainIssueWithSameIdentifier(issue1);
issues.ShouldContainIssueWithSameIdentifier(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue3);
issues.ShouldContainIssueWithSameIdentifier(issue4);
}

[Fact]
Expand Down Expand Up @@ -1054,8 +1054,8 @@ public void Should_Limit_Messages_To_Maximum_By_Priority()

// Then
issues.Count.ShouldBe(2);
issues.ShouldContain(issue2);
issues.ShouldContain(issue3);
issues.ShouldContainIssueWithSameIdentifier(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue3);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global limit of 1 issues which should be reported for issue provider 'ProviderTypeA'");
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global limit of 1 issues which should be reported for issue provider 'ProviderTypeB'");
}
Expand Down Expand Up @@ -1109,8 +1109,8 @@ public void Should_Limit_Messages_To_Maximum_By_FilePath()

// Then
issues.Count.ShouldBe(2);
issues.ShouldContain(issue2);
issues.ShouldContain(issue3);
issues.ShouldContainIssueWithSameIdentifier(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue3);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global limit of 1 issues which should be reported for issue provider 'ProviderTypeA'");
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global limit of 1 issues which should be reported for issue provider 'ProviderTypeB'");
}
Expand Down Expand Up @@ -1260,7 +1260,7 @@ public void Should_Limit_Messages_To_Maximum()

// Then
issues.Count.ShouldBe(1);
issues.ShouldContain(issue1);
issues.ShouldContainIssueWithSameIdentifier(issue1);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global issue limit of 2 across all runs (1 issues already posted in previous runs)");
}

Expand Down Expand Up @@ -1315,7 +1315,7 @@ public void Should_Limit_Messages_To_Maximum_By_Priority()

// Then
issues.Count.ShouldBe(1);
issues.ShouldContain(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue2);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global issue limit of 2 across all runs (1 issues already posted in previous runs)");
}

Expand Down Expand Up @@ -1369,7 +1369,7 @@ public void Should_Limit_Messages_To_Maximum_By_FilePath()

// Then
issues.Count.ShouldBe(1);
issues.ShouldContain(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue2);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered to match the global issue limit of 2 across all runs (1 issues already posted in previous runs)");
}
}
Expand Down Expand Up @@ -1428,8 +1428,8 @@ public void Should_Limit_Messages_To_Maximum()

// Then
issues.Count.ShouldBe(2);
issues.ShouldContain(issue1);
issues.ShouldContain(issue3);
issues.ShouldContainIssueWithSameIdentifier(issue1);
issues.ShouldContainIssueWithSameIdentifier(issue3);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) of type ProviderTypeA were filtered to match the maximum of 1 issues which should be reported for each issue provider");
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) of type ProviderTypeB were filtered to match the maximum of 1 issues which should be reported for each issue provider");
}
Expand Down Expand Up @@ -1486,8 +1486,8 @@ public void Should_Limit_Messages_To_Maximum_By_Priority()

// Then
issues.Count.ShouldBe(2);
issues.ShouldContain(issue2);
issues.ShouldContain(issue3);
issues.ShouldContainIssueWithSameIdentifier(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue3);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) of type ProviderTypeA were filtered to match the maximum of 1 issues which should be reported for each issue provider");
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) of type ProviderTypeB were filtered to match the maximum of 1 issues which should be reported for each issue provider");
}
Expand Down Expand Up @@ -1542,8 +1542,8 @@ public void Should_Limit_Messages_To_Maximum_By_FilePath()

// Then
issues.Count.ShouldBe(2);
issues.ShouldContain(issue2);
issues.ShouldContain(issue3);
issues.ShouldContainIssueWithSameIdentifier(issue2);
issues.ShouldContainIssueWithSameIdentifier(issue3);
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) of type ProviderTypeA were filtered to match the maximum of 1 issues which should be reported for each issue provider");
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) of type ProviderTypeB were filtered to match the maximum of 1 issues which should be reported for each issue provider");
}
Expand Down
Loading
Loading