From 018b6d6d072994d96be2962e943b41fdb66bbaea Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Mon, 17 Mar 2025 23:18:36 -0400 Subject: [PATCH] Initial pass fixing gh issue and gh pr comment There is still a bit of work to get the gh pr comment tests in order, however this goes a way towards fixing the issue along with acceptance tests. Also, it turns out some of the issue acceptance tests were really running `pr` tests. --- acceptance/acceptance_test.go | 2 +- ...ssue-comment-edit-last-with-comments.txtar | 31 +++++ ...t-edit-last-without-comments-creates.txtar | 23 ++++ ...nt-edit-last-without-comments-errors.txtar | 20 ++++ ...-comment.txtar => issue-comment-new.txtar} | 11 +- .../pr-comment-edit-last-with-comments.txtar | 40 +++++++ ...t-edit-last-without-comments-creates.txtar | 32 +++++ ...nt-edit-last-without-comments-errors.txtar | 29 +++++ ...{pr-comment.txtar => pr-comment-new.txtar} | 11 +- pkg/cmd/issue/comment/comment_test.go | 109 +++++++++++------- pkg/cmd/pr/shared/commentable.go | 47 +++++--- 11 files changed, 285 insertions(+), 70 deletions(-) create mode 100644 acceptance/testdata/issue/issue-comment-edit-last-with-comments.txtar create mode 100644 acceptance/testdata/issue/issue-comment-edit-last-without-comments-creates.txtar create mode 100644 acceptance/testdata/issue/issue-comment-edit-last-without-comments-errors.txtar rename acceptance/testdata/issue/{issue-comment.txtar => issue-comment-new.txtar} (64%) create mode 100644 acceptance/testdata/pr/pr-comment-edit-last-with-comments.txtar create mode 100644 acceptance/testdata/pr/pr-comment-edit-last-without-comments-creates.txtar create mode 100644 acceptance/testdata/pr/pr-comment-edit-last-without-comments-errors.txtar rename acceptance/testdata/pr/{pr-comment.txtar => pr-comment-new.txtar} (72%) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index b22929ed5..b8eff8389 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -69,7 +69,7 @@ func TestIssues(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "pr")) + testscript.Run(t, testScriptParamsFor(tsEnv, "issue")) } func TestLabels(t *testing.T) { diff --git a/acceptance/testdata/issue/issue-comment-edit-last-with-comments.txtar b/acceptance/testdata/issue/issue-comment-edit-last-with-comments.txtar new file mode 100644 index 000000000..d63c2e17a --- /dev/null +++ b/acceptance/testdata/issue/issue-comment-edit-last-with-comments.txtar @@ -0,0 +1,31 @@ +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$REPO --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$REPO + +# Clone the repo +exec gh repo clone $ORG/$REPO + +# Create an issue in the repo +cd $REPO +exec gh issue create --title 'Feature Request' --body 'Feature Body' +stdout2env ISSUE_URL + +# Comment on the issue +exec gh issue comment $ISSUE_URL --body 'Looks like a great feature!' + +# View the issue +exec gh issue view $ISSUE_URL --comments +stdout 'Looks like a great feature!' + +# Edit the last comment +exec gh issue comment $ISSUE_URL --edit-last --body 'Looks like an amazing feature!' + +# View the issue +exec gh issue view $ISSUE_URL --comments +! stdout 'Looks like a great feature!' +stdout 'Looks like an amazing feature!' diff --git a/acceptance/testdata/issue/issue-comment-edit-last-without-comments-creates.txtar b/acceptance/testdata/issue/issue-comment-edit-last-without-comments-creates.txtar new file mode 100644 index 000000000..6dc09d4db --- /dev/null +++ b/acceptance/testdata/issue/issue-comment-edit-last-without-comments-creates.txtar @@ -0,0 +1,23 @@ +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$REPO --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$REPO + +# Clone the repo +exec gh repo clone $ORG/$REPO + +# Create an issue in the repo +cd $REPO +exec gh issue create --title 'Feature Request' --body 'Feature Body' +stdout2env ISSUE_URL + +# Comment on the issue +exec gh issue comment $ISSUE_URL --edit-last --create-if-none --body 'Looks like a great feature!' + +# View the issue +exec gh issue view $ISSUE_URL --comments +stdout 'Looks like a great feature!' diff --git a/acceptance/testdata/issue/issue-comment-edit-last-without-comments-errors.txtar b/acceptance/testdata/issue/issue-comment-edit-last-without-comments-errors.txtar new file mode 100644 index 000000000..050ff92d5 --- /dev/null +++ b/acceptance/testdata/issue/issue-comment-edit-last-without-comments-errors.txtar @@ -0,0 +1,20 @@ +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$REPO --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$REPO + +# Clone the repo +exec gh repo clone $ORG/$REPO + +# Create an issue in the repo +cd $REPO +exec gh issue create --title 'Feature Request' --body 'Feature Body' +stdout2env ISSUE_URL + +# Comment on the issue +! exec gh issue comment $ISSUE_URL --edit-last --body 'Looks like a great feature!' +stderr 'no comments found for current user' diff --git a/acceptance/testdata/issue/issue-comment.txtar b/acceptance/testdata/issue/issue-comment-new.txtar similarity index 64% rename from acceptance/testdata/issue/issue-comment.txtar rename to acceptance/testdata/issue/issue-comment-new.txtar index f47bf619c..2ce318629 100644 --- a/acceptance/testdata/issue/issue-comment.txtar +++ b/acceptance/testdata/issue/issue-comment-new.txtar @@ -1,14 +1,17 @@ +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + # Create a repository with a file so it has a default branch -exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private +exec gh repo create $ORG/$REPO --add-readme --private # Defer repo cleanup -defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING +defer gh repo delete --yes $ORG/$REPO # Clone the repo -exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING +exec gh repo clone $ORG/$REPO # Create an issue in the repo -cd $SCRIPT_NAME-$RANDOM_STRING +cd $REPO exec gh issue create --title 'Feature Request' --body 'Feature Body' stdout2env ISSUE_URL diff --git a/acceptance/testdata/pr/pr-comment-edit-last-with-comments.txtar b/acceptance/testdata/pr/pr-comment-edit-last-with-comments.txtar new file mode 100644 index 000000000..5b371978c --- /dev/null +++ b/acceptance/testdata/pr/pr-comment-edit-last-with-comments.txtar @@ -0,0 +1,40 @@ +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$REPO --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$REPO + +# Clone the repo +exec gh repo clone $ORG/$REPO + +# Prepare a branch to PR +cd $REPO +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# Comment on the PR +exec gh pr comment $PR_URL --body 'Looks like a great feature!' + +# View the PR +exec gh pr view $PR_URL --comments +stdout 'Looks like a great feature!' + +# Edit the last comment +exec gh pr comment $PR_URL --edit-last --body 'Looks like an amazing feature!' + +# View the PR +exec gh pr view $PR_URL --comments +! stdout 'Looks like a great feature!' +stdout 'Looks like an amazing feature!' diff --git a/acceptance/testdata/pr/pr-comment-edit-last-without-comments-creates.txtar b/acceptance/testdata/pr/pr-comment-edit-last-without-comments-creates.txtar new file mode 100644 index 000000000..4f2039f9d --- /dev/null +++ b/acceptance/testdata/pr/pr-comment-edit-last-without-comments-creates.txtar @@ -0,0 +1,32 @@ +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$REPO --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$REPO + +# Clone the repo +exec gh repo clone $ORG/$REPO + +# Prepare a branch to PR +cd $REPO +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# Comment on the PR +exec gh pr comment $PR_URL --edit-last --create-if-none --body 'Looks like a great feature!' + +# View the PR +exec gh pr view $PR_URL --comments +stdout 'Looks like a great feature!' diff --git a/acceptance/testdata/pr/pr-comment-edit-last-without-comments-errors.txtar b/acceptance/testdata/pr/pr-comment-edit-last-without-comments-errors.txtar new file mode 100644 index 000000000..8f28db5cd --- /dev/null +++ b/acceptance/testdata/pr/pr-comment-edit-last-without-comments-errors.txtar @@ -0,0 +1,29 @@ +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$REPO --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$REPO + +# Clone the repo +exec gh repo clone $ORG/$REPO + +# Prepare a branch to PR +cd $REPO +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# Comment on the PR +! exec gh pr comment $PR_URL --edit-last --body 'Looks like a great feature!' +stderr 'no comments found for current user' diff --git a/acceptance/testdata/pr/pr-comment.txtar b/acceptance/testdata/pr/pr-comment-new.txtar similarity index 72% rename from acceptance/testdata/pr/pr-comment.txtar rename to acceptance/testdata/pr/pr-comment-new.txtar index 2c4d488b5..9856c0430 100644 --- a/acceptance/testdata/pr/pr-comment.txtar +++ b/acceptance/testdata/pr/pr-comment-new.txtar @@ -1,17 +1,20 @@ +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + # Use gh as a credential helper exec gh auth setup-git # Create a repository with a file so it has a default branch -exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private +exec gh repo create $ORG/$REPO --add-readme --private # Defer repo cleanup -defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING +defer gh repo delete --yes $ORG/$REPO # Clone the repo -exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING +exec gh repo clone $ORG/$REPO # Prepare a branch to PR -cd $SCRIPT_NAME-$RANDOM_STRING +cd $REPO exec git checkout -b feature-branch exec git commit --allow-empty -m 'Empty Commit' exec git push -u origin feature-branch diff --git a/pkg/cmd/issue/comment/comment_test.go b/pkg/cmd/issue/comment/comment_test.go index 461ae1065..c7feaf698 100644 --- a/pkg/cmd/issue/comment/comment_test.go +++ b/pkg/cmd/issue/comment/comment_test.go @@ -223,9 +223,10 @@ func Test_commentRun(t *testing.T) { httpStubs func(*testing.T, *httpmock.Registry) stdout string stderr string + wantsErr bool }{ { - name: "interactive editor", + name: "creating new issue comment with interactive editor succeeds", input: &shared.CommentableOptions{ Interactive: true, InputType: 0, @@ -234,13 +235,29 @@ func Test_commentRun(t *testing.T) { InteractiveEditSurvey: func(string) (string, error) { return "comment body", nil }, ConfirmSubmitSurvey: func() (bool, error) { return true, nil }, }, + emptyComments: true, httpStubs: func(t *testing.T, reg *httpmock.Registry) { mockCommentCreate(t, reg) }, stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n", }, { - name: "interactive editor with edit last", + name: "updating last issue comment with interactive editor fails if there are no comments and decline prompt to create", + input: &shared.CommentableOptions{ + Interactive: true, + InputType: 0, + Body: "", + EditLast: true, + + InteractiveEditSurvey: func(string) (string, error) { return "comment body", nil }, + ConfirmSubmitSurvey: func() (bool, error) { return true, nil }, + ConfirmCreateIfNoneSurvey: func() (bool, error) { return false, nil }, + }, + emptyComments: true, + wantsErr: true, + }, + { + name: "updating last issue comment with interactive editor succeeds if there are comments", input: &shared.CommentableOptions{ Interactive: true, InputType: 0, @@ -253,10 +270,11 @@ func Test_commentRun(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { mockCommentUpdate(t, reg) }, - stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n", + emptyComments: false, + stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n", }, { - name: "interactive editor with edit last and create if none", + name: "updating last issue comment with interactive editor creates new comment if there are no comments but --create-if-none", input: &shared.CommentableOptions{ Interactive: true, InputType: 0, @@ -269,12 +287,14 @@ func Test_commentRun(t *testing.T) { ConfirmSubmitSurvey: func() (bool, error) { return true, nil }, }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - mockCommentUpdate(t, reg) + mockCommentCreate(t, reg) }, - stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n", + emptyComments: true, + stderr: "No comments found. Creating a new comment.\n", + stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n", }, { - name: "non-interactive web", + name: "creating new issue comment with non-interactive web opens issue in browser focusing on new comment", input: &shared.CommentableOptions{ Interactive: false, InputType: shared.InputTypeWeb, @@ -282,39 +302,11 @@ func Test_commentRun(t *testing.T) { OpenInBrowser: func(string) error { return nil }, }, - stderr: "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n", - }, - { - name: "non-interactive web with edit last", - input: &shared.CommentableOptions{ - Interactive: false, - InputType: shared.InputTypeWeb, - Body: "", - EditLast: true, - - OpenInBrowser: func(string) error { return nil }, - }, - stderr: "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n", - }, - { - name: "non-interactive web with edit last and create if none for empty comments", - input: &shared.CommentableOptions{ - Interactive: false, - InputType: shared.InputTypeWeb, - Body: "", - EditLast: true, - CreateIfNone: true, - - OpenInBrowser: func(u string) error { - assert.Contains(t, u, "#issuecomment-new") - return nil - }, - }, emptyComments: true, stderr: "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n", }, { - name: "non-interactive web with edit last and create if none", + name: "updating last issue comment with non-interactive web opens issue in browser focusing on the last comment", input: &shared.CommentableOptions{ Interactive: false, InputType: shared.InputTypeWeb, @@ -330,7 +322,19 @@ func Test_commentRun(t *testing.T) { stderr: "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n", }, { - name: "non-interactive editor", + name: "updating last issue comment with non-interactive web opens issue in browser focusing on new comment if there are no comments", + input: &shared.CommentableOptions{ + Interactive: false, + InputType: shared.InputTypeWeb, + Body: "", + + OpenInBrowser: func(string) error { return nil }, + }, + emptyComments: true, + stderr: "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n", + }, + { + name: "creating new issue comment with non-interactive editor succeeds", input: &shared.CommentableOptions{ Interactive: false, InputType: shared.InputTypeEditor, @@ -344,7 +348,20 @@ func Test_commentRun(t *testing.T) { stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n", }, { - name: "non-interactive editor with edit last", + name: "updating last issue comment with non-interactive editor fails if there are no comments", + input: &shared.CommentableOptions{ + Interactive: false, + InputType: shared.InputTypeEditor, + Body: "", + EditLast: true, + + EditSurvey: func(string) (string, error) { return "comment body", nil }, + }, + emptyComments: true, + wantsErr: true, + }, + { + name: "updating last issue comment with non-interactive editor succeeds if there are comments", input: &shared.CommentableOptions{ Interactive: false, InputType: shared.InputTypeEditor, @@ -356,10 +373,11 @@ func Test_commentRun(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { mockCommentUpdate(t, reg) }, - stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n", + emptyComments: false, + stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n", }, { - name: "non-interactive editor with edit last and create if none", + name: "updating last issue comment with non-interactive editor creates new issue if there are no comments but --create-if-none", input: &shared.CommentableOptions{ Interactive: false, InputType: shared.InputTypeEditor, @@ -376,7 +394,7 @@ func Test_commentRun(t *testing.T) { stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n", }, { - name: "non-interactive inline", + name: "creating new issue comment with non-interactive succeeds if comment body is provided", input: &shared.CommentableOptions{ Interactive: false, InputType: shared.InputTypeInline, @@ -388,7 +406,7 @@ func Test_commentRun(t *testing.T) { stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n", }, { - name: "non-interactive inline with edit last", + name: "updating last issue comment with non-interactive succeeds if there are comments and comment body is provided", input: &shared.CommentableOptions{ Interactive: false, InputType: shared.InputTypeInline, @@ -398,7 +416,8 @@ func Test_commentRun(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { mockCommentUpdate(t, reg) }, - stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n", + emptyComments: false, + stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n", }, } for _, tt := range tests { @@ -437,6 +456,10 @@ func Test_commentRun(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := shared.CommentableRun(tt.input) + if tt.wantsErr { + assert.Error(t, err) + return + } assert.NoError(t, err) assert.Equal(t, tt.stdout, stdout.String()) assert.Equal(t, tt.stderr, stderr.String()) diff --git a/pkg/cmd/pr/shared/commentable.go b/pkg/cmd/pr/shared/commentable.go index 46e3072e2..c10c4befa 100644 --- a/pkg/cmd/pr/shared/commentable.go +++ b/pkg/cmd/pr/shared/commentable.go @@ -92,27 +92,38 @@ func CommentableRun(opts *CommentableOptions) error { return err } opts.Host = repo.RepoHost() - if opts.EditLast { - err := updateComment(commentable, opts) - if !errors.Is(err, errNoUserComments) { + + // Create new comment, bail before complexities of updating the last comment + if !opts.EditLast { + return createComment(commentable, opts) + } + + // Update the last comment, handling success or unexpected errors accordingly + err = updateComment(commentable, opts) + if err == nil { + return nil + } + if !errors.Is(err, errNoUserComments) { + return err + } + + // Determine whether to create new comment, prompt user if interactive and missing option + if !opts.CreateIfNone && opts.Interactive { + opts.CreateIfNone, err = opts.ConfirmCreateIfNoneSurvey() + if err != nil { return err } - - if opts.Interactive { - if opts.CreateIfNone { - fmt.Fprintln(opts.IO.ErrOut, "No comments found. Creating a new comment.") - } else { - ok, err := opts.ConfirmCreateIfNoneSurvey() - if err != nil { - return err - } - if !ok { - return errNoUserComments - } - } - } } - return createComment(commentable, opts) + + // Create new comment because updating the last comment failed due to no user comments + if opts.CreateIfNone { + if opts.Interactive { + fmt.Fprintln(opts.IO.ErrOut, "No comments found. Creating a new comment.") + } + return createComment(commentable, opts) + } + + return errNoUserComments } func createComment(commentable Commentable, opts *CommentableOptions) error {