Skip to content

Commit 7ef4293

Browse files
Copilotsilverwind
andcommitted
Improve error handling and fix test realism
- Use IsErrPullRequestNotExist for proper error handling - Remove redundant nil check (guaranteed after successful load) - Fix test to use different commit SHAs (realistic Git scenario) - All tests still passing Co-authored-by: silverwind <115237+silverwind@users.noreply.github.com>
1 parent 69a0ecf commit 7ef4293

2 files changed

Lines changed: 10 additions & 7 deletions

File tree

services/issue/commit.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,10 @@ func UpdateIssuesCommit(ctx context.Context, doer *user_model.User, repo *repo_m
161161
// Skip self-references: if this commit is the merge commit of the PR it references
162162
if refIssue.IsPull {
163163
if err := refIssue.LoadPullRequest(ctx); err != nil {
164-
log.Error("LoadPullRequest: %v", err)
165-
} else if refIssue.PullRequest != nil && refIssue.PullRequest.MergedCommitID == c.Sha1 {
164+
if !issues_model.IsErrPullRequestNotExist(err) {
165+
log.Error("LoadPullRequest: %v", err)
166+
}
167+
} else if refIssue.PullRequest.MergedCommitID == c.Sha1 {
166168
// This is a self-reference (PR merge commit referencing its own PR), skip it
167169
continue
168170
}

services/issue/commit_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -332,22 +332,23 @@ func TestUpdateIssuesCommit_SelfReference(t *testing.T) {
332332
unittest.AssertNotExistsBean(t, selfRefCommentBean)
333333
unittest.CheckConsistencyFor(t, &activities_model.Action{})
334334

335-
// Test that the same merge commit can still create references to other issues
335+
// Test that normal (non-self-referencing) commit comments are still created
336+
// Use a different commit that is NOT a merge commit for any PR
336337
pushCommits2 := []*repository.PushCommit{
337338
{
338-
Sha1: "1a8823cd1a9549fde083f992f6b9b87a7ab74fb3",
339+
Sha1: "abcdef9876543210", // Different commit, not a merge commit
339340
CommitterEmail: "user2@example.com",
340341
CommitterName: "User Two",
341342
AuthorEmail: "user2@example.com",
342343
AuthorName: "User Two",
343-
Message: "Merge pull request (#2) - also fixes #1", // References a different issue
344+
Message: "Fix bug, refs #1", // References issue #1
344345
},
345346
}
346347

347-
// This comment SHOULD be created (reference to a different issue)
348+
// This comment SHOULD be created (reference to a different issue from a non-merge commit)
348349
otherRefCommentBean := &issues_model.Comment{
349350
Type: issues_model.CommentTypeCommitRef,
350-
CommitSHA: "1a8823cd1a9549fde083f992f6b9b87a7ab74fb3",
351+
CommitSHA: "abcdef9876543210",
351352
PosterID: user.ID,
352353
IssueID: 1, // References issue #1
353354
}

0 commit comments

Comments
 (0)