Skip to content
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

Tweak to InitialIssueTriage and PullRequestTriage rules #5830

Merged
merged 3 commits into from
Mar 28, 2023
Merged

Tweak to InitialIssueTriage and PullRequestTriage rules #5830

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
3dcd555
Tweak to InitialIssueTriage and PullRequestTriage rules
JimSuplizio Mar 27, 2023
dbaa0c5
Stop using IssueUpdate for actions processing
JimSuplizio Mar 28, 2023
8262ed7
Update message to change added to removed
JimSuplizio Mar 28, 2023
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
30 changes: 30 additions & 0 deletions ...ithub-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/MockGitHubEventClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,18 @@ public override Task<int> ProcessPendingUpdates(long repositoryId, int issueOrPu
{
Console.WriteLine("MockGitHubEventClient::ProcessPendingUpdates, Issue Update is null");
}

if (_labelsToAdd.Count > 0)
{
Console.WriteLine($"MockGitHubEventClient::ProcessPendingUpdates, number of labels to add = {_labelsToAdd.Count} (only 1 call)");
// Adding labels is a single call to add them all
numUpdates++;
}

Console.WriteLine($"MockGitHubEventClient::ProcessPendingUpdates, number of labels to remove = {_labelsToRemove.Count}");
// Removing labels is a call for each one being removed
numUpdates += _labelsToRemove.Count;

Console.WriteLine($"MockGitHubEventClient::ProcessPendingUpdates, number of pending comments = {_gitHubComments.Count}");
numUpdates += _gitHubComments.Count;

Expand Down Expand Up @@ -434,6 +446,24 @@ public IssueUpdate GetIssueUpdate()
return _issueUpdate;
}

/// <summary>
/// Convenience function for testing, get labels to add stored on the GitHubEventClient
/// </summary>
/// <returns>List of strings</returns>
public List<string> GetLabelsToAdd()
{
return _labelsToAdd;
}

/// <summary>
/// Convenience function for testing, get labels to remove stored on the GitHubEventClient
/// </summary>
/// <returns>List of strings</returns>
public List<string> GetLabelsToRemove()
{
return _labelsToRemove;
}

/// <summary>
/// Convenience function for testing, get the list of GitHub issues to update. For normal action
/// processing this list won't be used as actions make changes to a common IssueUpdate. For scheduled,
Expand Down
73 changes: 27 additions & 46 deletions ...rocessor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/IssueCommentProcessingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,25 +38,18 @@ public async Task TestAuthorFeedback(string rule, string payloadFile, RuleState
Assert.AreEqual(ruleState == RuleState.On, mockGitHubEventClient.RulesConfiguration.RuleEnabled(rule), $"Rule '{rule}' enabled should have been {ruleState == RuleState.On} but RuleEnabled returned {ruleState != RuleState.On}.'");

var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueCommentPayload.Repository.Id, issueCommentPayload.Issue.Number);
var numComments = mockGitHubEventClient.GetComments().Count;
if (RuleState.On == ruleState)
{
// There should be two updates, an IssueUpdate with the label updates and a single comment
Assert.AreEqual(2, totalUpdates, $"The number of updates should have been 2 but was instead, {totalUpdates}");
Assert.AreEqual(1, numComments, $"{rule} should have created a single comment but {numComments} comments were created.");

// Retrieve the IssueUpdate and verify the expected changes
var issueUpdate = mockGitHubEventClient.GetIssueUpdate();
Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NeedsTeamAttention} removed and {LabelConstants.NeedsAuthorFeedback} added.");
// There should be 2 updates, one label being added and one
Assert.AreEqual(2, totalUpdates, $"The number of updates should have been 3 but was instead, {totalUpdates}");
// Verify that NeedsTeamAttention was added
Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamAttention), $"IssueUpdate does not contain {LabelConstants.NeedsTeamAttention} label whcih should have been added.");
Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTeamAttention), $"Labels to Add list does not contain {LabelConstants.NeedsTeamAttention}.");
// Verify that NeedsAuthorFeedback was removed
Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsAuthorFeedback), $"IssueUpdate contains {LabelConstants.NeedsAuthorFeedback} label which should have been removed.");
Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsAuthorFeedback), $"Lables to Remove list does not contain {LabelConstants.NeedsAuthorFeedback}.");
}
else
{
Assert.AreEqual(0, numComments, $"{rule} is {ruleState} and should not have created any comments but {numComments} comments were created.");
Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate.");
Assert.AreEqual(0, totalUpdates, $"{rule} is {ruleState} and should not have been any updates but {totalUpdates} comments were created.");
}
}

