From 11130fd6bed281a99b554cb9ddc9d32eed48d984 Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Mon, 27 Apr 2026 15:42:36 -0500 Subject: [PATCH 1/8] test(discussion view): add tests for view command client methods and --replies mode Add table-driven tests for: Client (client_impl_test.go): - TestGetByNumber: field mapping, discussions disabled - TestGetWithComments: field mapping, forward/backward pagination, reply reversal in newest mode, discussions disabled - TestGetCommentReplies: field mapping, forward/backward pagination, reply reversal, discussions disabled, nil node, wrong node type Command (view_test.go): - TestNewCmdView_repliesFlags: mutual exclusivity with --comments/--web, --order/--limit/--after require --comments or --replies, pagination flags work with --replies - TestViewRun_replies: TTY/non-TTY/JSON output, pagination hints, routing assertion (GetCommentReplies called, not GetByNumber or GetWithComments) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl_test.go | 575 ++++++++++++++++++ pkg/cmd/discussion/view/view_test.go | 285 +++++++++ 2 files changed, 860 insertions(+) diff --git a/pkg/cmd/discussion/client/client_impl_test.go b/pkg/cmd/discussion/client/client_impl_test.go index 0a6392798..462d84915 100644 --- a/pkg/cmd/discussion/client/client_impl_test.go +++ b/pkg/cmd/discussion/client/client_impl_test.go @@ -1006,3 +1006,578 @@ func TestListCategories(t *testing.T) { }) } } + +// --------------------------------------------------------------------------- +// GetByNumber +// --------------------------------------------------------------------------- + +// getByNumberResp builds a mock DiscussionMinimal JSON response. +func getByNumberResp(hasDiscussions bool, commentTotal int, node string) string { + return heredoc.Docf(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": %t, + "discussion": %s + } + } + } + `, hasDiscussions, wrapCommentTotal(node, commentTotal)) +} + +// wrapCommentTotal merges a `comments` block into a discussion JSON node. +func wrapCommentTotal(node string, total int) string { + // Insert "comments":{"totalCount": N} right before the closing brace. + trimmed := strings.TrimRight(node, " \t\n") + trimmed = trimmed[:len(trimmed)-1] // strip trailing } + return fmt.Sprintf(`%s, "comments": {"totalCount": %d}}`, trimmed, total) +} + +func TestGetByNumber(t *testing.T) { + repo := ghrepo.New("OWNER", "REPO") + + tests := []struct { + name string + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + wantDisc func(*testing.T, *Discussion) + }{ + { + name: "maps all fields", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + node := heredoc.Doc(` + { + "id": "D_1", + "number": 42, + "title": "Test Discussion", + "body": "This is a test", + "url": "https://github.com/OWNER/REPO/discussions/42", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [{"content": "THUMBS_UP", "users": {"totalCount": 3}}], + "createdAt": "2025-01-01T00:00:00Z", + "updatedAt": "2025-01-02T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false + } + `) + reg.Register( + httpmock.GraphQL(`query DiscussionMinimal\b`), + httpmock.StringResponse(getByNumberResp(true, 5, node)), + ) + }, + wantDisc: func(t *testing.T, d *Discussion) { + assert.Equal(t, "D_1", d.ID) + assert.Equal(t, 42, d.Number) + assert.Equal(t, "Test Discussion", d.Title) + assert.Equal(t, "This is a test", d.Body) + assert.Equal(t, "alice", d.Author.Login) + assert.Equal(t, 5, d.Comments.TotalCount) + require.Len(t, d.ReactionGroups, 1) + assert.Equal(t, "THUMBS_UP", d.ReactionGroups[0].Content) + assert.Equal(t, 3, d.ReactionGroups[0].TotalCount) + }, + }, + { + name: "discussions disabled", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + node := minimalNode("D_1", "Test") + reg.Register( + httpmock.GraphQL(`query DiscussionMinimal\b`), + httpmock.StringResponse(getByNumberResp(false, 0, node)), + ) + }, + wantErr: "discussions disabled", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + if tt.httpStubs != nil { + tt.httpStubs(t, reg) + } + + c := newTestDiscussionClient(reg) + d, err := c.GetByNumber(repo, 42) + + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + require.NotNil(t, d) + if tt.wantDisc != nil { + tt.wantDisc(t, d) + } + }) + } +} + +// --------------------------------------------------------------------------- +// GetWithComments +// --------------------------------------------------------------------------- + +// getWithCommentsResp builds a mock DiscussionWithComments JSON response. +func getWithCommentsResp(hasDiscussions bool, node string, commentNodes string, commentTotal int, hasNext, hasPrev bool, endCursor, startCursor string) string { + return heredoc.Docf(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": %t, + "discussion": %s + } + } + } + `, hasDiscussions, wrapCommentsBlock(node, commentNodes, commentTotal, hasNext, hasPrev, endCursor, startCursor)) +} + +func wrapCommentsBlock(node string, commentNodes string, total int, hasNext, hasPrev bool, endCursor, startCursor string) string { + trimmed := strings.TrimRight(node, " \t\n") + trimmed = trimmed[:len(trimmed)-1] + return fmt.Sprintf(`%s, "comments": {"totalCount": %d, "pageInfo": {"endCursor": %q, "hasNextPage": %t, "startCursor": %q, "hasPreviousPage": %t}, "nodes": [%s]}}`, + trimmed, total, endCursor, hasNext, startCursor, hasPrev, commentNodes) +} + +// commentNode builds a JSON comment node with nested replies. +func commentNode(id, login, body string, isAnswer bool, replyNodes string, replyTotal int) string { + return heredoc.Docf(` + { + "id": %q, + "url": "https://github.com/OWNER/REPO/discussions/1#comment-%s", + "author": {"__typename": "User", "login": %q}, + "body": %q, + "createdAt": "2025-01-01T00:00:00Z", + "isAnswer": %t, + "upvoteCount": 0, + "reactionGroups": [], + "replies": {"totalCount": %d, "nodes": [%s]} + } + `, id, id, login, body, isAnswer, replyTotal, replyNodes) +} + +// replyNode builds a JSON reply node. +func replyNode(id, login, body string) string { + return heredoc.Docf(` + { + "id": %q, + "url": "https://github.com/OWNER/REPO/discussions/1#reply-%s", + "author": {"__typename": "User", "login": %q}, + "body": %q, + "createdAt": "2025-02-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [] + } + `, id, id, login, body) +} + +func TestGetWithComments(t *testing.T) { + repo := ghrepo.New("OWNER", "REPO") + + tests := []struct { + name string + limit int + after string + newest bool + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + wantComments int + wantTotal int + wantCursor string + wantNext string + wantDirection DiscussionCommentListDirection + wantDisc func(*testing.T, *Discussion) + }{ + { + name: "maps comments with replies", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reply := replyNode("R1", "hubot", "Thanks!") + comment := commentNode("C1", "octocat", "Main comment", true, reply, 1) + node := minimalNode("D_1", "Test Discussion") + reg.Register( + httpmock.GraphQL(`query DiscussionWithComments\b`), + httpmock.StringResponse(getWithCommentsResp(true, node, comment, 1, false, false, "", "")), + ) + }, + wantComments: 1, + wantTotal: 1, + wantDirection: DiscussionCommentListDirectionForward, + wantDisc: func(t *testing.T, d *Discussion) { + c := d.Comments.Comments[0] + assert.Equal(t, "C1", c.ID) + assert.Equal(t, "octocat", c.Author.Login) + assert.True(t, c.IsAnswer) + require.Len(t, c.Replies.Comments, 1) + assert.Equal(t, "R1", c.Replies.Comments[0].ID) + assert.Equal(t, "hubot", c.Replies.Comments[0].Author.Login) + }, + }, + { + name: "pagination forward", + limit: 5, + after: "CUR_A", + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + comment := commentNode("C1", "alice", "Hello", false, "", 0) + node := minimalNode("D_1", "Test") + reg.Register( + httpmock.GraphQL(`query DiscussionWithComments\b`), + httpmock.StringResponse(getWithCommentsResp(true, node, comment, 3, true, false, "CUR_B", "")), + ) + }, + wantComments: 1, + wantTotal: 3, + wantCursor: "CUR_A", + wantNext: "CUR_B", + wantDirection: DiscussionCommentListDirectionForward, + }, + { + name: "pagination backward newest", + limit: 5, + after: "CUR_X", + newest: true, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + c1 := commentNode("C1", "alice", "First", false, "", 0) + c2 := commentNode("C2", "bob", "Second", false, "", 0) + node := minimalNode("D_1", "Test") + reg.Register( + httpmock.GraphQL(`query DiscussionWithComments\b`), + httpmock.StringResponse(getWithCommentsResp(true, node, c1+","+c2, 5, false, true, "", "CUR_Y")), + ) + }, + wantComments: 2, + wantTotal: 5, + wantCursor: "CUR_X", + wantNext: "CUR_Y", + wantDirection: DiscussionCommentListDirectionBackward, + wantDisc: func(t *testing.T, d *Discussion) { + // Newest mode reverses the order + assert.Equal(t, "C2", d.Comments.Comments[0].ID) + assert.Equal(t, "C1", d.Comments.Comments[1].ID) + }, + }, + { + name: "no more pages", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + comment := commentNode("C1", "alice", "Only one", false, "", 0) + node := minimalNode("D_1", "Test") + reg.Register( + httpmock.GraphQL(`query DiscussionWithComments\b`), + httpmock.StringResponse(getWithCommentsResp(true, node, comment, 1, false, false, "", "")), + ) + }, + wantComments: 1, + wantTotal: 1, + wantNext: "", + wantDirection: DiscussionCommentListDirectionForward, + }, + { + name: "discussions disabled", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + node := minimalNode("D_1", "Test") + reg.Register( + httpmock.GraphQL(`query DiscussionWithComments\b`), + httpmock.StringResponse(getWithCommentsResp(false, node, "", 0, false, false, "", "")), + ) + }, + wantErr: "discussions disabled", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + if tt.httpStubs != nil { + tt.httpStubs(t, reg) + } + + c := newTestDiscussionClient(reg) + d, err := c.GetWithComments(repo, 1, tt.limit, tt.after, tt.newest) + + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + require.NotNil(t, d) + assert.Len(t, d.Comments.Comments, tt.wantComments) + assert.Equal(t, tt.wantTotal, d.Comments.TotalCount) + assert.Equal(t, tt.wantCursor, d.Comments.Cursor) + assert.Equal(t, tt.wantNext, d.Comments.NextCursor) + assert.Equal(t, tt.wantDirection, d.Comments.Direction) + if tt.wantDisc != nil { + tt.wantDisc(t, d) + } + }) + } +} + +// --------------------------------------------------------------------------- +// GetCommentReplies +// --------------------------------------------------------------------------- + +// getCommentRepliesResp builds a mock DiscussionCommentReplies JSON response. +// The shurcooL graphql library treats inline fragments as transparent — the +// comment fields are placed directly inside the "node" object, not nested +// under a "DiscussionComment" key. +func getCommentRepliesResp(hasDiscussions bool, discNode string, commentNode *string, replyNodes string, replyTotal int, hasNext, hasPrev bool, endCursor, startCursor string) string { + nodeBlock := "null" + if commentNode != nil { + nodeBlock = wrapRepliesBlock(*commentNode, replyNodes, replyTotal, hasNext, hasPrev, endCursor, startCursor) + } + return heredoc.Docf(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": %t, + "discussion": %s + }, + "node": %s + } + } + `, hasDiscussions, discNode, nodeBlock) +} + +func wrapRepliesBlock(commentJSON string, replyNodes string, total int, hasNext, hasPrev bool, endCursor, startCursor string) string { + trimmed := strings.TrimRight(commentJSON, " \t\n") + trimmed = trimmed[:len(trimmed)-1] + return fmt.Sprintf(`%s, "replies": {"totalCount": %d, "pageInfo": {"endCursor": %q, "hasNextPage": %t, "startCursor": %q, "hasPreviousPage": %t}, "nodes": [%s]}}`, + trimmed, total, endCursor, hasNext, startCursor, hasPrev, replyNodes) +} + +// bareCommentNode builds a comment JSON node without replies (used for GetCommentReplies). +func bareCommentNode(id, login, body string, isAnswer bool) string { + return heredoc.Docf(` + { + "id": %q, + "url": "https://github.com/OWNER/REPO/discussions/1#comment-%s", + "author": {"__typename": "User", "login": %q}, + "body": %q, + "createdAt": "2025-01-01T00:00:00Z", + "isAnswer": %t, + "upvoteCount": 2, + "reactionGroups": [{"content": "HEART", "users": {"totalCount": 1}}] + } + `, id, id, login, body, isAnswer) +} + +func TestGetCommentReplies(t *testing.T) { + repo := ghrepo.New("OWNER", "REPO") + + tests := []struct { + name string + commentID string + limit int + after string + newest bool + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + wantReplies int + wantTotal int + wantCursor string + wantNext string + wantDirection DiscussionCommentListDirection + wantDisc func(*testing.T, *Discussion) + }{ + { + name: "maps all fields", + commentID: "DC_abc", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + discNode := minimalNode("D_1", "Test Discussion") + comment := bareCommentNode("DC_abc", "octocat", "Top-level comment", true) + reply := replyNode("R1", "hubot", "A reply") + reg.Register( + httpmock.GraphQL(`query DiscussionCommentReplies\b`), + httpmock.StringResponse(getCommentRepliesResp(true, discNode, &comment, reply, 1, false, false, "", "")), + ) + }, + wantReplies: 1, + wantTotal: 1, + wantDirection: DiscussionCommentListDirectionForward, + wantDisc: func(t *testing.T, d *Discussion) { + assert.Equal(t, "Test Discussion", d.Title) + require.Len(t, d.Comments.Comments, 1) + c := d.Comments.Comments[0] + assert.Equal(t, "DC_abc", c.ID) + assert.Equal(t, "octocat", c.Author.Login) + assert.Equal(t, "Top-level comment", c.Body) + assert.True(t, c.IsAnswer) + assert.Equal(t, 2, c.UpvoteCount) + require.Len(t, c.ReactionGroups, 1) + assert.Equal(t, "HEART", c.ReactionGroups[0].Content) + require.Len(t, c.Replies.Comments, 1) + assert.Equal(t, "R1", c.Replies.Comments[0].ID) + assert.Equal(t, "hubot", c.Replies.Comments[0].Author.Login) + }, + }, + { + name: "pagination forward oldest", + commentID: "DC_abc", + limit: 5, + after: "CUR_A", + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + discNode := minimalNode("D_1", "Test") + comment := bareCommentNode("DC_abc", "alice", "Comment", false) + r1 := replyNode("R1", "bob", "Reply 1") + reg.Register( + httpmock.GraphQL(`query DiscussionCommentReplies\b`), + httpmock.StringResponse(getCommentRepliesResp(true, discNode, &comment, r1, 3, true, false, "CUR_B", "")), + ) + }, + wantReplies: 1, + wantTotal: 3, + wantCursor: "CUR_A", + wantNext: "CUR_B", + wantDirection: DiscussionCommentListDirectionForward, + }, + { + name: "pagination backward newest reverses replies", + commentID: "DC_abc", + limit: 5, + after: "CUR_X", + newest: true, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + discNode := minimalNode("D_1", "Test") + comment := bareCommentNode("DC_abc", "alice", "Comment", false) + r1 := replyNode("R1", "bob", "Older") + r2 := replyNode("R2", "carol", "Newer") + reg.Register( + httpmock.GraphQL(`query DiscussionCommentReplies\b`), + httpmock.StringResponse(getCommentRepliesResp(true, discNode, &comment, r1+","+r2, 5, false, true, "", "CUR_Y")), + ) + }, + wantReplies: 2, + wantTotal: 5, + wantCursor: "CUR_X", + wantNext: "CUR_Y", + wantDirection: DiscussionCommentListDirectionBackward, + wantDisc: func(t *testing.T, d *Discussion) { + replies := d.Comments.Comments[0].Replies.Comments + assert.Equal(t, "R2", replies[0].ID, "newest mode should reverse replies") + assert.Equal(t, "R1", replies[1].ID) + }, + }, + { + name: "no more pages", + commentID: "DC_abc", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + discNode := minimalNode("D_1", "Test") + comment := bareCommentNode("DC_abc", "alice", "Comment", false) + r1 := replyNode("R1", "bob", "Only reply") + reg.Register( + httpmock.GraphQL(`query DiscussionCommentReplies\b`), + httpmock.StringResponse(getCommentRepliesResp(true, discNode, &comment, r1, 1, false, false, "", "")), + ) + }, + wantReplies: 1, + wantTotal: 1, + wantNext: "", + wantDirection: DiscussionCommentListDirectionForward, + }, + { + name: "discussions disabled", + commentID: "DC_abc", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + discNode := minimalNode("D_1", "Test") + comment := bareCommentNode("DC_abc", "alice", "Comment", false) + reg.Register( + httpmock.GraphQL(`query DiscussionCommentReplies\b`), + httpmock.StringResponse(getCommentRepliesResp(false, discNode, &comment, "", 0, false, false, "", "")), + ) + }, + wantErr: "discussions disabled", + }, + { + name: "node not found nil", + commentID: "DC_invalid", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + discNode := minimalNode("D_1", "Test") + reg.Register( + httpmock.GraphQL(`query DiscussionCommentReplies\b`), + httpmock.StringResponse(getCommentRepliesResp(true, discNode, nil, "", 0, false, false, "", "")), + ) + }, + wantErr: "comment DC_invalid not found", + }, + { + name: "node is not a discussion comment", + commentID: "I_notacomment", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + discNode := minimalNode("D_1", "Test") + // Return a node with an empty DiscussionComment (wrong type) + emptyComment := `{"id":"","url":"","author":{"__typename":"User","login":""},"body":"","createdAt":"0001-01-01T00:00:00Z","isAnswer":false,"upvoteCount":0,"reactionGroups":[]}` + reg.Register( + httpmock.GraphQL(`query DiscussionCommentReplies\b`), + httpmock.StringResponse(getCommentRepliesResp(true, discNode, &emptyComment, "", 0, false, false, "", "")), + ) + }, + wantErr: "node I_notacomment is not a discussion comment", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + if tt.httpStubs != nil { + tt.httpStubs(t, reg) + } + + c := newTestDiscussionClient(reg) + d, err := c.GetCommentReplies(repo, 1, tt.commentID, tt.limit, tt.after, tt.newest) + + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + require.NotNil(t, d) + require.Len(t, d.Comments.Comments, 1, "GetCommentReplies should return exactly one comment") + + comment := d.Comments.Comments[0] + assert.Len(t, comment.Replies.Comments, tt.wantReplies) + assert.Equal(t, tt.wantTotal, comment.Replies.TotalCount) + assert.Equal(t, tt.wantCursor, comment.Replies.Cursor) + assert.Equal(t, tt.wantNext, comment.Replies.NextCursor) + assert.Equal(t, tt.wantDirection, comment.Replies.Direction) + if tt.wantDisc != nil { + tt.wantDisc(t, d) + } + }) + } +} diff --git a/pkg/cmd/discussion/view/view_test.go b/pkg/cmd/discussion/view/view_test.go index 1ae739168..3f5e0348c 100644 --- a/pkg/cmd/discussion/view/view_test.go +++ b/pkg/cmd/discussion/view/view_test.go @@ -860,3 +860,288 @@ func TestViewRun_jsonWithoutComments_usesGetByNumber(t *testing.T) { assert.Equal(t, 1, len(mock.GetByNumberCalls())) assert.Equal(t, 0, len(mock.GetWithCommentsCalls())) } + +// --------------------------------------------------------------------------- +// --replies flag validation +// --------------------------------------------------------------------------- + +func TestNewCmdView_repliesFlags(t *testing.T) { + tests := []struct { + name string + args []string + wantErr string + }{ + { + name: "replies with comments is mutually exclusive", + args: []string{"123", "--replies", "DC_abc", "--comments"}, + wantErr: "specify only one of --comments, --replies, or --web", + }, + { + name: "replies with web is mutually exclusive", + args: []string{"123", "--replies", "DC_abc", "--web"}, + wantErr: "specify only one of --comments, --replies, or --web", + }, + { + name: "order requires comments or replies", + args: []string{"123", "--order", "newest"}, + wantErr: "--order requires --comments or --replies", + }, + { + name: "limit requires comments or replies", + args: []string{"123", "--limit", "5"}, + wantErr: "--limit requires --comments or --replies", + }, + { + name: "after requires comments or replies", + args: []string{"123", "--after", "CURSOR"}, + wantErr: "--after requires --comments or --replies", + }, + { + name: "order works with replies", + args: []string{"123", "--replies", "DC_abc", "--order", "oldest"}, + }, + { + name: "limit works with replies", + args: []string{"123", "--replies", "DC_abc", "--limit", "10"}, + }, + { + name: "after works with replies", + args: []string{"123", "--replies", "DC_abc", "--after", "CURSOR"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + f.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + } + f.Browser = &browser.Stub{} + + cmd := NewCmdView(f, func(opts *ViewOptions) error { + return nil + }) + + cmd.SetArgs(tt.args) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + err := cmd.Execute() + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + }) + } +} + +// --------------------------------------------------------------------------- +// --replies viewRun tests (table-driven) +// --------------------------------------------------------------------------- + +func testDiscussionWithReplies(nextCursor string) *client.Discussion { + d := testDiscussion() + d.Comments = client.DiscussionCommentList{ + TotalCount: 1, + Comments: []client.DiscussionComment{ + { + ID: "DC_abc", + URL: "https://github.com/OWNER/REPO/discussions/123#discussioncomment-1", + Author: client.DiscussionActor{Login: "octocat"}, + Body: "This is the parent comment", + CreatedAt: time.Date(2025, 3, 2, 0, 0, 0, 0, time.UTC), + IsAnswer: true, + ReactionGroups: []client.ReactionGroup{ + {Content: "THUMBS_UP", TotalCount: 3}, + }, + Replies: client.DiscussionCommentList{ + TotalCount: 2, + NextCursor: nextCursor, + Comments: []client.DiscussionComment{ + { + ID: "R1", + URL: "https://github.com/OWNER/REPO/discussions/123#discussioncomment-2", + Author: client.DiscussionActor{Login: "hubot"}, + Body: "First reply", + CreatedAt: time.Date(2025, 3, 2, 1, 0, 0, 0, time.UTC), + }, + { + ID: "R2", + URL: "https://github.com/OWNER/REPO/discussions/123#discussioncomment-3", + Author: client.DiscussionActor{Login: "monalisa"}, + Body: "Second reply", + CreatedAt: time.Date(2025, 3, 2, 2, 0, 0, 0, time.UTC), + }, + }, + }, + }, + }, + } + return d +} + +func TestViewRun_replies(t *testing.T) { + tests := []struct { + name string + tty bool + replies string + limit int + after string + order string + exporter cmdutil.Exporter + nextCursor string + wantContains []string + wantExcludes []string + wantClient func(*testing.T, *client.DiscussionClientMock) + }{ + { + name: "tty renders comment and replies", + tty: true, + replies: "DC_abc", + limit: 30, + order: "newest", + wantContains: []string{ + "octocat", + "This is the parent comment", + "✓ Answer", + "hubot", + "First reply", + "monalisa", + "Second reply", + }, + }, + { + name: "tty shows pagination hint", + tty: true, + replies: "DC_abc", + limit: 30, + order: "newest", + nextCursor: "NEXT_CUR", + wantContains: []string{ + "--after NEXT_CUR", + }, + }, + { + name: "tty no pagination hint when no next cursor", + tty: true, + replies: "DC_abc", + limit: 30, + order: "newest", + wantExcludes: []string{ + "--after", + }, + }, + { + name: "nontty raw output", + tty: false, + replies: "DC_abc", + limit: 30, + order: "oldest", + wantContains: []string{ + "comment:\toctocat\t", + "answer", + "replies:\t2", + "This is the parent comment", + "hubot", + "First reply", + }, + }, + { + name: "nontty shows next cursor", + tty: false, + replies: "DC_abc", + limit: 30, + order: "oldest", + nextCursor: "NEXT_CUR_456", + wantContains: []string{ + "next:\tNEXT_CUR_456", + }, + }, + { + name: "json output", + tty: false, + replies: "DC_abc", + limit: 30, + order: "newest", + exporter: func() cmdutil.Exporter { + e := cmdutil.NewJSONExporter() + e.SetFields(discussionFields) + return e + }(), + wantContains: []string{ + `"totalCount"`, + `"isAnswer":true`, + `"octocat"`, + }, + }, + { + name: "routes to GetCommentReplies only", + tty: false, + replies: "DC_abc", + limit: 10, + after: "CUR_A", + order: "oldest", + wantClient: func(t *testing.T, mock *client.DiscussionClientMock) { + require.Equal(t, 1, len(mock.GetCommentRepliesCalls())) + assert.Equal(t, 0, len(mock.GetByNumberCalls())) + assert.Equal(t, 0, len(mock.GetWithCommentsCalls())) + + call := mock.GetCommentRepliesCalls()[0] + assert.Equal(t, "DC_abc", call.CommentID) + assert.Equal(t, 10, call.Limit) + assert.Equal(t, "CUR_A", call.After) + assert.Equal(t, false, call.Newest) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(tt.tty) + ios.SetStderrTTY(tt.tty) + + d := testDiscussionWithReplies(tt.nextCursor) + mock := &client.DiscussionClientMock{ + GetCommentRepliesFunc: func(repo ghrepo.Interface, number int, commentID string, limit int, after string, newest bool) (*client.Discussion, error) { + return d, nil + }, + } + + opts := &ViewOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Client: func() (client.DiscussionClient, error) { + return mock, nil + }, + DiscussionNumber: 123, + Replies: tt.replies, + Limit: tt.limit, + After: tt.after, + Order: tt.order, + Exporter: tt.exporter, + Now: func() time.Time { return time.Date(2025, 3, 4, 0, 0, 0, 0, time.UTC) }, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + for _, s := range tt.wantContains { + assert.Contains(t, out, s) + } + for _, s := range tt.wantExcludes { + assert.NotContains(t, out, s) + } + if tt.wantClient != nil { + tt.wantClient(t, mock) + } + }) + } +} From 45bd958f750409a6ef3bd07179a3be8de3858520 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 28 Apr 2026 09:30:10 +0100 Subject: [PATCH 2/8] test(discussion/client): improve GetByNumber test coverage Inline mock JSON responses, use non-default values for all fields to verify mapping, add repo-not-found test case, and match real API behaviour for discussions-disabled response (null discussion with NOT_FOUND error). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl_test.go | 174 +++++++++++------- 1 file changed, 105 insertions(+), 69 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl_test.go b/pkg/cmd/discussion/client/client_impl_test.go index 462d84915..aad03047f 100644 --- a/pkg/cmd/discussion/client/client_impl_test.go +++ b/pkg/cmd/discussion/client/client_impl_test.go @@ -1007,93 +1007,130 @@ func TestListCategories(t *testing.T) { } } -// --------------------------------------------------------------------------- -// GetByNumber -// --------------------------------------------------------------------------- - -// getByNumberResp builds a mock DiscussionMinimal JSON response. -func getByNumberResp(hasDiscussions bool, commentTotal int, node string) string { - return heredoc.Docf(` - { - "data": { - "repository": { - "hasDiscussionsEnabled": %t, - "discussion": %s - } - } - } - `, hasDiscussions, wrapCommentTotal(node, commentTotal)) -} - -// wrapCommentTotal merges a `comments` block into a discussion JSON node. -func wrapCommentTotal(node string, total int) string { - // Insert "comments":{"totalCount": N} right before the closing brace. - trimmed := strings.TrimRight(node, " \t\n") - trimmed = trimmed[:len(trimmed)-1] // strip trailing } - return fmt.Sprintf(`%s, "comments": {"totalCount": %d}}`, trimmed, total) -} - func TestGetByNumber(t *testing.T) { repo := ghrepo.New("OWNER", "REPO") tests := []struct { - name string - httpStubs func(*testing.T, *httpmock.Registry) - wantErr string - wantDisc func(*testing.T, *Discussion) + name string + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + assertDisc *Discussion }{ { name: "maps all fields", httpStubs: func(t *testing.T, reg *httpmock.Registry) { - node := heredoc.Doc(` - { - "id": "D_1", - "number": 42, - "title": "Test Discussion", - "body": "This is a test", - "url": "https://github.com/OWNER/REPO/discussions/42", - "closed": false, - "stateReason": "", - "isAnswered": false, - "answerChosenAt": "0001-01-01T00:00:00Z", - "author": {"__typename": "User", "login": "alice"}, - "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, - "answerChosenBy": null, - "labels": {"nodes": []}, - "reactionGroups": [{"content": "THUMBS_UP", "users": {"totalCount": 3}}], - "createdAt": "2025-01-01T00:00:00Z", - "updatedAt": "2025-01-02T00:00:00Z", - "closedAt": "0001-01-01T00:00:00Z", - "locked": false - } - `) reg.Register( httpmock.GraphQL(`query DiscussionMinimal\b`), - httpmock.StringResponse(getByNumberResp(true, 5, node)), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 42, + "title": "Test Discussion", + "body": "This is a test", + "url": "https://github.com/OWNER/REPO/discussions/42", + "closed": true, + "stateReason": "RESOLVED", + "isAnswered": true, + "answerChosenAt": "2025-06-01T12:00:00Z", + "author": {"__typename": "User", "login": "alice", "id": "U1", "name": "Alice"}, + "category": {"id": "C1", "name": "Q&A", "slug": "q-a", "emoji": ":question:", "isAnswerable": true}, + "answerChosenBy": {"__typename": "User", "login": "bob", "id": "U2", "name": "Bob"}, + "labels": {"nodes": [{"id": "L1", "name": "bug", "color": "d73a4a"}]}, + "reactionGroups": [{"content": "THUMBS_UP", "users": {"totalCount": 3}}], + "createdAt": "2025-01-01T00:00:00Z", + "updatedAt": "2025-01-02T00:00:00Z", + "closedAt": "2025-06-01T00:00:00Z", + "locked": true, + "comments": {"totalCount": 5} + } + } + } + } + `)), ) }, - wantDisc: func(t *testing.T, d *Discussion) { - assert.Equal(t, "D_1", d.ID) - assert.Equal(t, 42, d.Number) - assert.Equal(t, "Test Discussion", d.Title) - assert.Equal(t, "This is a test", d.Body) - assert.Equal(t, "alice", d.Author.Login) - assert.Equal(t, 5, d.Comments.TotalCount) - require.Len(t, d.ReactionGroups, 1) - assert.Equal(t, "THUMBS_UP", d.ReactionGroups[0].Content) - assert.Equal(t, 3, d.ReactionGroups[0].TotalCount) + assertDisc: &Discussion{ + ID: "D_1", + Number: 42, + Title: "Test Discussion", + Body: "This is a test", + URL: "https://github.com/OWNER/REPO/discussions/42", + Closed: true, + StateReason: "RESOLVED", + Author: DiscussionActor{ID: "U1", Login: "alice", Name: "Alice"}, + Category: DiscussionCategory{ + ID: "C1", + Name: "Q&A", + Slug: "q-a", + Emoji: ":question:", + IsAnswerable: true, + }, + Labels: []DiscussionLabel{{ID: "L1", Name: "bug", Color: "d73a4a"}}, + Answered: true, + AnswerChosenAt: time.Date(2025, 6, 1, 12, 0, 0, 0, time.UTC), + AnswerChosenBy: &DiscussionActor{ID: "U2", Login: "bob", Name: "Bob"}, + ReactionGroups: []ReactionGroup{ + {Content: "THUMBS_UP", TotalCount: 3}, + }, + Comments: DiscussionCommentList{TotalCount: 5}, + CreatedAt: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC), + ClosedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), + Locked: true, }, }, { name: "discussions disabled", httpStubs: func(t *testing.T, reg *httpmock.Registry) { - node := minimalNode("D_1", "Test") reg.Register( httpmock.GraphQL(`query DiscussionMinimal\b`), - httpmock.StringResponse(getByNumberResp(false, 0, node)), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": false, + "discussion": null + } + }, + "errors": [ + { + "type": "NOT_FOUND", + "path": ["repository", "discussion"], + "message": "Could not resolve to a Discussion with the number of 42." + } + ] + } + `)), ) }, - wantErr: "discussions disabled", + wantErr: "Could not resolve to a Discussion with the number of 42.", + }, + { + name: "repo not found", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query DiscussionMinimal\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": null + }, + "errors": [ + { + "type": "NOT_FOUND", + "path": ["repository"], + "message": "Could not resolve to a Repository with the name 'OWNER/REPO'." + } + ] + } + `)), + ) + }, + wantErr: "Could not resolve to a Repository with the name 'OWNER/REPO'.", }, } @@ -1117,9 +1154,8 @@ func TestGetByNumber(t *testing.T) { require.NoError(t, err) require.NotNil(t, d) - if tt.wantDisc != nil { - tt.wantDisc(t, d) - } + require.NotNil(t, tt.assertDisc, "assertDisc must be set for non-error cases") + assert.Equal(t, tt.assertDisc, d) }) } } From 573f3f0a635f41e311b4b759c6db70450d013a1e Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 28 Apr 2026 09:31:36 +0100 Subject: [PATCH 3/8] test(discussion/test): use nil next page to match API behaviour Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/client/client_impl_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/discussion/client/client_impl_test.go b/pkg/cmd/discussion/client/client_impl_test.go index aad03047f..0910810c3 100644 --- a/pkg/cmd/discussion/client/client_impl_test.go +++ b/pkg/cmd/discussion/client/client_impl_test.go @@ -166,7 +166,7 @@ func TestList(t *testing.T) { "totalCount": 0, "pageInfo": { "hasNextPage": false, - "endCursor": "" + "endCursor": null }, "nodes": [] } From 19369d0444e064e1b4d47ba58880590d4586bf4d Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 28 Apr 2026 10:10:25 +0100 Subject: [PATCH 4/8] test(discussion/client): improve GetWithComments test coverage Inline all JSON response helpers (getWithCommentsResp, wrapCommentsBlock, commentNode) to avoid cross-test coupling. Add missing test cases for empty comments, first page newest reversal, multiple replies on a single comment, and repo not found. Populate the "maps comments with replies" case with non-zero field values and use a single assert.Equal for the full Discussion struct. Match real API error responses for discussions disabled and repo not found cases. Rename wantDisc to assertDisc across all client test functions and require it for non-error cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl_test.go | 677 +++++++++++++++--- 1 file changed, 573 insertions(+), 104 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl_test.go b/pkg/cmd/discussion/client/client_impl_test.go index 0910810c3..d04632d96 100644 --- a/pkg/cmd/discussion/client/client_impl_test.go +++ b/pkg/cmd/discussion/client/client_impl_test.go @@ -1164,44 +1164,6 @@ func TestGetByNumber(t *testing.T) { // GetWithComments // --------------------------------------------------------------------------- -// getWithCommentsResp builds a mock DiscussionWithComments JSON response. -func getWithCommentsResp(hasDiscussions bool, node string, commentNodes string, commentTotal int, hasNext, hasPrev bool, endCursor, startCursor string) string { - return heredoc.Docf(` - { - "data": { - "repository": { - "hasDiscussionsEnabled": %t, - "discussion": %s - } - } - } - `, hasDiscussions, wrapCommentsBlock(node, commentNodes, commentTotal, hasNext, hasPrev, endCursor, startCursor)) -} - -func wrapCommentsBlock(node string, commentNodes string, total int, hasNext, hasPrev bool, endCursor, startCursor string) string { - trimmed := strings.TrimRight(node, " \t\n") - trimmed = trimmed[:len(trimmed)-1] - return fmt.Sprintf(`%s, "comments": {"totalCount": %d, "pageInfo": {"endCursor": %q, "hasNextPage": %t, "startCursor": %q, "hasPreviousPage": %t}, "nodes": [%s]}}`, - trimmed, total, endCursor, hasNext, startCursor, hasPrev, commentNodes) -} - -// commentNode builds a JSON comment node with nested replies. -func commentNode(id, login, body string, isAnswer bool, replyNodes string, replyTotal int) string { - return heredoc.Docf(` - { - "id": %q, - "url": "https://github.com/OWNER/REPO/discussions/1#comment-%s", - "author": {"__typename": "User", "login": %q}, - "body": %q, - "createdAt": "2025-01-01T00:00:00Z", - "isAnswer": %t, - "upvoteCount": 0, - "reactionGroups": [], - "replies": {"totalCount": %d, "nodes": [%s]} - } - `, id, id, login, body, isAnswer, replyTotal, replyNodes) -} - // replyNode builds a JSON reply node. func replyNode(id, login, body string) string { return heredoc.Docf(` @@ -1222,43 +1184,142 @@ func TestGetWithComments(t *testing.T) { repo := ghrepo.New("OWNER", "REPO") tests := []struct { - name string - limit int - after string - newest bool - httpStubs func(*testing.T, *httpmock.Registry) - wantErr string - wantComments int - wantTotal int - wantCursor string - wantNext string - wantDirection DiscussionCommentListDirection - wantDisc func(*testing.T, *Discussion) + name string + limit int + after string + newest bool + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + assertDisc func(*testing.T, *Discussion) }{ { name: "maps comments with replies", limit: 10, newest: false, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - reply := replyNode("R1", "hubot", "Thanks!") - comment := commentNode("C1", "octocat", "Main comment", true, reply, 1) - node := minimalNode("D_1", "Test Discussion") reg.Register( httpmock.GraphQL(`query DiscussionWithComments\b`), - httpmock.StringResponse(getWithCommentsResp(true, node, comment, 1, false, false, "", "")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 42, + "title": "Test Discussion", + "body": "Discussion body", + "url": "https://github.com/OWNER/REPO/discussions/42", + "closed": true, + "stateReason": "RESOLVED", + "isAnswered": true, + "answerChosenAt": "2025-06-01T12:00:00Z", + "author": {"__typename": "User", "login": "alice", "id": "U_alice", "name": "Alice"}, + "category": {"id": "CAT1", "name": "Q&A", "slug": "q-a", "emoji": ":question:", "isAnswerable": true}, + "answerChosenBy": {"__typename": "User", "login": "bob", "id": "U_bob", "name": "Bob"}, + "labels": {"nodes": [{"id": "L1", "name": "bug", "color": "d73a4a"}]}, + "reactionGroups": [{"content": "THUMBS_UP", "users": {"totalCount": 3}}], + "createdAt": "2025-01-01T00:00:00Z", + "updatedAt": "2025-01-02T00:00:00Z", + "closedAt": "2025-06-01T00:00:00Z", + "locked": true, + "comments": { + "totalCount": 1, + "pageInfo": {"endCursor": "COM_CUR", "hasNextPage": true, "startCursor": "COM_START", "hasPreviousPage": false}, + "nodes": [ + { + "id": "C1", + "url": "https://github.com/OWNER/REPO/discussions/42#comment-1", + "author": {"__typename": "User", "login": "octocat", "id": "U_octocat", "name": "Octocat"}, + "body": "Main comment", + "createdAt": "2025-03-01T00:00:00Z", + "isAnswer": true, + "upvoteCount": 5, + "reactionGroups": [{"content": "HEART", "users": {"totalCount": 2}}], + "replies": { + "totalCount": 1, + "nodes": [ + { + "id": "R1", + "url": "https://github.com/OWNER/REPO/discussions/42#reply-1", + "author": {"__typename": "User", "login": "hubot", "id": "U_hubot", "name": "Hubot"}, + "body": "Thanks!", + "createdAt": "2025-04-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 1, + "reactionGroups": [{"content": "THUMBS_UP", "users": {"totalCount": 1}}] + } + ] + } + } + ] + } + } + } + } + } + `)), ) }, - wantComments: 1, - wantTotal: 1, - wantDirection: DiscussionCommentListDirectionForward, - wantDisc: func(t *testing.T, d *Discussion) { - c := d.Comments.Comments[0] - assert.Equal(t, "C1", c.ID) - assert.Equal(t, "octocat", c.Author.Login) - assert.True(t, c.IsAnswer) - require.Len(t, c.Replies.Comments, 1) - assert.Equal(t, "R1", c.Replies.Comments[0].ID) - assert.Equal(t, "hubot", c.Replies.Comments[0].Author.Login) + assertDisc: func(t *testing.T, d *Discussion) { + assert.Equal(t, Discussion{ + ID: "D_1", + Number: 42, + Title: "Test Discussion", + Body: "Discussion body", + URL: "https://github.com/OWNER/REPO/discussions/42", + Closed: true, + StateReason: "RESOLVED", + Author: DiscussionActor{ID: "U_alice", Login: "alice", Name: "Alice"}, + Category: DiscussionCategory{ + ID: "CAT1", + Name: "Q&A", + Slug: "q-a", + Emoji: ":question:", + IsAnswerable: true, + }, + Labels: []DiscussionLabel{{ID: "L1", Name: "bug", Color: "d73a4a"}}, + Answered: true, + AnswerChosenAt: time.Date(2025, 6, 1, 12, 0, 0, 0, time.UTC), + AnswerChosenBy: &DiscussionActor{ID: "U_bob", Login: "bob", Name: "Bob"}, + ReactionGroups: []ReactionGroup{{Content: "THUMBS_UP", TotalCount: 3}}, + CreatedAt: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC), + ClosedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), + Locked: true, + Comments: DiscussionCommentList{ + TotalCount: 1, + NextCursor: "COM_CUR", + Direction: DiscussionCommentListDirectionForward, + Comments: []DiscussionComment{ + { + ID: "C1", + URL: "https://github.com/OWNER/REPO/discussions/42#comment-1", + Author: DiscussionActor{ID: "U_octocat", Login: "octocat", Name: "Octocat"}, + Body: "Main comment", + CreatedAt: time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC), + IsAnswer: true, + UpvoteCount: 5, + ReactionGroups: []ReactionGroup{{Content: "HEART", TotalCount: 2}}, + Replies: DiscussionCommentList{ + TotalCount: 1, + Direction: DiscussionCommentListDirectionBackward, + Comments: []DiscussionComment{ + { + ID: "R1", + URL: "https://github.com/OWNER/REPO/discussions/42#reply-1", + Author: DiscussionActor{ID: "U_hubot", Login: "hubot", Name: "Hubot"}, + Body: "Thanks!", + CreatedAt: time.Date(2025, 4, 1, 0, 0, 0, 0, time.UTC), + UpvoteCount: 1, + ReactionGroups: []ReactionGroup{{Content: "THUMBS_UP", TotalCount: 1}}, + }, + }, + }, + }, + }, + }, + }, *d) }, }, { @@ -1267,18 +1328,64 @@ func TestGetWithComments(t *testing.T) { after: "CUR_A", newest: false, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - comment := commentNode("C1", "alice", "Hello", false, "", 0) - node := minimalNode("D_1", "Test") reg.Register( httpmock.GraphQL(`query DiscussionWithComments\b`), - httpmock.StringResponse(getWithCommentsResp(true, node, comment, 3, true, false, "CUR_B", "")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": { + "totalCount": 3, + "pageInfo": {"endCursor": "CUR_B", "hasNextPage": true, "startCursor": "", "hasPreviousPage": false}, + "nodes": [ + { + "id": "C1", + "url": "", + "author": {"__typename": "User", "login": "alice"}, + "body": "Hello", + "createdAt": "2025-01-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": {"totalCount": 0, "nodes": []} + } + ] + } + } + } + } + } + `)), ) }, - wantComments: 1, - wantTotal: 3, - wantCursor: "CUR_A", - wantNext: "CUR_B", - wantDirection: DiscussionCommentListDirectionForward, + assertDisc: func(t *testing.T, d *Discussion) { + comments := d.Comments + assert.Len(t, comments.Comments, 1) + assert.Equal(t, 3, comments.TotalCount) + assert.Equal(t, "CUR_A", comments.Cursor) + assert.Equal(t, "CUR_B", comments.NextCursor) + assert.Equal(t, DiscussionCommentListDirectionForward, comments.Direction) + }, }, { name: "pagination backward newest", @@ -1286,23 +1393,76 @@ func TestGetWithComments(t *testing.T) { after: "CUR_X", newest: true, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - c1 := commentNode("C1", "alice", "First", false, "", 0) - c2 := commentNode("C2", "bob", "Second", false, "", 0) - node := minimalNode("D_1", "Test") reg.Register( httpmock.GraphQL(`query DiscussionWithComments\b`), - httpmock.StringResponse(getWithCommentsResp(true, node, c1+","+c2, 5, false, true, "", "CUR_Y")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": { + "totalCount": 5, + "pageInfo": {"endCursor": "", "hasNextPage": false, "startCursor": "CUR_Y", "hasPreviousPage": true}, + "nodes": [ + { + "id": "C1", + "url": "", + "author": {"__typename": "User", "login": "alice"}, + "body": "First", + "createdAt": "2025-01-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": {"totalCount": 0, "nodes": []} + }, + { + "id": "C2", + "url": "", + "author": {"__typename": "User", "login": "bob"}, + "body": "Second", + "createdAt": "2025-01-02T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": {"totalCount": 0, "nodes": []} + } + ] + } + } + } + } + } + `)), ) }, - wantComments: 2, - wantTotal: 5, - wantCursor: "CUR_X", - wantNext: "CUR_Y", - wantDirection: DiscussionCommentListDirectionBackward, - wantDisc: func(t *testing.T, d *Discussion) { - // Newest mode reverses the order - assert.Equal(t, "C2", d.Comments.Comments[0].ID) - assert.Equal(t, "C1", d.Comments.Comments[1].ID) + assertDisc: func(t *testing.T, d *Discussion) { + comments := d.Comments + assert.Len(t, comments.Comments, 2) + assert.Equal(t, 5, comments.TotalCount) + assert.Equal(t, "CUR_X", comments.Cursor) + assert.Equal(t, "CUR_Y", comments.NextCursor) + assert.Equal(t, DiscussionCommentListDirectionBackward, comments.Direction) + assert.Equal(t, "C2", comments.Comments[0].ID, "newest mode should reverse comments") + assert.Equal(t, "C1", comments.Comments[1].ID) }, }, { @@ -1310,30 +1470,345 @@ func TestGetWithComments(t *testing.T) { limit: 10, newest: false, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - comment := commentNode("C1", "alice", "Only one", false, "", 0) - node := minimalNode("D_1", "Test") reg.Register( httpmock.GraphQL(`query DiscussionWithComments\b`), - httpmock.StringResponse(getWithCommentsResp(true, node, comment, 1, false, false, "", "")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": { + "totalCount": 1, + "pageInfo": {"endCursor": "", "hasNextPage": false, "startCursor": "", "hasPreviousPage": false}, + "nodes": [ + { + "id": "C1", + "url": "", + "author": {"__typename": "User", "login": "alice"}, + "body": "Only one", + "createdAt": "2025-01-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": {"totalCount": 0, "nodes": []} + } + ] + } + } + } + } + } + `)), ) }, - wantComments: 1, - wantTotal: 1, - wantNext: "", - wantDirection: DiscussionCommentListDirectionForward, + assertDisc: func(t *testing.T, d *Discussion) { + comments := d.Comments + assert.Len(t, comments.Comments, 1) + assert.Equal(t, 1, comments.TotalCount) + assert.Equal(t, "", comments.NextCursor) + assert.Equal(t, DiscussionCommentListDirectionForward, comments.Direction) + }, }, { name: "discussions disabled", limit: 10, newest: false, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - node := minimalNode("D_1", "Test") reg.Register( httpmock.GraphQL(`query DiscussionWithComments\b`), - httpmock.StringResponse(getWithCommentsResp(false, node, "", 0, false, false, "", "")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": false, + "discussion": null + } + }, + "errors": [ + { + "type": "NOT_FOUND", + "path": ["repository", "discussion"], + "message": "Could not resolve to a Discussion with the number of 1." + } + ] + } + `)), ) }, - wantErr: "discussions disabled", + wantErr: "Could not resolve to a Discussion", + }, + { + name: "repo not found", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query DiscussionWithComments\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": null + }, + "errors": [ + { + "type": "NOT_FOUND", + "path": ["repository"], + "message": "Could not resolve to a Repository with the name 'OWNER/REPO'." + } + ] + } + `)), + ) + }, + wantErr: "Could not resolve to a Repository with the name 'OWNER/REPO'.", + }, + { + name: "empty comments", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query DiscussionWithComments\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": { + "totalCount": 0, + "pageInfo": {"endCursor": null, "hasNextPage": false, "startCursor": null, "hasPreviousPage": false}, + "nodes": [] + } + } + } + } + } + `)), + ) + }, + assertDisc: func(t *testing.T, d *Discussion) { + comments := d.Comments + assert.Len(t, comments.Comments, 0) + assert.Equal(t, 0, comments.TotalCount) + assert.Equal(t, DiscussionCommentListDirectionForward, comments.Direction) + }, + }, + { + name: "first page newest reverses comments", + limit: 5, + newest: true, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query DiscussionWithComments\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": { + "totalCount": 8, + "pageInfo": {"endCursor": "", "hasNextPage": false, "startCursor": "CUR_START", "hasPreviousPage": true}, + "nodes": [ + { + "id": "C4", + "url": "", + "author": {"__typename": "User", "login": "alice"}, + "body": "Fourth", + "createdAt": "2025-01-04T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": {"totalCount": 0, "nodes": []} + }, + { + "id": "C5", + "url": "", + "author": {"__typename": "User", "login": "bob"}, + "body": "Fifth", + "createdAt": "2025-01-05T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": {"totalCount": 0, "nodes": []} + } + ] + } + } + } + } + } + `)), + ) + }, + assertDisc: func(t *testing.T, d *Discussion) { + comments := d.Comments + assert.Len(t, comments.Comments, 2) + assert.Equal(t, 8, comments.TotalCount) + assert.Equal(t, "", comments.Cursor) + assert.Equal(t, "CUR_START", comments.NextCursor) + assert.Equal(t, DiscussionCommentListDirectionBackward, comments.Direction) + assert.Equal(t, "C5", comments.Comments[0].ID, "newest mode should reverse comments") + assert.Equal(t, "C4", comments.Comments[1].ID) + }, + }, + { + name: "multiple replies on comment", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query DiscussionWithComments\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": { + "totalCount": 1, + "pageInfo": {"endCursor": "", "hasNextPage": false, "startCursor": "", "hasPreviousPage": false}, + "nodes": [ + { + "id": "C1", + "url": "", + "author": {"__typename": "User", "login": "alice"}, + "body": "Parent", + "createdAt": "2025-01-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": { + "totalCount": 3, + "nodes": [ + { + "id": "R1", + "url": "", + "author": {"__typename": "User", "login": "bob"}, + "body": "First reply", + "createdAt": "2025-01-02T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [] + }, + { + "id": "R2", + "url": "", + "author": {"__typename": "User", "login": "carol"}, + "body": "Second reply", + "createdAt": "2025-01-03T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [] + }, + { + "id": "R3", + "url": "", + "author": {"__typename": "User", "login": "dave"}, + "body": "Third reply", + "createdAt": "2025-01-04T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [] + } + ] + } + } + ] + } + } + } + } + } + `)), + ) + }, + assertDisc: func(t *testing.T, d *Discussion) { + comments := d.Comments + assert.Len(t, comments.Comments, 1) + assert.Equal(t, 1, comments.TotalCount) + assert.Equal(t, DiscussionCommentListDirectionForward, comments.Direction) + c := comments.Comments[0] + require.Len(t, c.Replies.Comments, 3) + assert.Equal(t, 3, c.Replies.TotalCount) + assert.Equal(t, "R1", c.Replies.Comments[0].ID) + assert.Equal(t, "R2", c.Replies.Comments[1].ID) + assert.Equal(t, "R3", c.Replies.Comments[2].ID) + }, }, } @@ -1357,14 +1832,8 @@ func TestGetWithComments(t *testing.T) { require.NoError(t, err) require.NotNil(t, d) - assert.Len(t, d.Comments.Comments, tt.wantComments) - assert.Equal(t, tt.wantTotal, d.Comments.TotalCount) - assert.Equal(t, tt.wantCursor, d.Comments.Cursor) - assert.Equal(t, tt.wantNext, d.Comments.NextCursor) - assert.Equal(t, tt.wantDirection, d.Comments.Direction) - if tt.wantDisc != nil { - tt.wantDisc(t, d) - } + require.NotNil(t, tt.assertDisc, "assertDisc must be set for non-error cases") + tt.assertDisc(t, d) }) } } From e263abfb2c4ae1038fb04821094a2e0d74d9f043 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 28 Apr 2026 10:37:02 +0100 Subject: [PATCH 5/8] test(discussion/client): improve GetCommentReplies test coverage Inline all JSON response helpers (getCommentRepliesResp, wrapRepliesBlock, bareCommentNode, replyNode) to avoid cross-test coupling. Fix JSON response shape to place "node" as sibling of "repository" under "data", matching the real GraphQL query structure. Populate "maps all fields" with non-zero values and use a single assert.Equal for the full Discussion struct. Match real API error responses: discussions disabled returns NOT_FOUND on discussion, node not found returns NOT_FOUND with null node, wrong-type node returns empty object. Add missing test cases for repo not found and first page newest reversal. Move shared wantComments/wantTotal/wantCursor/wantNext/wantDirection fields into assertDisc callbacks for both TestGetWithComments and TestGetCommentReplies, giving each case full ownership of its assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl_test.go | 697 ++++++++++++++---- 1 file changed, 549 insertions(+), 148 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl_test.go b/pkg/cmd/discussion/client/client_impl_test.go index d04632d96..824de4219 100644 --- a/pkg/cmd/discussion/client/client_impl_test.go +++ b/pkg/cmd/discussion/client/client_impl_test.go @@ -1164,22 +1164,6 @@ func TestGetByNumber(t *testing.T) { // GetWithComments // --------------------------------------------------------------------------- -// replyNode builds a JSON reply node. -func replyNode(id, login, body string) string { - return heredoc.Docf(` - { - "id": %q, - "url": "https://github.com/OWNER/REPO/discussions/1#reply-%s", - "author": {"__typename": "User", "login": %q}, - "body": %q, - "createdAt": "2025-02-01T00:00:00Z", - "isAnswer": false, - "upvoteCount": 0, - "reactionGroups": [] - } - `, id, id, login, body) -} - func TestGetWithComments(t *testing.T) { repo := ghrepo.New("OWNER", "REPO") @@ -1842,68 +1826,18 @@ func TestGetWithComments(t *testing.T) { // GetCommentReplies // --------------------------------------------------------------------------- -// getCommentRepliesResp builds a mock DiscussionCommentReplies JSON response. -// The shurcooL graphql library treats inline fragments as transparent — the -// comment fields are placed directly inside the "node" object, not nested -// under a "DiscussionComment" key. -func getCommentRepliesResp(hasDiscussions bool, discNode string, commentNode *string, replyNodes string, replyTotal int, hasNext, hasPrev bool, endCursor, startCursor string) string { - nodeBlock := "null" - if commentNode != nil { - nodeBlock = wrapRepliesBlock(*commentNode, replyNodes, replyTotal, hasNext, hasPrev, endCursor, startCursor) - } - return heredoc.Docf(` - { - "data": { - "repository": { - "hasDiscussionsEnabled": %t, - "discussion": %s - }, - "node": %s - } - } - `, hasDiscussions, discNode, nodeBlock) -} - -func wrapRepliesBlock(commentJSON string, replyNodes string, total int, hasNext, hasPrev bool, endCursor, startCursor string) string { - trimmed := strings.TrimRight(commentJSON, " \t\n") - trimmed = trimmed[:len(trimmed)-1] - return fmt.Sprintf(`%s, "replies": {"totalCount": %d, "pageInfo": {"endCursor": %q, "hasNextPage": %t, "startCursor": %q, "hasPreviousPage": %t}, "nodes": [%s]}}`, - trimmed, total, endCursor, hasNext, startCursor, hasPrev, replyNodes) -} - -// bareCommentNode builds a comment JSON node without replies (used for GetCommentReplies). -func bareCommentNode(id, login, body string, isAnswer bool) string { - return heredoc.Docf(` - { - "id": %q, - "url": "https://github.com/OWNER/REPO/discussions/1#comment-%s", - "author": {"__typename": "User", "login": %q}, - "body": %q, - "createdAt": "2025-01-01T00:00:00Z", - "isAnswer": %t, - "upvoteCount": 2, - "reactionGroups": [{"content": "HEART", "users": {"totalCount": 1}}] - } - `, id, id, login, body, isAnswer) -} - func TestGetCommentReplies(t *testing.T) { repo := ghrepo.New("OWNER", "REPO") tests := []struct { - name string - commentID string - limit int - after string - newest bool - httpStubs func(*testing.T, *httpmock.Registry) - wantErr string - wantReplies int - wantTotal int - wantCursor string - wantNext string - wantDirection DiscussionCommentListDirection - wantDisc func(*testing.T, *Discussion) + name string + commentID string + limit int + after string + newest bool + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + assertDisc func(*testing.T, *Discussion) }{ { name: "maps all fields", @@ -1911,31 +1845,123 @@ func TestGetCommentReplies(t *testing.T) { limit: 10, newest: false, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - discNode := minimalNode("D_1", "Test Discussion") - comment := bareCommentNode("DC_abc", "octocat", "Top-level comment", true) - reply := replyNode("R1", "hubot", "A reply") reg.Register( httpmock.GraphQL(`query DiscussionCommentReplies\b`), - httpmock.StringResponse(getCommentRepliesResp(true, discNode, &comment, reply, 1, false, false, "", "")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 42, + "title": "Test Discussion", + "body": "Discussion body", + "url": "https://github.com/OWNER/REPO/discussions/42", + "closed": true, + "stateReason": "RESOLVED", + "isAnswered": true, + "answerChosenAt": "2025-06-01T12:00:00Z", + "author": {"__typename": "User", "login": "alice", "id": "U_alice", "name": "Alice"}, + "category": {"id": "CAT1", "name": "Q&A", "slug": "q-a", "emoji": ":question:", "isAnswerable": true}, + "answerChosenBy": {"__typename": "User", "login": "bob", "id": "U_bob", "name": "Bob"}, + "labels": {"nodes": [{"id": "L1", "name": "bug", "color": "d73a4a"}]}, + "reactionGroups": [{"content": "THUMBS_UP", "users": {"totalCount": 3}}], + "createdAt": "2025-01-01T00:00:00Z", + "updatedAt": "2025-01-02T00:00:00Z", + "closedAt": "2025-06-01T00:00:00Z", + "locked": true + } + }, + "node": { + "id": "DC_abc", + "url": "https://github.com/OWNER/REPO/discussions/42#discussioncomment-1", + "author": {"__typename": "User", "login": "octocat", "id": "U_octocat", "name": "Octocat"}, + "body": "Top-level comment", + "createdAt": "2025-03-01T00:00:00Z", + "isAnswer": true, + "upvoteCount": 5, + "reactionGroups": [{"content": "HEART", "users": {"totalCount": 2}}], + "replies": { + "totalCount": 1, + "pageInfo": {"endCursor": "REP_CUR", "hasNextPage": true, "startCursor": "REP_START", "hasPreviousPage": false}, + "nodes": [ + { + "id": "R1", + "url": "https://github.com/OWNER/REPO/discussions/42#discussioncomment-2", + "author": {"__typename": "User", "login": "hubot", "id": "U_hubot", "name": "Hubot"}, + "body": "A reply", + "createdAt": "2025-04-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 1, + "reactionGroups": [{"content": "THUMBS_UP", "users": {"totalCount": 1}}] + } + ] + } + } + } + } + `)), ) }, - wantReplies: 1, - wantTotal: 1, - wantDirection: DiscussionCommentListDirectionForward, - wantDisc: func(t *testing.T, d *Discussion) { - assert.Equal(t, "Test Discussion", d.Title) - require.Len(t, d.Comments.Comments, 1) - c := d.Comments.Comments[0] - assert.Equal(t, "DC_abc", c.ID) - assert.Equal(t, "octocat", c.Author.Login) - assert.Equal(t, "Top-level comment", c.Body) - assert.True(t, c.IsAnswer) - assert.Equal(t, 2, c.UpvoteCount) - require.Len(t, c.ReactionGroups, 1) - assert.Equal(t, "HEART", c.ReactionGroups[0].Content) - require.Len(t, c.Replies.Comments, 1) - assert.Equal(t, "R1", c.Replies.Comments[0].ID) - assert.Equal(t, "hubot", c.Replies.Comments[0].Author.Login) + assertDisc: func(t *testing.T, d *Discussion) { + assert.Equal(t, Discussion{ + ID: "D_1", + Number: 42, + Title: "Test Discussion", + Body: "Discussion body", + URL: "https://github.com/OWNER/REPO/discussions/42", + Closed: true, + StateReason: "RESOLVED", + Author: DiscussionActor{ID: "U_alice", Login: "alice", Name: "Alice"}, + Category: DiscussionCategory{ + ID: "CAT1", + Name: "Q&A", + Slug: "q-a", + Emoji: ":question:", + IsAnswerable: true, + }, + Labels: []DiscussionLabel{{ID: "L1", Name: "bug", Color: "d73a4a"}}, + Answered: true, + AnswerChosenAt: time.Date(2025, 6, 1, 12, 0, 0, 0, time.UTC), + AnswerChosenBy: &DiscussionActor{ID: "U_bob", Login: "bob", Name: "Bob"}, + ReactionGroups: []ReactionGroup{{Content: "THUMBS_UP", TotalCount: 3}}, + CreatedAt: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC), + ClosedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), + Locked: true, + Comments: DiscussionCommentList{ + TotalCount: 1, + Comments: []DiscussionComment{ + { + ID: "DC_abc", + URL: "https://github.com/OWNER/REPO/discussions/42#discussioncomment-1", + Author: DiscussionActor{ID: "U_octocat", Login: "octocat", Name: "Octocat"}, + Body: "Top-level comment", + CreatedAt: time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC), + IsAnswer: true, + UpvoteCount: 5, + ReactionGroups: []ReactionGroup{{Content: "HEART", TotalCount: 2}}, + Replies: DiscussionCommentList{ + TotalCount: 1, + NextCursor: "REP_CUR", + Direction: DiscussionCommentListDirectionForward, + Comments: []DiscussionComment{ + { + ID: "R1", + URL: "https://github.com/OWNER/REPO/discussions/42#discussioncomment-2", + Author: DiscussionActor{ID: "U_hubot", Login: "hubot", Name: "Hubot"}, + Body: "A reply", + CreatedAt: time.Date(2025, 4, 1, 0, 0, 0, 0, time.UTC), + UpvoteCount: 1, + ReactionGroups: []ReactionGroup{{Content: "THUMBS_UP", TotalCount: 1}}, + }, + }, + }, + }, + }, + }, + }, *d) }, }, { @@ -1945,19 +1971,85 @@ func TestGetCommentReplies(t *testing.T) { after: "CUR_A", newest: false, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - discNode := minimalNode("D_1", "Test") - comment := bareCommentNode("DC_abc", "alice", "Comment", false) - r1 := replyNode("R1", "bob", "Reply 1") reg.Register( httpmock.GraphQL(`query DiscussionCommentReplies\b`), - httpmock.StringResponse(getCommentRepliesResp(true, discNode, &comment, r1, 3, true, false, "CUR_B", "")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false + } + }, + "node": { + "id": "DC_abc", + "url": "", + "author": {"__typename": "User", "login": "alice"}, + "body": "Comment", + "createdAt": "2025-01-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": { + "totalCount": 3, + "pageInfo": {"endCursor": "CUR_B", "hasNextPage": true, "startCursor": "CUR_A", "hasPreviousPage": false}, + "nodes": [ + { + "id": "R1", + "url": "", + "author": {"__typename": "User", "login": "bob"}, + "body": "Reply 1", + "createdAt": "2025-02-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [] + }, + { + "id": "R2", + "url": "", + "author": {"__typename": "User", "login": "carol"}, + "body": "Reply 2", + "createdAt": "2025-03-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [] + } + ] + } + } + } + } + `)), ) }, - wantReplies: 1, - wantTotal: 3, - wantCursor: "CUR_A", - wantNext: "CUR_B", - wantDirection: DiscussionCommentListDirectionForward, + assertDisc: func(t *testing.T, d *Discussion) { + replies := d.Comments.Comments[0].Replies + assert.Len(t, replies.Comments, 2) + assert.Equal(t, 3, replies.TotalCount) + assert.Equal(t, "CUR_A", replies.Cursor) + assert.Equal(t, "CUR_B", replies.NextCursor) + assert.Equal(t, DiscussionCommentListDirectionForward, replies.Direction) + assert.Equal(t, "R1", replies.Comments[0].ID, "forward mode should preserve chronological order") + assert.Equal(t, "R2", replies.Comments[1].ID) + }, }, { name: "pagination backward newest reverses replies", @@ -1966,24 +2058,170 @@ func TestGetCommentReplies(t *testing.T) { after: "CUR_X", newest: true, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - discNode := minimalNode("D_1", "Test") - comment := bareCommentNode("DC_abc", "alice", "Comment", false) - r1 := replyNode("R1", "bob", "Older") - r2 := replyNode("R2", "carol", "Newer") reg.Register( httpmock.GraphQL(`query DiscussionCommentReplies\b`), - httpmock.StringResponse(getCommentRepliesResp(true, discNode, &comment, r1+","+r2, 5, false, true, "", "CUR_Y")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false + } + }, + "node": { + "id": "DC_abc", + "url": "", + "author": {"__typename": "User", "login": "alice"}, + "body": "Comment", + "createdAt": "2025-01-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": { + "totalCount": 5, + "pageInfo": {"endCursor": "CUR_END", "hasNextPage": false, "startCursor": "CUR_Y", "hasPreviousPage": true}, + "nodes": [ + { + "id": "R1", + "url": "", + "author": {"__typename": "User", "login": "bob"}, + "body": "Older", + "createdAt": "2025-02-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [] + }, + { + "id": "R2", + "url": "", + "author": {"__typename": "User", "login": "carol"}, + "body": "Newer", + "createdAt": "2025-03-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [] + } + ] + } + } + } + } + `)), ) }, - wantReplies: 2, - wantTotal: 5, - wantCursor: "CUR_X", - wantNext: "CUR_Y", - wantDirection: DiscussionCommentListDirectionBackward, - wantDisc: func(t *testing.T, d *Discussion) { - replies := d.Comments.Comments[0].Replies.Comments - assert.Equal(t, "R2", replies[0].ID, "newest mode should reverse replies") - assert.Equal(t, "R1", replies[1].ID) + assertDisc: func(t *testing.T, d *Discussion) { + replies := d.Comments.Comments[0].Replies + assert.Len(t, replies.Comments, 2) + assert.Equal(t, 5, replies.TotalCount) + assert.Equal(t, "CUR_X", replies.Cursor) + assert.Equal(t, "CUR_Y", replies.NextCursor) + assert.Equal(t, DiscussionCommentListDirectionBackward, replies.Direction) + assert.Equal(t, "R2", replies.Comments[0].ID, "newest mode should reverse replies") + assert.Equal(t, "R1", replies.Comments[1].ID) + }, + }, + { + name: "first page newest reverses replies", + commentID: "DC_abc", + limit: 5, + newest: true, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query DiscussionCommentReplies\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false + } + }, + "node": { + "id": "DC_abc", + "url": "", + "author": {"__typename": "User", "login": "alice"}, + "body": "Comment", + "createdAt": "2025-01-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": { + "totalCount": 3, + "pageInfo": {"endCursor": "", "hasNextPage": false, "startCursor": "CUR_START", "hasPreviousPage": true}, + "nodes": [ + { + "id": "R1", + "url": "", + "author": {"__typename": "User", "login": "bob"}, + "body": "Older", + "createdAt": "2025-02-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [] + }, + { + "id": "R2", + "url": "", + "author": {"__typename": "User", "login": "carol"}, + "body": "Newer", + "createdAt": "2025-03-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [] + } + ] + } + } + } + } + `)), + ) + }, + assertDisc: func(t *testing.T, d *Discussion) { + replies := d.Comments.Comments[0].Replies + assert.Len(t, replies.Comments, 2) + assert.Equal(t, 3, replies.TotalCount) + assert.Equal(t, "", replies.Cursor) + assert.Equal(t, "CUR_START", replies.NextCursor) + assert.Equal(t, DiscussionCommentListDirectionBackward, replies.Direction) + assert.Equal(t, "R2", replies.Comments[0].ID, "newest mode should reverse replies") + assert.Equal(t, "R1", replies.Comments[1].ID) }, }, { @@ -1992,18 +2230,72 @@ func TestGetCommentReplies(t *testing.T) { limit: 10, newest: false, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - discNode := minimalNode("D_1", "Test") - comment := bareCommentNode("DC_abc", "alice", "Comment", false) - r1 := replyNode("R1", "bob", "Only reply") reg.Register( httpmock.GraphQL(`query DiscussionCommentReplies\b`), - httpmock.StringResponse(getCommentRepliesResp(true, discNode, &comment, r1, 1, false, false, "", "")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false + } + }, + "node": { + "id": "DC_abc", + "url": "", + "author": {"__typename": "User", "login": "alice"}, + "body": "Comment", + "createdAt": "2025-01-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": { + "totalCount": 1, + "pageInfo": {"endCursor": "CUR_ONLY", "hasNextPage": false, "startCursor": "CUR_ONLY", "hasPreviousPage": false}, + "nodes": [ + { + "id": "R1", + "url": "", + "author": {"__typename": "User", "login": "bob"}, + "body": "Only reply", + "createdAt": "2025-02-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [] + } + ] + } + } + } + } + `)), ) }, - wantReplies: 1, - wantTotal: 1, - wantNext: "", - wantDirection: DiscussionCommentListDirectionForward, + assertDisc: func(t *testing.T, d *Discussion) { + replies := d.Comments.Comments[0].Replies + assert.Len(t, replies.Comments, 1) + assert.Equal(t, 1, replies.TotalCount) + assert.Equal(t, "", replies.NextCursor) + assert.Equal(t, DiscussionCommentListDirectionForward, replies.Direction) + }, }, { name: "discussions disabled", @@ -2011,28 +2303,119 @@ func TestGetCommentReplies(t *testing.T) { limit: 10, newest: false, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - discNode := minimalNode("D_1", "Test") - comment := bareCommentNode("DC_abc", "alice", "Comment", false) reg.Register( httpmock.GraphQL(`query DiscussionCommentReplies\b`), - httpmock.StringResponse(getCommentRepliesResp(false, discNode, &comment, "", 0, false, false, "", "")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": false, + "discussion": null + }, + "node": { + "id": "DC_abc", + "url": "", + "author": {"__typename": "User", "login": "alice"}, + "body": "Comment", + "createdAt": "2025-01-01T00:00:00Z", + "isAnswer": false, + "upvoteCount": 0, + "reactionGroups": [], + "replies": { + "totalCount": 0, + "pageInfo": {"endCursor": null, "hasNextPage": false, "startCursor": null, "hasPreviousPage": false}, + "nodes": [] + } + } + }, + "errors": [ + { + "type": "NOT_FOUND", + "path": ["repository", "discussion"], + "message": "Could not resolve to a Discussion with the number of 1." + } + ] + } + `)), ) }, - wantErr: "discussions disabled", + wantErr: "Could not resolve to a Discussion", }, { - name: "node not found nil", + name: "repo not found", + commentID: "DC_abc", + limit: 10, + newest: false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query DiscussionCommentReplies\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": null, + "node": null + }, + "errors": [ + { + "type": "NOT_FOUND", + "path": ["repository"], + "message": "Could not resolve to a Repository with the name 'OWNER/REPO'." + } + ] + } + `)), + ) + }, + wantErr: "Could not resolve to a Repository", + }, + { + name: "reply node not found", commentID: "DC_invalid", limit: 10, newest: false, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - discNode := minimalNode("D_1", "Test") reg.Register( httpmock.GraphQL(`query DiscussionCommentReplies\b`), - httpmock.StringResponse(getCommentRepliesResp(true, discNode, nil, "", 0, false, false, "", "")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false + } + }, + "node": null + }, + "errors": [ + { + "type": "NOT_FOUND", + "path": ["node"], + "message": "Could not resolve to a node with the global id of 'DC_invalid'" + } + ] + } + `)), ) }, - wantErr: "comment DC_invalid not found", + wantErr: "Could not resolve to a node", }, { name: "node is not a discussion comment", @@ -2040,12 +2423,38 @@ func TestGetCommentReplies(t *testing.T) { limit: 10, newest: false, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - discNode := minimalNode("D_1", "Test") - // Return a node with an empty DiscussionComment (wrong type) - emptyComment := `{"id":"","url":"","author":{"__typename":"User","login":""},"body":"","createdAt":"0001-01-01T00:00:00Z","isAnswer":false,"upvoteCount":0,"reactionGroups":[]}` reg.Register( httpmock.GraphQL(`query DiscussionCommentReplies\b`), - httpmock.StringResponse(getCommentRepliesResp(true, discNode, &emptyComment, "", 0, false, false, "", "")), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "hasDiscussionsEnabled": true, + "discussion": { + "id": "D_1", + "number": 1, + "title": "Test", + "body": "", + "url": "", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "C1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false + } + }, + "node": {} + } + } + `)), ) }, wantErr: "node I_notacomment is not a discussion comment", @@ -2073,16 +2482,8 @@ func TestGetCommentReplies(t *testing.T) { require.NoError(t, err) require.NotNil(t, d) require.Len(t, d.Comments.Comments, 1, "GetCommentReplies should return exactly one comment") - - comment := d.Comments.Comments[0] - assert.Len(t, comment.Replies.Comments, tt.wantReplies) - assert.Equal(t, tt.wantTotal, comment.Replies.TotalCount) - assert.Equal(t, tt.wantCursor, comment.Replies.Cursor) - assert.Equal(t, tt.wantNext, comment.Replies.NextCursor) - assert.Equal(t, tt.wantDirection, comment.Replies.Direction) - if tt.wantDisc != nil { - tt.wantDisc(t, d) - } + require.NotNil(t, tt.assertDisc, "assertDisc must be set for non-error cases") + tt.assertDisc(t, d) }) } } From cf40f9293d2052671a79a29bacfa941c42238e25 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 28 Apr 2026 10:41:54 +0100 Subject: [PATCH 6/8] chore(discussion/client): polish and cleanup tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl_test.go b/pkg/cmd/discussion/client/client_impl_test.go index 824de4219..16c0813a2 100644 --- a/pkg/cmd/discussion/client/client_impl_test.go +++ b/pkg/cmd/discussion/client/client_impl_test.go @@ -105,10 +105,6 @@ func searchResp(hasNext bool, cursor string, count int, nodes string) string { `, count, hasNext, cursor, nodes) } -// --------------------------------------------------------------------------- -// List -// --------------------------------------------------------------------------- - func TestList(t *testing.T) { repo := ghrepo.New("OWNER", "REPO") @@ -1160,10 +1156,6 @@ func TestGetByNumber(t *testing.T) { } } -// --------------------------------------------------------------------------- -// GetWithComments -// --------------------------------------------------------------------------- - func TestGetWithComments(t *testing.T) { repo := ghrepo.New("OWNER", "REPO") @@ -1822,10 +1814,6 @@ func TestGetWithComments(t *testing.T) { } } -// --------------------------------------------------------------------------- -// GetCommentReplies -// --------------------------------------------------------------------------- - func TestGetCommentReplies(t *testing.T) { repo := ghrepo.New("OWNER", "REPO") From 54ddede12f1ab8779e7d6ce69b4a1bfb4e92dc2d Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 28 Apr 2026 11:03:55 +0100 Subject: [PATCH 7/8] test(discussion/view): consolidate NewCmdView flag parsing tests Replace scattered standalone test functions with a single table-driven TestNewCmdView covering all arg/flag parsing: number, hash, URL, web, comments, replies, limit, after, order, mutual exclusivity, and invalid inputs. Uses shlex.Split for arg parsing and asserts opts fields individually per case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/view/view_test.go | 196 ++++++++++++++++++++++++--- 1 file changed, 178 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/discussion/view/view_test.go b/pkg/cmd/discussion/view/view_test.go index 3f5e0348c..71897a173 100644 --- a/pkg/cmd/discussion/view/view_test.go +++ b/pkg/cmd/discussion/view/view_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/discussion/client" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/google/shlex" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -40,36 +41,182 @@ func testDiscussion() *client.Discussion { func TestNewCmdView(t *testing.T) { tests := []struct { - name string - args []string - wantNum int - wantErr string + name string + args string + wantErr string + wantOpts ViewOptions + wantRepo string }{ { - name: "number argument", - args: []string{"123"}, - wantNum: 123, + name: "number argument", + args: "123", + wantOpts: ViewOptions{ + DiscussionNumber: 123, + Limit: 30, + Order: "newest", + }, }, { - name: "hash number argument", - args: []string{"#456"}, - wantNum: 456, + name: "hash number argument", + args: "'#456'", + wantOpts: ViewOptions{ + DiscussionNumber: 456, + Limit: 30, + Order: "newest", + }, }, { - name: "URL argument", - args: []string{"https://github.com/OWNER/REPO/discussions/789"}, - wantNum: 789, + name: "URL argument", + args: "https://github.com/OTHER/REPO/discussions/789", + wantOpts: ViewOptions{ + DiscussionNumber: 789, + Limit: 30, + Order: "newest", + }, + wantRepo: "OTHER/REPO", }, { name: "invalid argument", - args: []string{"not-a-number"}, + args: "not-a-number", wantErr: "invalid discussion argument", }, { name: "no arguments", - args: []string{}, + args: "", wantErr: "accepts 1 arg(s), received 0", }, + { + name: "web flag", + args: "123 --web", + wantOpts: ViewOptions{ + DiscussionNumber: 123, + WebMode: true, + Limit: 30, + Order: "newest", + }, + }, + { + name: "comments flag", + args: "123 --comments", + wantOpts: ViewOptions{ + DiscussionNumber: 123, + Comments: true, + Limit: 30, + Order: "newest", + }, + }, + { + name: "comments with limit", + args: "123 --comments --limit 10", + wantOpts: ViewOptions{ + DiscussionNumber: 123, + Comments: true, + Limit: 10, + Order: "newest", + }, + }, + { + name: "comments with after", + args: "123 --comments --after CURSOR_ABC", + wantOpts: ViewOptions{ + DiscussionNumber: 123, + Comments: true, + Limit: 30, + After: "CURSOR_ABC", + Order: "newest", + }, + }, + { + name: "comments with order oldest", + args: "123 --comments --order oldest", + wantOpts: ViewOptions{ + DiscussionNumber: 123, + Comments: true, + Limit: 30, + Order: "oldest", + }, + }, + { + name: "replies flag", + args: "123 --replies DC_abc", + wantOpts: ViewOptions{ + DiscussionNumber: 123, + Replies: "DC_abc", + Limit: 30, + Order: "newest", + }, + }, + { + name: "replies with limit", + args: "123 --replies DC_abc --limit 10", + wantOpts: ViewOptions{ + DiscussionNumber: 123, + Replies: "DC_abc", + Limit: 10, + Order: "newest", + }, + }, + { + name: "replies with after", + args: "123 --replies DC_abc --after CURSOR", + wantOpts: ViewOptions{ + DiscussionNumber: 123, + Replies: "DC_abc", + Limit: 30, + After: "CURSOR", + Order: "newest", + }, + }, + { + name: "replies with order oldest", + args: "123 --replies DC_abc --order oldest", + wantOpts: ViewOptions{ + DiscussionNumber: 123, + Replies: "DC_abc", + Limit: 30, + Order: "oldest", + }, + }, + { + name: "replies with comments is mutually exclusive", + args: "123 --replies DC_abc --comments", + wantErr: "specify only one of --comments, --replies, or --web", + }, + { + name: "replies with web is mutually exclusive", + args: "123 --replies DC_abc --web", + wantErr: "specify only one of --comments, --replies, or --web", + }, + { + name: "comments with web is mutually exclusive", + args: "123 --comments --web", + wantErr: "specify only one of --comments, --replies, or --web", + }, + { + name: "order requires comments or replies", + args: "123 --order newest", + wantErr: "--order requires --comments or --replies", + }, + { + name: "limit requires comments or replies", + args: "123 --limit 5", + wantErr: "--limit requires --comments or --replies", + }, + { + name: "after requires comments or replies", + args: "123 --after CURSOR", + wantErr: "--after requires --comments or --replies", + }, + { + name: "invalid limit zero", + args: "123 --comments --limit 0", + wantErr: "invalid limit", + }, + { + name: "invalid limit negative", + args: "123 --comments --limit -5", + wantErr: "invalid limit", + }, } for _, tt := range tests { @@ -88,18 +235,31 @@ func TestNewCmdView(t *testing.T) { return nil }) - cmd.SetArgs(tt.args) + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) cmd.SetOut(&bytes.Buffer{}) cmd.SetErr(&bytes.Buffer{}) - err := cmd.Execute() + _, err = cmd.ExecuteC() if tt.wantErr != "" { require.Error(t, err) assert.Contains(t, err.Error(), tt.wantErr) return } require.NoError(t, err) - assert.Equal(t, tt.wantNum, gotOpts.DiscussionNumber) + repo, err := gotOpts.BaseRepo() + require.NoError(t, err) + if tt.wantRepo != "" { + assert.Equal(t, tt.wantRepo, ghrepo.FullName(repo)) + } + assert.Equal(t, tt.wantOpts.DiscussionNumber, gotOpts.DiscussionNumber) + assert.Equal(t, tt.wantOpts.WebMode, gotOpts.WebMode) + assert.Equal(t, tt.wantOpts.Comments, gotOpts.Comments) + assert.Equal(t, tt.wantOpts.Replies, gotOpts.Replies) + assert.Equal(t, tt.wantOpts.Limit, gotOpts.Limit) + assert.Equal(t, tt.wantOpts.After, gotOpts.After) + assert.Equal(t, tt.wantOpts.Order, gotOpts.Order) }) } } From 52f219a5aca05384e76bc243768214022248cbb3 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Wed, 29 Apr 2026 13:05:21 +0100 Subject: [PATCH 8/8] test(discussion view): consolidate view run tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/view/view_test.go | 1798 ++++++++++++-------------- 1 file changed, 823 insertions(+), 975 deletions(-) diff --git a/pkg/cmd/discussion/view/view_test.go b/pkg/cmd/discussion/view/view_test.go index 71897a173..0c975f3c2 100644 --- a/pkg/cmd/discussion/view/view_test.go +++ b/pkg/cmd/discussion/view/view_test.go @@ -2,41 +2,46 @@ package view import ( "bytes" + "encoding/json" + "fmt" "testing" "time" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/discussion/client" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/pkg/jsonfieldstest" "github.com/google/shlex" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func testDiscussion() *client.Discussion { - return &client.Discussion{ - ID: "D_123", - Number: 123, - Title: "How to authenticate with SSO?", - Body: "I need help with SSO authentication.", - URL: "https://github.com/OWNER/REPO/discussions/123", - Closed: false, - Author: client.DiscussionActor{Login: "monalisa"}, - Category: client.DiscussionCategory{ - Name: "Q&A", Slug: "q-a", IsAnswerable: true, - }, - Labels: []client.DiscussionLabel{{Name: "help-wanted", Color: "0075ca"}}, - Answered: false, - Comments: client.DiscussionCommentList{TotalCount: 3}, - ReactionGroups: []client.ReactionGroup{ - {Content: "THUMBS_UP", TotalCount: 5}, - {Content: "ROCKET", TotalCount: 2}, - }, - CreatedAt: time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC), - UpdatedAt: time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC), - } +func TestJSONFields(t *testing.T) { + jsonfieldstest.ExpectCommandToSupportJSONFields(t, NewCmdView, []string{ + "id", + "number", + "title", + "body", + "url", + "closed", + "state", + "stateReason", + "author", + "category", + "labels", + "answered", + "answerChosenAt", + "answerChosenBy", + "comments", + "reactionGroups", + "createdAt", + "updatedAt", + "closedAt", + "locked", + }) } func TestNewCmdView(t *testing.T) { @@ -264,257 +269,750 @@ func TestNewCmdView(t *testing.T) { } } -func TestViewRun_tty(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) +func TestViewRun(t *testing.T) { + fixedNow := func() time.Time { return time.Date(2025, 3, 1, 1, 0, 0, 0, time.UTC) } - d := testDiscussion() - mock := &client.DiscussionClientMock{ - GetByNumberFunc: func(repo ghrepo.Interface, number int) (*client.Discussion, error) { - return d, nil + tests := []struct { + name string + tty bool + clientStub func(*testing.T, *client.DiscussionClientMock) + opts ViewOptions + wantStdout string + wantStderr string + wantBrowser string + }{ + { + name: "tty", + tty: true, + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetByNumberFunc = func(repo ghrepo.Interface, number int) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + return exampleAnswerableDiscussion(), nil + } + }, + wantStdout: heredoc.Doc(` + an interesting question #123 + Open · Q&A · Asked by monalisa · about 1 hour ago · 3 comments + Labels: help-wanted + + + about my interesting question + + + 👍 5 • 🚀 2 + + View this discussion on GitHub: https://github.com/OWNER/REPO/discussions/123 + `), + }, + { + name: "nontty", + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetByNumberFunc = func(repo ghrepo.Interface, number int) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + return exampleAnswerableDiscussion(), nil + } + }, + wantStdout: heredoc.Doc(` + title: an interesting question + state: OPEN + category: Q&A + author: monalisa + labels: help-wanted + comments: 3 + number: 123 + url: https://github.com/OWNER/REPO/discussions/123 + -- + about my interesting question + `), + }, + { + name: "web", + tty: true, + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetByNumberFunc = func(repo ghrepo.Interface, number int) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + return exampleAnswerableDiscussion(), nil + } + }, + opts: ViewOptions{ + WebMode: true, + }, + wantStderr: "Opening https://github.com/OWNER/REPO/discussions/123 in your browser.\n", + wantBrowser: "https://github.com/OWNER/REPO/discussions/123", + }, + { + name: "not answerable tty", + tty: true, + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetByNumberFunc = func(repo ghrepo.Interface, number int) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + return exampleUnanswerableDiscussion(), nil + } + }, + wantStdout: heredoc.Doc(` + a cool discussion #123 + Open · General · Started by monalisa · about 1 hour ago · 3 comments + Labels: help-wanted + + + about my cool idea + + + 👍 5 • 🚀 2 + + View this discussion on GitHub: https://github.com/OWNER/REPO/discussions/123 + `), + }, + { + name: "comments tty", + tty: true, + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetWithCommentsFunc = func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, 30, commentLimit) + assert.Equal(t, "", after) + assert.Equal(t, false, newest) + return exampleDiscussionWithComments(), nil + } + }, + opts: ViewOptions{ + Comments: true, + Order: "oldest", + }, + wantStdout: heredoc.Doc(` + an interesting question #123 + Open · Q&A · Asked by monalisa · about 1 hour ago · 2 comments + Labels: help-wanted + + + about my interesting question + + + 👍 5 • 🚀 2 + + Comments + + octocat commented less than a minute ago ✓ Answer + + This is a comment + + 👍 3 + + hubot commented less than a minute ago + + Thanks! + + + And 4 more replies + + monalisa commented less than a minute ago + + Another comment + + + View this discussion on GitHub: https://github.com/OWNER/REPO/discussions/123 + `), + }, + { + name: "comments nontty", + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetWithCommentsFunc = func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, 30, commentLimit) + assert.Equal(t, "", after) + assert.Equal(t, false, newest) + return exampleDiscussionWithComments(), nil + } + }, + opts: ViewOptions{ + Comments: true, + Order: "oldest", + }, + wantStdout: heredoc.Doc(` + title: an interesting question + state: OPEN + category: Q&A + author: monalisa + labels: help-wanted + comments: 2 + number: 123 + url: https://github.com/OWNER/REPO/discussions/123 + -- + about my interesting question + comment: octocat 2025-03-02T00:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-1 answer + -- + This is a comment + comment: hubot 2025-03-02T01:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-2 + -- + Thanks! + comment: monalisa 2025-03-03T00:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-3 + -- + Another comment + `), + }, + { + name: "comments pagination tty", + tty: true, + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + d := exampleDiscussionWithComments() + d.Comments.NextCursor = "NEXT_CURSOR_123" + m.GetWithCommentsFunc = func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, 10, commentLimit) + assert.Equal(t, "CURSOR_ABC", after) + assert.Equal(t, false, newest) + return d, nil + } + }, + opts: ViewOptions{ + Comments: true, + Limit: 10, + After: "CURSOR_ABC", + Order: "oldest", + }, + wantStdout: heredoc.Doc(` + an interesting question #123 + Open · Q&A · Asked by monalisa · about 1 hour ago · 2 comments + Labels: help-wanted + + + about my interesting question + + + 👍 5 • 🚀 2 + + Comments + + octocat commented less than a minute ago ✓ Answer + + This is a comment + + 👍 3 + + hubot commented less than a minute ago + + Thanks! + + + And 4 more replies + + monalisa commented less than a minute ago + + Another comment + + + To see more comments, pass: --after NEXT_CURSOR_123 + + View this discussion on GitHub: https://github.com/OWNER/REPO/discussions/123 + `), + }, + { + name: "comments pagination nontty", + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + d := exampleDiscussionWithComments() + d.Comments.NextCursor = "NEXT_CURSOR_456" + m.GetWithCommentsFunc = func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, 30, commentLimit) + assert.Equal(t, "", after) + assert.Equal(t, false, newest) + return d, nil + } + }, + opts: ViewOptions{ + Comments: true, + Order: "oldest", + }, + wantStdout: heredoc.Doc(` + title: an interesting question + state: OPEN + category: Q&A + author: monalisa + labels: help-wanted + comments: 2 + next: NEXT_CURSOR_456 + number: 123 + url: https://github.com/OWNER/REPO/discussions/123 + -- + about my interesting question + comment: octocat 2025-03-02T00:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-1 answer + -- + This is a comment + comment: hubot 2025-03-02T01:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-2 + -- + Thanks! + comment: monalisa 2025-03-03T00:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-3 + -- + Another comment + `), + }, + { + name: "json without comments field", + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetByNumberFunc = func(repo ghrepo.Interface, number int) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + return exampleAnswerableDiscussion(), nil + } + }, + opts: ViewOptions{ + Exporter: jsonExporter("title", "url"), + }, + wantStdout: compactJSON(heredoc.Doc(` + { + "title": "an interesting question", + "url": "https://github.com/OWNER/REPO/discussions/123" + } + `)), + }, + { + name: "json with comments field", + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetWithCommentsFunc = func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, 30, commentLimit) + assert.Equal(t, "", after) + assert.Equal(t, true, newest) + return exampleDiscussionWithComments(), nil + } + }, + opts: ViewOptions{ + Exporter: jsonExporter("comments"), + }, + wantStdout: compactJSON(heredoc.Doc(` + { + "comments": { + "nodes": [ + { + "author": {"id": "", "login": "octocat", "name": ""}, + "body": "This is a comment", + "createdAt": "2025-03-02T00:00:00Z", + "id": "C_1", + "isAnswer": true, + "reactionGroups": [ + {"content": "THUMBS_UP", "totalCount": 3} + ], + "replies": { + "nodes": [ + { + "author": {"id": "", "login": "hubot", "name": ""}, + "body": "Thanks!", + "createdAt": "2025-03-02T01:00:00Z", + "id": "C_1_R1", + "isAnswer": false, + "reactionGroups": [], + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-2" + } + ], + "totalCount": 5 + }, + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-1" + }, + { + "author": {"id": "", "login": "monalisa", "name": ""}, + "body": "Another comment", + "createdAt": "2025-03-03T00:00:00Z", + "id": "C_2", + "isAnswer": false, + "reactionGroups": [], + "replies": { + "nodes": [], + "totalCount": 0 + }, + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-3" + } + ], + "totalCount": 2 + } + } + `)), + }, + { + name: "json with comments field pagination", + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetWithCommentsFunc = func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, 30, commentLimit) + assert.Equal(t, "", after) + assert.Equal(t, true, newest) + d := exampleDiscussionWithComments() + d.Comments.NextCursor = "NEXT_COM_CUR" + return d, nil + } + }, + opts: ViewOptions{ + Exporter: jsonExporter("comments"), + }, + wantStdout: compactJSON(heredoc.Doc(` + { + "comments": { + "next": "NEXT_COM_CUR", + "nodes": [ + { + "author": {"id": "", "login": "octocat", "name": ""}, + "body": "This is a comment", + "createdAt": "2025-03-02T00:00:00Z", + "id": "C_1", + "isAnswer": true, + "reactionGroups": [ + {"content": "THUMBS_UP", "totalCount": 3} + ], + "replies": { + "nodes": [ + { + "author": {"id": "", "login": "hubot", "name": ""}, + "body": "Thanks!", + "createdAt": "2025-03-02T01:00:00Z", + "id": "C_1_R1", + "isAnswer": false, + "reactionGroups": [], + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-2" + } + ], + "totalCount": 5 + }, + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-1" + }, + { + "author": {"id": "", "login": "monalisa", "name": ""}, + "body": "Another comment", + "createdAt": "2025-03-03T00:00:00Z", + "id": "C_2", + "isAnswer": false, + "reactionGroups": [], + "replies": { + "nodes": [], + "totalCount": 0 + }, + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-3" + } + ], + "totalCount": 2 + } + } + `)), + }, + { + name: "replies tty", + tty: true, + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetCommentRepliesFunc = func(repo ghrepo.Interface, number int, commentID string, limit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, "DC_abc", commentID) + assert.Equal(t, 30, limit) + assert.Equal(t, "", after) + assert.Equal(t, true, newest) + return exampleDiscussionWithReplies(""), nil + } + }, + opts: ViewOptions{ + Replies: "DC_abc", + }, + wantStdout: heredoc.Doc(` + octocat commented less than a minute ago ✓ Answer + + This is the parent comment + + 👍 3 + + hubot commented less than a minute ago + + First reply + + + monalisa commented less than a minute ago + + Second reply + + + `), + }, + { + name: "replies pagination tty", + tty: true, + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetCommentRepliesFunc = func(repo ghrepo.Interface, number int, commentID string, limit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, "DC_abc", commentID) + assert.Equal(t, 30, limit) + assert.Equal(t, "", after) + assert.Equal(t, true, newest) + return exampleDiscussionWithReplies("NEXT_CUR"), nil + } + }, + opts: ViewOptions{ + Replies: "DC_abc", + }, + wantStdout: heredoc.Doc(` + octocat commented less than a minute ago ✓ Answer + + This is the parent comment + + 👍 3 + + hubot commented less than a minute ago + + First reply + + + monalisa commented less than a minute ago + + Second reply + + + To see more replies, pass: --after NEXT_CUR + + `), + }, + { + name: "replies nontty", + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetCommentRepliesFunc = func(repo ghrepo.Interface, number int, commentID string, limit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, "DC_abc", commentID) + assert.Equal(t, 30, limit) + assert.Equal(t, "", after) + assert.Equal(t, false, newest) + return exampleDiscussionWithReplies(""), nil + } + }, + opts: ViewOptions{ + Replies: "DC_abc", + Order: "oldest", + }, + wantStdout: heredoc.Doc(` + comment: octocat 2025-03-02T00:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-1 answer + replies: 2 + -- + This is the parent comment + comment: hubot 2025-03-02T01:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-2 + -- + First reply + comment: monalisa 2025-03-02T02:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-3 + -- + Second reply + `), + }, + { + name: "replies pagination nontty", + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetCommentRepliesFunc = func(repo ghrepo.Interface, number int, commentID string, limit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, "DC_abc", commentID) + assert.Equal(t, 30, limit) + assert.Equal(t, "", after) + assert.Equal(t, false, newest) + return exampleDiscussionWithReplies("NEXT_CUR_456"), nil + } + }, + opts: ViewOptions{ + Replies: "DC_abc", + Order: "oldest", + }, + wantStdout: heredoc.Doc(` + comment: octocat 2025-03-02T00:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-1 answer + replies: 2 + next: NEXT_CUR_456 + -- + This is the parent comment + comment: hubot 2025-03-02T01:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-2 + -- + First reply + comment: monalisa 2025-03-02T02:00:00Z https://github.com/OWNER/REPO/discussions/123#discussioncomment-3 + -- + Second reply + `), + }, + { + name: "replies json", + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetCommentRepliesFunc = func(repo ghrepo.Interface, number int, commentID string, limit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, "DC_abc", commentID) + assert.Equal(t, 30, limit) + assert.Equal(t, "", after) + assert.Equal(t, true, newest) + return exampleDiscussionWithReplies(""), nil + } + }, + opts: ViewOptions{ + Replies: "DC_abc", + Exporter: jsonExporter("comments"), + }, + wantStdout: compactJSON(heredoc.Doc(` + { + "comments": { + "nodes": [ + { + "author": {"id": "", "login": "octocat", "name": ""}, + "body": "This is the parent comment", + "createdAt": "2025-03-02T00:00:00Z", + "id": "DC_abc", + "isAnswer": true, + "reactionGroups": [ + {"content": "THUMBS_UP", "totalCount": 3} + ], + "replies": { + "nodes": [ + { + "author": {"id": "", "login": "hubot", "name": ""}, + "body": "First reply", + "createdAt": "2025-03-02T01:00:00Z", + "id": "R1", + "isAnswer": false, + "reactionGroups": [], + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-2" + }, + { + "author": {"id": "", "login": "monalisa", "name": ""}, + "body": "Second reply", + "createdAt": "2025-03-02T02:00:00Z", + "id": "R2", + "isAnswer": false, + "reactionGroups": [], + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-3" + } + ], + "totalCount": 2 + }, + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-1" + } + ], + "totalCount": 1 + } + } + `)), + }, + { + name: "replies json pagination", + clientStub: func(t *testing.T, m *client.DiscussionClientMock) { + m.GetCommentRepliesFunc = func(repo ghrepo.Interface, number int, commentID string, limit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, "OWNER/REPO", ghrepo.FullName(repo)) + assert.Equal(t, 123, number) + assert.Equal(t, "DC_abc", commentID) + assert.Equal(t, 30, limit) + assert.Equal(t, "", after) + assert.Equal(t, true, newest) + return exampleDiscussionWithReplies("NEXT_REP_CUR"), nil + } + }, + opts: ViewOptions{ + Replies: "DC_abc", + Exporter: jsonExporter("comments"), + }, + wantStdout: compactJSON(heredoc.Doc(` + { + "comments": { + "nodes": [ + { + "author": {"id": "", "login": "octocat", "name": ""}, + "body": "This is the parent comment", + "createdAt": "2025-03-02T00:00:00Z", + "id": "DC_abc", + "isAnswer": true, + "reactionGroups": [ + {"content": "THUMBS_UP", "totalCount": 3} + ], + "replies": { + "next": "NEXT_REP_CUR", + "nodes": [ + { + "author": {"id": "", "login": "hubot", "name": ""}, + "body": "First reply", + "createdAt": "2025-03-02T01:00:00Z", + "id": "R1", + "isAnswer": false, + "reactionGroups": [], + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-2" + }, + { + "author": {"id": "", "login": "monalisa", "name": ""}, + "body": "Second reply", + "createdAt": "2025-03-02T02:00:00Z", + "id": "R2", + "isAnswer": false, + "reactionGroups": [], + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-3" + } + ], + "totalCount": 2 + }, + "upvoteCount": 0, + "url": "https://github.com/OWNER/REPO/discussions/123#discussioncomment-1" + } + ], + "totalCount": 1 + } + } + `)), }, } - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Now: func() time.Time { return time.Date(2025, 3, 1, 1, 0, 0, 0, time.UTC) }, + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(tt.tty) + ios.SetStderrTTY(tt.tty) + + mock := &client.DiscussionClientMock{} + tt.clientStub(t, mock) + + b := &browser.Stub{} + + opts := tt.opts + opts.IO = ios + opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil } + opts.Client = func() (client.DiscussionClient, error) { return mock, nil } + opts.Browser = b + opts.DiscussionNumber = 123 + opts.Now = fixedNow + if opts.Limit == 0 { + opts.Limit = 30 + } + if opts.Order == "" { + opts.Order = "newest" + } + + err := viewRun(&opts) + require.NoError(t, err) + + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + if tt.wantBrowser != "" { + b.Verify(t, tt.wantBrowser) + } + }) } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - assert.Contains(t, out, "How to authenticate with SSO?") - assert.Contains(t, out, "#123") - assert.Contains(t, out, "Q&A") - assert.Contains(t, out, "Asked by") - assert.Contains(t, out, "monalisa") - assert.Contains(t, out, "3 comments") - assert.Contains(t, out, "help-wanted") - assert.Contains(t, out, "View this discussion on GitHub") } -func TestViewRun_nontty(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(false) - - d := testDiscussion() - mock := &client.DiscussionClientMock{ - GetByNumberFunc: func(repo ghrepo.Interface, number int) (*client.Discussion, error) { - return d, nil - }, - } - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Now: time.Now, - } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - assert.Contains(t, out, "title:\tHow to authenticate with SSO?") - assert.Contains(t, out, "state:\tOPEN") - assert.Contains(t, out, "category:\tQ&A") - assert.Contains(t, out, "author:\tmonalisa") - assert.Contains(t, out, "labels:\thelp-wanted") - assert.Contains(t, out, "number:\t123") - assert.Contains(t, out, "--") - assert.Contains(t, out, "I need help with SSO authentication.") -} - -func TestViewRun_json(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(false) - - d := testDiscussionWithComments() - mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { - return d, nil - }, - } - - exporter := cmdutil.NewJSONExporter() - exporter.SetFields(discussionFields) - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Limit: 30, - Order: "newest", - Exporter: exporter, - Now: time.Now, - } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - assert.Contains(t, out, `"title"`) - assert.Contains(t, out, `"number"`) - assert.Contains(t, out, "How to authenticate with SSO?") -} - -func TestViewRun_web(t *testing.T) { - ios, _, _, stderr := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - - b := &browser.Stub{} - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Browser: b, - DiscussionNumber: 123, - WebMode: true, - Now: time.Now, - } - - err := viewRun(opts) - require.NoError(t, err) - - b.Verify(t, "https://github.com/OWNER/REPO/discussions/123") - assert.Contains(t, stderr.String(), "Opening") -} - -func TestViewRun_urlArg(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(false) - - d := testDiscussion() - d.URL = "https://github.com/OTHER/REPO/discussions/42" - d.Number = 42 - - mock := &client.DiscussionClientMock{ - GetByNumberFunc: func(repo ghrepo.Interface, number int) (*client.Discussion, error) { - assert.Equal(t, "OTHER", repo.RepoOwner()) - assert.Equal(t, "REPO", repo.RepoName()) - assert.Equal(t, 42, number) - return d, nil - }, - } - - f := &cmdutil.Factory{} - f.IOStreams = ios - f.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - } - f.Browser = &browser.Stub{} - - var gotOpts *ViewOptions - cmd := NewCmdView(f, func(opts *ViewOptions) error { - gotOpts = opts - opts.Client = func() (client.DiscussionClient, error) { - return mock, nil - } - return viewRun(opts) - }) - - cmd.SetArgs([]string{"https://github.com/OTHER/REPO/discussions/42"}) - cmd.SetOut(&bytes.Buffer{}) - cmd.SetErr(&bytes.Buffer{}) - - err := cmd.Execute() - require.NoError(t, err) - assert.Equal(t, 42, gotOpts.DiscussionNumber) - - out := stdout.String() - assert.Contains(t, out, "number:\t42") -} - -func TestViewRun_answerable(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - - d := testDiscussion() - d.Category.IsAnswerable = true - - mock := &client.DiscussionClientMock{ - GetByNumberFunc: func(repo ghrepo.Interface, number int) (*client.Discussion, error) { - return d, nil - }, - } - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Now: func() time.Time { return time.Date(2025, 3, 1, 1, 0, 0, 0, time.UTC) }, - } - - err := viewRun(opts) - require.NoError(t, err) - assert.Contains(t, stdout.String(), "Asked by") -} - -func TestViewRun_notAnswerable(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - - d := testDiscussion() - d.Category.Name = "General" - d.Category.IsAnswerable = false - - mock := &client.DiscussionClientMock{ - GetByNumberFunc: func(repo ghrepo.Interface, number int) (*client.Discussion, error) { - return d, nil - }, - } - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Now: func() time.Time { return time.Date(2025, 3, 1, 1, 0, 0, 0, time.UTC) }, - } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - assert.Contains(t, out, "Started by") - assert.NotContains(t, out, "Asked by") -} - -func testDiscussionWithComments() *client.Discussion { - d := testDiscussion() +func exampleDiscussionWithComments() *client.Discussion { + d := exampleAnswerableDiscussion() d.Comments = client.DiscussionCommentList{ TotalCount: 2, Comments: []client.DiscussionComment{ @@ -553,558 +1051,8 @@ func testDiscussionWithComments() *client.Discussion { return d } -func TestViewRun_comments_tty(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - - d := testDiscussionWithComments() - mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { - assert.Equal(t, 30, commentLimit) - assert.Equal(t, false, newest) - return d, nil - }, - } - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Comments: true, - Limit: 30, - Order: "oldest", - Now: func() time.Time { return time.Date(2025, 3, 4, 0, 0, 0, 0, time.UTC) }, - } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - assert.Contains(t, out, "Comments") - assert.Contains(t, out, "octocat") - assert.Contains(t, out, "✓ Answer") - assert.Contains(t, out, "This is a comment") - assert.Contains(t, out, "hubot") - assert.Contains(t, out, "Thanks!") - assert.Contains(t, out, "And 4 more replies") - assert.Contains(t, out, "monalisa") - assert.Contains(t, out, "Another comment") -} - -func TestViewRun_comments_nontty(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(false) - - d := testDiscussionWithComments() - mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { - return d, nil - }, - } - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Comments: true, - Limit: 30, - Order: "oldest", - Now: time.Now, - } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - assert.Contains(t, out, "comment:\toctocat\t") - assert.Contains(t, out, "answer") - assert.Contains(t, out, "This is a comment") - assert.Contains(t, out, "comment:\thubot\t") - assert.Contains(t, out, "comment:\tmonalisa\t") -} - -func TestViewRun_comments_json(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(false) - - d := testDiscussionWithComments() - mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { - return d, nil - }, - } - - exporter := cmdutil.NewJSONExporter() - exporter.SetFields(discussionFields) - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Comments: true, - Limit: 30, - Order: "oldest", - Exporter: exporter, - Now: time.Now, - } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - assert.Contains(t, out, `"totalCount"`) - assert.Contains(t, out, `"isAnswer":true`) - assert.Contains(t, out, `"octocat"`) -} - -func TestNewCmdView_orderWithoutComments(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, _, _ := iostreams.Test() - f.IOStreams = ios - f.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - } - f.Browser = &browser.Stub{} - - cmd := NewCmdView(f, func(opts *ViewOptions) error { - return nil - }) - - cmd.SetArgs([]string{"123", "--order", "newest"}) - cmd.SetOut(&bytes.Buffer{}) - cmd.SetErr(&bytes.Buffer{}) - - err := cmd.Execute() - require.Error(t, err) - assert.Contains(t, err.Error(), "--order requires --comments") -} - -func TestViewRun_noComments_usesGetByNumber(t *testing.T) { - ios, _, _, _ := iostreams.Test() - ios.SetStdoutTTY(false) - - d := testDiscussion() - mock := &client.DiscussionClientMock{ - GetByNumberFunc: func(repo ghrepo.Interface, number int) (*client.Discussion, error) { - return d, nil - }, - } - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Comments: false, - Now: time.Now, - } - - err := viewRun(opts) - require.NoError(t, err) - - assert.Equal(t, 1, len(mock.GetByNumberCalls())) - assert.Equal(t, 0, len(mock.GetWithCommentsCalls())) -} - -func TestNewCmdView_limitWithoutComments(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, _, _ := iostreams.Test() - f.IOStreams = ios - f.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - } - f.Browser = &browser.Stub{} - - cmd := NewCmdView(f, func(opts *ViewOptions) error { - return nil - }) - - cmd.SetArgs([]string{"123", "--limit", "10"}) - cmd.SetOut(&bytes.Buffer{}) - cmd.SetErr(&bytes.Buffer{}) - - err := cmd.Execute() - require.Error(t, err) - assert.Contains(t, err.Error(), "--limit requires --comments") -} - -func TestNewCmdView_afterWithoutComments(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, _, _ := iostreams.Test() - f.IOStreams = ios - f.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - } - f.Browser = &browser.Stub{} - - cmd := NewCmdView(f, func(opts *ViewOptions) error { - return nil - }) - - cmd.SetArgs([]string{"123", "--after", "CURSOR_ABC"}) - cmd.SetOut(&bytes.Buffer{}) - cmd.SetErr(&bytes.Buffer{}) - - err := cmd.Execute() - require.Error(t, err) - assert.Contains(t, err.Error(), "--after requires --comments") -} - -func TestNewCmdView_invalidLimit(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, _, _ := iostreams.Test() - f.IOStreams = ios - f.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - } - f.Browser = &browser.Stub{} - - cmd := NewCmdView(f, func(opts *ViewOptions) error { - return nil - }) - - cmd.SetArgs([]string{"123", "--comments", "--limit", "0"}) - cmd.SetOut(&bytes.Buffer{}) - cmd.SetErr(&bytes.Buffer{}) - - err := cmd.Execute() - require.Error(t, err) - assert.Contains(t, err.Error(), "invalid limit") -} - -func TestViewRun_commentsWithPagination_tty(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - - d := testDiscussionWithComments() - d.Comments.NextCursor = "NEXT_CURSOR_123" - - mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { - assert.Equal(t, 10, commentLimit) - assert.Equal(t, "CURSOR_ABC", after) - assert.Equal(t, false, newest) - return d, nil - }, - } - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Comments: true, - Limit: 10, - After: "CURSOR_ABC", - Order: "oldest", - Now: func() time.Time { return time.Date(2025, 3, 4, 0, 0, 0, 0, time.UTC) }, - } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - assert.Contains(t, out, "To see more comments, pass: --after NEXT_CURSOR_123") -} - -func TestViewRun_commentsWithPagination_nontty(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(false) - - d := testDiscussionWithComments() - d.Comments.NextCursor = "NEXT_CURSOR_456" - - mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { - return d, nil - }, - } - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Comments: true, - Limit: 30, - Order: "oldest", - Now: time.Now, - } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - assert.Contains(t, out, "next:\tNEXT_CURSOR_456") -} - -func TestViewRun_commentsWithPagination_json(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(false) - - d := testDiscussionWithComments() - d.Comments.Cursor = "PREV_CURSOR" - d.Comments.NextCursor = "NEXT_CURSOR_789" - - mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { - return d, nil - }, - } - - exporter := cmdutil.NewJSONExporter() - exporter.SetFields(discussionFields) - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Comments: true, - Limit: 30, - Order: "oldest", - Exporter: exporter, - Now: time.Now, - } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - assert.Contains(t, out, `"cursor":"PREV_CURSOR"`) - assert.Contains(t, out, `"next":"NEXT_CURSOR_789"`) -} - -func TestViewRun_noPaginationCursor_tty(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - - d := testDiscussionWithComments() - - mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { - return d, nil - }, - } - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Comments: true, - Limit: 30, - Order: "oldest", - Now: func() time.Time { return time.Date(2025, 3, 4, 0, 0, 0, 0, time.UTC) }, - } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - assert.NotContains(t, out, "--after") -} - -func TestViewRun_jsonComments_usesGetWithComments(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(false) - - d := testDiscussionWithComments() - mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { - return d, nil - }, - } - - exporter := cmdutil.NewJSONExporter() - exporter.SetFields([]string{"comments"}) - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Comments: false, - Limit: 30, - Order: "newest", - Exporter: exporter, - Now: time.Now, - } - - err := viewRun(opts) - require.NoError(t, err) - - // --json comments should use GetWithComments even without --comments flag - assert.Equal(t, 0, len(mock.GetByNumberCalls())) - assert.Equal(t, 1, len(mock.GetWithCommentsCalls())) - - out := stdout.String() - assert.Contains(t, out, `"totalCount"`) - assert.Contains(t, out, `"octocat"`) -} - -func TestViewRun_jsonWithoutComments_usesGetByNumber(t *testing.T) { - ios, _, _, _ := iostreams.Test() - ios.SetStdoutTTY(false) - - d := testDiscussion() - mock := &client.DiscussionClientMock{ - GetByNumberFunc: func(repo ghrepo.Interface, number int) (*client.Discussion, error) { - return d, nil - }, - } - - exporter := cmdutil.NewJSONExporter() - exporter.SetFields([]string{"title", "number"}) - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Comments: false, - Exporter: exporter, - Now: time.Now, - } - - err := viewRun(opts) - require.NoError(t, err) - - // --json title,number should NOT fetch comments - assert.Equal(t, 1, len(mock.GetByNumberCalls())) - assert.Equal(t, 0, len(mock.GetWithCommentsCalls())) -} - -// --------------------------------------------------------------------------- -// --replies flag validation -// --------------------------------------------------------------------------- - -func TestNewCmdView_repliesFlags(t *testing.T) { - tests := []struct { - name string - args []string - wantErr string - }{ - { - name: "replies with comments is mutually exclusive", - args: []string{"123", "--replies", "DC_abc", "--comments"}, - wantErr: "specify only one of --comments, --replies, or --web", - }, - { - name: "replies with web is mutually exclusive", - args: []string{"123", "--replies", "DC_abc", "--web"}, - wantErr: "specify only one of --comments, --replies, or --web", - }, - { - name: "order requires comments or replies", - args: []string{"123", "--order", "newest"}, - wantErr: "--order requires --comments or --replies", - }, - { - name: "limit requires comments or replies", - args: []string{"123", "--limit", "5"}, - wantErr: "--limit requires --comments or --replies", - }, - { - name: "after requires comments or replies", - args: []string{"123", "--after", "CURSOR"}, - wantErr: "--after requires --comments or --replies", - }, - { - name: "order works with replies", - args: []string{"123", "--replies", "DC_abc", "--order", "oldest"}, - }, - { - name: "limit works with replies", - args: []string{"123", "--replies", "DC_abc", "--limit", "10"}, - }, - { - name: "after works with replies", - args: []string{"123", "--replies", "DC_abc", "--after", "CURSOR"}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, _, _ := iostreams.Test() - f.IOStreams = ios - f.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - } - f.Browser = &browser.Stub{} - - cmd := NewCmdView(f, func(opts *ViewOptions) error { - return nil - }) - - cmd.SetArgs(tt.args) - cmd.SetOut(&bytes.Buffer{}) - cmd.SetErr(&bytes.Buffer{}) - - err := cmd.Execute() - if tt.wantErr != "" { - require.Error(t, err) - assert.Contains(t, err.Error(), tt.wantErr) - return - } - require.NoError(t, err) - }) - } -} - -// --------------------------------------------------------------------------- -// --replies viewRun tests (table-driven) -// --------------------------------------------------------------------------- - -func testDiscussionWithReplies(nextCursor string) *client.Discussion { - d := testDiscussion() +func exampleDiscussionWithReplies(nextCursor string) *client.Discussion { + d := exampleAnswerableDiscussion() d.Comments = client.DiscussionCommentList{ TotalCount: 1, Comments: []client.DiscussionComment{ @@ -1144,164 +1092,64 @@ func testDiscussionWithReplies(nextCursor string) *client.Discussion { return d } -func TestViewRun_replies(t *testing.T) { - tests := []struct { - name string - tty bool - replies string - limit int - after string - order string - exporter cmdutil.Exporter - nextCursor string - wantContains []string - wantExcludes []string - wantClient func(*testing.T, *client.DiscussionClientMock) - }{ - { - name: "tty renders comment and replies", - tty: true, - replies: "DC_abc", - limit: 30, - order: "newest", - wantContains: []string{ - "octocat", - "This is the parent comment", - "✓ Answer", - "hubot", - "First reply", - "monalisa", - "Second reply", - }, +func exampleAnswerableDiscussion() *client.Discussion { + return &client.Discussion{ + ID: "D_123", + Number: 123, + Title: "an interesting question", + Body: "about my interesting question", + URL: "https://github.com/OWNER/REPO/discussions/123", + Closed: false, + Author: client.DiscussionActor{Login: "monalisa"}, + Category: client.DiscussionCategory{ + Name: "Q&A", Slug: "q-a", IsAnswerable: true, }, - { - name: "tty shows pagination hint", - tty: true, - replies: "DC_abc", - limit: 30, - order: "newest", - nextCursor: "NEXT_CUR", - wantContains: []string{ - "--after NEXT_CUR", - }, + Labels: []client.DiscussionLabel{{Name: "help-wanted", Color: "0075ca"}}, + Answered: false, + Comments: client.DiscussionCommentList{TotalCount: 3}, + ReactionGroups: []client.ReactionGroup{ + {Content: "THUMBS_UP", TotalCount: 5}, + {Content: "ROCKET", TotalCount: 2}, }, - { - name: "tty no pagination hint when no next cursor", - tty: true, - replies: "DC_abc", - limit: 30, - order: "newest", - wantExcludes: []string{ - "--after", - }, - }, - { - name: "nontty raw output", - tty: false, - replies: "DC_abc", - limit: 30, - order: "oldest", - wantContains: []string{ - "comment:\toctocat\t", - "answer", - "replies:\t2", - "This is the parent comment", - "hubot", - "First reply", - }, - }, - { - name: "nontty shows next cursor", - tty: false, - replies: "DC_abc", - limit: 30, - order: "oldest", - nextCursor: "NEXT_CUR_456", - wantContains: []string{ - "next:\tNEXT_CUR_456", - }, - }, - { - name: "json output", - tty: false, - replies: "DC_abc", - limit: 30, - order: "newest", - exporter: func() cmdutil.Exporter { - e := cmdutil.NewJSONExporter() - e.SetFields(discussionFields) - return e - }(), - wantContains: []string{ - `"totalCount"`, - `"isAnswer":true`, - `"octocat"`, - }, - }, - { - name: "routes to GetCommentReplies only", - tty: false, - replies: "DC_abc", - limit: 10, - after: "CUR_A", - order: "oldest", - wantClient: func(t *testing.T, mock *client.DiscussionClientMock) { - require.Equal(t, 1, len(mock.GetCommentRepliesCalls())) - assert.Equal(t, 0, len(mock.GetByNumberCalls())) - assert.Equal(t, 0, len(mock.GetWithCommentsCalls())) - - call := mock.GetCommentRepliesCalls()[0] - assert.Equal(t, "DC_abc", call.CommentID) - assert.Equal(t, 10, call.Limit) - assert.Equal(t, "CUR_A", call.After) - assert.Equal(t, false, call.Newest) - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(tt.tty) - ios.SetStderrTTY(tt.tty) - - d := testDiscussionWithReplies(tt.nextCursor) - mock := &client.DiscussionClientMock{ - GetCommentRepliesFunc: func(repo ghrepo.Interface, number int, commentID string, limit int, after string, newest bool) (*client.Discussion, error) { - return d, nil - }, - } - - opts := &ViewOptions{ - IO: ios, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Client: func() (client.DiscussionClient, error) { - return mock, nil - }, - DiscussionNumber: 123, - Replies: tt.replies, - Limit: tt.limit, - After: tt.after, - Order: tt.order, - Exporter: tt.exporter, - Now: func() time.Time { return time.Date(2025, 3, 4, 0, 0, 0, 0, time.UTC) }, - } - - err := viewRun(opts) - require.NoError(t, err) - - out := stdout.String() - for _, s := range tt.wantContains { - assert.Contains(t, out, s) - } - for _, s := range tt.wantExcludes { - assert.NotContains(t, out, s) - } - if tt.wantClient != nil { - tt.wantClient(t, mock) - } - }) + CreatedAt: time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC), } } + +func exampleUnanswerableDiscussion() *client.Discussion { + return &client.Discussion{ + ID: "D_123", + Number: 123, + Title: "a cool discussion", + Body: "about my cool idea", + URL: "https://github.com/OWNER/REPO/discussions/123", + Closed: false, + Author: client.DiscussionActor{Login: "monalisa"}, + Category: client.DiscussionCategory{ + Name: "General", Slug: "general", IsAnswerable: false, + }, + Labels: []client.DiscussionLabel{{Name: "help-wanted", Color: "0075ca"}}, + Answered: false, + Comments: client.DiscussionCommentList{TotalCount: 3}, + ReactionGroups: []client.ReactionGroup{ + {Content: "THUMBS_UP", TotalCount: 5}, + {Content: "ROCKET", TotalCount: 2}, + }, + CreatedAt: time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC), + } +} + +func compactJSON(s string) string { + var buf bytes.Buffer + if err := json.Compact(&buf, []byte(s)); err != nil { + panic(fmt.Sprintf("compactJSON: %v", err)) + } + return buf.String() + "\n" +} + +func jsonExporter(fields ...string) cmdutil.Exporter { + e := cmdutil.NewJSONExporter() + e.SetFields(fields) + return e +}