-
Notifications
You must be signed in to change notification settings - Fork 475
fix: add generated footer to close_issue comment body #47584
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -383,6 +383,8 @@ describe("close_issue", () => { | |
| expect(result.success).toBe(true); | ||
| expect(commentCalls.length).toBe(1); | ||
| expect(commentCalls[0].body).toContain("This issue is being closed automatically"); | ||
| expect(commentCalls[0].body).toContain("for #123"); | ||
| expect(commentCalls[0].body).toContain("gh-aw-agentic-workflow:"); | ||
| }); | ||
|
|
||
| it("should use body field from message over config comment", async () => { | ||
|
|
@@ -413,7 +415,9 @@ describe("close_issue", () => { | |
| expect(result.success).toBe(true); | ||
| expect(commentCalls.length).toBe(1); | ||
| // Should use the body from message, not the config comment | ||
| expect(commentCalls[0].body).toBe("Custom body from message"); | ||
| expect(commentCalls[0].body).toContain("Custom body from message"); | ||
| expect(commentCalls[0].body).toContain("for #123"); | ||
| expect(commentCalls[0].body).toContain("gh-aw-agentic-workflow:"); | ||
| expect(commentCalls[0].body).not.toContain("Default comment from config"); | ||
| }); | ||
|
|
||
|
|
@@ -506,7 +510,7 @@ describe("close_issue", () => { | |
|
|
||
| expect(result.success).toBe(true); | ||
| expect(result.alreadyClosed).toBe(true); | ||
| expect(commentBody).toBe("Closing comment with details"); | ||
| expect(commentBody).toContain("Closing comment with details"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/diagnosing-bugs] Same weakened assertion as line 416 — 💡 Suggested fixexpect(commentBody).toContain("Closing comment with details");
// confirm footer was appended
expect(commentBody.length).toBeGreaterThan("Closing comment with details".length);Or match a known footer marker string from @copilot please address this. |
||
| expect(issueUpdateCalled).toBe(false); // Should not call update for already closed issue | ||
| }); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same concern as line 416:
.toContain(body)alone doesn't confirm the footer was appended. The test would still pass ifbuildCommentBodyis reverted to returningsanitizedBodydirectly. Add an assertion on a footer-specific token, e.g.:@copilot please address this.