Expand Down Expand Up @@ -84,19 +77,15 @@ public async Task TestResetIssueActivity(string rule, string payloadFile, RuleSt
var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueCommentPayload.Repository.Id, issueCommentPayload.Issue.Number);
if (RuleState.On == ruleState)
{
// There should be one update, an IssueUpdate with the label NoRecentActivity removed
// There should be one update, the label NoRecentActivity removed
Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}");

// Retrieve the IssueUpdate and verify the expected changes
var issueUpdate = mockGitHubEventClient.GetIssueUpdate();
Assert.IsNotNull(issueUpdate, $"IssueUpdate is null. {rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NoRecentActivity} removed.");
// Verify that NoRecentActivity was removed
Assert.False(issueUpdate.Labels.Contains(LabelConstants.NoRecentActivity), $"IssueUpdate contains {LabelConstants.NoRecentActivity} label which should have been removed.");
// Verify that NoRecentActivity is in the remove list
Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NoRecentActivity), $"Labels to remove list does not contain {LabelConstants.NoRecentActivity}.");
}
else
{
Assert.AreEqual(0, totalUpdates, $"The number of updates should have been 0 but was instead, {totalUpdates}");
Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate.");
}
}

Expand Down Expand Up @@ -125,25 +114,24 @@ public async Task TestReopenIssue(string rule, string payloadFile, RuleState rul
var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueCommentPayload.Repository.Id, issueCommentPayload.Issue.Number);
if (RuleState.On == ruleState)
{
// There should be one update, an IssueUpdate with the labels NeedsAuthorFeedback and NoRecentActivity removed
// and NeedsTeamAttention added
Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}");
// There should be 4 updates, the issue state change, 1 label added and 2 labels removed
Assert.AreEqual(4, totalUpdates, $"The number of updates should have been 4 but was instead, {totalUpdates}");

// Retrieve the IssueUpdate and verify the expected changes
var issueUpdate = mockGitHubEventClient.GetIssueUpdate();
Assert.IsNotNull(issueUpdate, $"IssueUpdate is null. {rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NeedsAuthorFeedback} and {LabelConstants.NoRecentActivity} removed and {LabelConstants.NeedsTeamAttention} added.");
Assert.IsNotNull(issueUpdate, $"IssueUpdate is null. {rule} is {ruleState} and should have produced an IssueUpdate with its State being {ItemState.Open}.");
Assert.AreEqual(issueUpdate.State, ItemState.Open, $"Issue's State should be {ItemState.Open} but was {issueUpdate.State}");

// Verify that the NeedsTeamAttention label was added
Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamAttention), $"IssueUpdate does not contain {LabelConstants.NeedsTeamAttention} label which should have been added.");
Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTeamAttention), $"Labels to add should contain {LabelConstants.NeedsTeamAttention} and does not.");

// Verify that NeedsAuthorFeedback and NoRecentActivity labels were removed
Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsAuthorFeedback), $"IssueUpdate contains {LabelConstants.NeedsAuthorFeedback} label which should have been removed.");
Assert.False(issueUpdate.Labels.Contains(LabelConstants.NoRecentActivity), $"IssueUpdate contains {LabelConstants.NoRecentActivity} label which should have been removed.");
Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsAuthorFeedback), $"Lables to remove should contain {LabelConstants.NeedsAuthorFeedback} and does not.");
Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NoRecentActivity), $"Lables to remove should contain {LabelConstants.NoRecentActivity} and does not.");
}
else
{
Assert.AreEqual(0, totalUpdates, $"The number of updates should have been 0 but was instead, {totalUpdates}");
Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate.");
Assert.AreEqual(0, totalUpdates, $"{rule} is {ruleState} and should not have produced any updates but produced {totalUpdates} updates.");
}
}

Expand Down Expand Up @@ -221,24 +209,20 @@ public async Task TestIssueAddressedCommands_SameUser(string rule, string payloa
var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueCommentPayload.Repository.Id, issueCommentPayload.Issue.Number);
if (RuleState.On == ruleState)
{
// There should only be 1 update, the IssueUpdate
Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}");
// There should 3 updates, the IssueUpdate with State set to ItemState.Open, 1 label removed and 1 added
Assert.AreEqual(3, totalUpdates, $"The number of updates should have been 3 but was instead, {totalUpdates}");

var issueUpdate = mockGitHubEventClient.GetIssueUpdate();
// Verify the IssueUpdate is not null
Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate.");
// Verify the IssueUpdate contains the following changes:
// State = ItemState.Open
// Verify the IssueUpdate's State = ItemState.Open
Assert.AreEqual(issueUpdate.State, ItemState.Open, $"IssueUpdate's state should be ItemState.Open and was not.");
// IssueAddressed label has been removed
Assert.False(issueUpdate.Labels.Contains(LabelConstants.IssueAddressed), $"IssueUpdate contains {LabelConstants.IssueAddressed} label which should have been removed.");
Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.IssueAddressed), $"Labels to remove should contain {LabelConstants.IssueAddressed} and does not.");
// NeedsTeamAttention has been added
Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamAttention), $"IssueUpdate does not contain {LabelConstants.NeedsTeamAttention} label which should have been added.");
Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTeamAttention), $"Labels to add should contain {LabelConstants.NeedsTeamAttention} and does not.");
}
else
{
Assert.AreEqual(0, totalUpdates, $"The number of updates should have been 0 but was instead, {totalUpdates}");
Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate.");
}
}

Expand Down Expand Up @@ -268,26 +252,23 @@ public async Task TestIssueAddressedCommands_DifferentUser(string rule, string p
var numComments = mockGitHubEventClient.GetComments().Count;
if (userHasAdminOrWritePermission)
{
// There should only be 1 update, the IssueUpdate
Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}");

// There should 3 updates, the IssueUpdate with State set to ItemState.Open, 1 label removed and 1 added
Assert.AreEqual(3, totalUpdates, $"The number of updates should have been 3 but was instead, {totalUpdates}");

var issueUpdate = mockGitHubEventClient.GetIssueUpdate();
// Verify the IssueUpdate is not null
Assert.IsNotNull(issueUpdate, $"{rule} is should have produced an IssueUpdate.");
// Verify the IssueUpdate contains the following changes:
// State = ItemState.Open
// Verify the IssueUpdate's State = ItemState.Open
Assert.AreEqual(issueUpdate.State, ItemState.Open, $"IssueUpdate's state should be ItemState.Open and was not.");
// IssueAddressed label has been removed
Assert.False(issueUpdate.Labels.Contains(LabelConstants.IssueAddressed), $"IssueUpdate contains {LabelConstants.IssueAddressed} label which should have been removed.");
Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.IssueAddressed), $"Labels to remove should contain {LabelConstants.IssueAddressed} and does not.");
// NeedsTeamAttention has been added
Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamAttention), $"IssueUpdate does not contain {LabelConstants.NeedsTeamAttention} label which should have been added.");
Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTeamAttention), $"Labels to add should contain {LabelConstants.NeedsTeamAttention} and does not.");
}
else
{
// If the user does not have admin or write permission then there should be a single comment
Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}");
Assert.AreEqual(1, numComments, $"Without admin or write permissions, 1 comment should have been created but {numComments} comments were created.");
Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"Without admin or write permissions there should not be an IssueUpdate.");
}
}
}
Expand Down
Loading