From 94fbbdf9b5b81a433c8bb60cd16b8d179822d834 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 19 Sep 2023 10:26:52 +0200 Subject: [PATCH] Cleanup release create around new --notes-from-tag flag (#8016) --- pkg/cmd/release/create/create.go | 28 +-- pkg/cmd/release/create/create_test.go | 254 +++++++++++++++----------- pkg/httpmock/stub.go | 1 + 3 files changed, 163 insertions(+), 120 deletions(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index aa021748b..a0307d34a 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -93,7 +93,8 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co To create a release from an annotated git tag, first create one locally with git, push the tag to GitHub, then run this command. - You may also use %[1]s--notes-from-tag%[1]s to create it noninteractively. + Use %[1]s--notes-from-tag%[1]s to automatically generate the release notes + from the annotated git tag. When using automatically generated release notes, a release title will also be automatically generated unless a title was explicitly passed. Additional release notes can be prepended to @@ -149,7 +150,11 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co } if opts.NotesFromTag && (opts.GenerateNotes || opts.NotesStartTag != "") { - return cmdutil.FlagErrorf("using `--notes-from-tag` with `--generate-notes` or `notes-start-tag` is not supported") + return cmdutil.FlagErrorf("using `--notes-from-tag` with `--generate-notes` or `--notes-start-tag` is not supported") + } + + if opts.NotesFromTag && opts.RepoOverride != "" { + return cmdutil.FlagErrorf("using `--notes-from-tag` with `--repo` is not supported") } opts.Concurrency = 5 @@ -182,7 +187,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd.Flags().StringVar(&opts.NotesStartTag, "notes-start-tag", "", "Tag to use as the starting point for generating release notes") cmdutil.NilBoolFlag(cmd, &opts.IsLatest, "latest", "", "Mark this release as \"Latest\" (default: automatic based on date and version)") cmd.Flags().BoolVarP(&opts.VerifyTag, "verify-tag", "", false, "Abort in case the git tag doesn't already exist in the remote repository") - cmd.Flags().BoolVarP(&opts.NotesFromTag, "notes-from-tag", "", false, "Automatically populate the release notes with the message in the tag specified by `gh release create []`") + cmd.Flags().BoolVarP(&opts.NotesFromTag, "notes-from-tag", "", false, "Automatically generate notes from annotated tag") _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "target") @@ -248,6 +253,12 @@ func createRun(opts *CreateOptions) error { var tagDescription string if opts.RepoOverride == "" { tagDescription, _ = gitTagInfo(opts.GitClient, opts.TagName) + + if opts.NotesFromTag && tagDescription == "" { + return fmt.Errorf("cannot generate release notes from tag %s as it does not exist locally", + opts.TagName) + } + // If there is a local tag with the same name as specified // the user may not want to create a new tag on the remote // as the local one might be annotated or signed. @@ -266,17 +277,6 @@ func createRun(opts *CreateOptions) error { opts.TagName, ghrepo.FullName(baseRepo)) } } - - if opts.NotesFromTag { - remoteExists, err := remoteTagExists(httpClient, baseRepo, opts.TagName) - if err != nil { - return err - } - if !remoteExists { - return fmt.Errorf("tag %s doesn't exist in the repo %s, cannot populate release notes with annotated git tag message using the `--notes-from-tag` flag", - opts.TagName, ghrepo.FullName(baseRepo)) - } - } } if !opts.BodyProvided && opts.IO.CanPrompt() { diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index 53af2168b..644177aba 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -322,41 +322,28 @@ func Test_NewCmdCreate(t *testing.T) { }, }, { - name: "tag and --notes-from-tag", + name: "with --notes-from-tag", args: "v1.2.3 --notes-from-tag", isTTY: false, want: CreateOptions{ - TagName: "v1.2.3", - Target: "", - Name: "", - Body: "", - BodyProvided: true, - Draft: false, - Prerelease: false, - RepoOverride: "", - Concurrency: 5, - Assets: []*shared.AssetForUpload(nil), - GenerateNotes: false, - NotesFromTag: true, + TagName: "v1.2.3", + BodyProvided: true, + Concurrency: 5, + Assets: []*shared.AssetForUpload(nil), + NotesFromTag: true, }, }, { - name: "--notes-from-tag only ", - args: "--notes-from-tag", - isTTY: false, - wantErr: "tag required when not running interactively", - }, - { - name: "tag and --notes-from-tag and --generate-notes", + name: "with --notes-from-tag and --generate-notes", args: "v1.2.3 --notes-from-tag --generate-notes", isTTY: false, - wantErr: "using `--notes-from-tag` with `--generate-notes` or `notes-start-tag` is not supported", + wantErr: "using `--notes-from-tag` with `--generate-notes` or `--notes-start-tag` is not supported", }, { - name: "tag and --notes-from-tag and --notes-start-tag", + name: "with --notes-from-tag and --notes-start-tag", args: "v1.2.3 --notes-from-tag --notes-start-tag v1.2.3", isTTY: false, - wantErr: "using `--notes-from-tag` with `--generate-notes` or `notes-start-tag` is not supported", + wantErr: "using `--notes-from-tag` with `--generate-notes` or `--notes-start-tag` is not supported", }, } for _, tt := range tests { @@ -412,6 +399,7 @@ func Test_NewCmdCreate(t *testing.T) { assert.Equal(t, tt.want.NotesStartTag, opts.NotesStartTag) assert.Equal(t, tt.want.IsLatest, opts.IsLatest) assert.Equal(t, tt.want.VerifyTag, opts.VerifyTag) + assert.Equal(t, tt.want.NotesFromTag, opts.NotesFromTag) require.Equal(t, len(tt.want.Assets), len(opts.Assets)) for i := range tt.want.Assets { @@ -428,6 +416,7 @@ func Test_createRun(t *testing.T) { isTTY bool opts CreateOptions httpStubs func(t *testing.T, reg *httpmock.Registry) + runStubs func(rs *run.CommandStubber) wantErr string wantStdout string wantStderr string @@ -442,6 +431,9 @@ func Test_createRun(t *testing.T) { BodyProvided: true, Target: "", }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -471,6 +463,9 @@ func Test_createRun(t *testing.T) { Target: "", DiscussionCategory: "General", }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -500,6 +495,9 @@ func Test_createRun(t *testing.T) { BodyProvided: true, Target: "main", }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -528,6 +526,9 @@ func Test_createRun(t *testing.T) { Draft: true, Target: "", }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -556,6 +557,9 @@ func Test_createRun(t *testing.T) { BodyProvided: true, GenerateNotes: false, }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -584,6 +588,9 @@ func Test_createRun(t *testing.T) { BodyProvided: true, GenerateNotes: true, }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -613,6 +620,9 @@ func Test_createRun(t *testing.T) { GenerateNotes: true, NotesStartTag: "v1.1.0", }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases/generate-notes"), httpmock.RESTPayload(200, `{ @@ -653,6 +663,9 @@ func Test_createRun(t *testing.T) { GenerateNotes: true, NotesStartTag: "v1.1.0", }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases/generate-notes"), httpmock.RESTPayload(200, `{ @@ -701,6 +714,9 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(404, ``)) reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ @@ -758,6 +774,9 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(200, ``)) }, @@ -785,6 +804,9 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(404, ``)) reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.StatusStringResponse(201, `{ @@ -819,6 +841,9 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(404, ``)) reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.StatusStringResponse(201, `{ @@ -854,6 +879,9 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(200, ``)) }, @@ -882,6 +910,9 @@ func Test_createRun(t *testing.T) { DiscussionCategory: "general", Concurrency: 1, }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") + }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(404, ``)) reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ @@ -922,87 +953,92 @@ func Test_createRun(t *testing.T) { wantStderr: ``, }, { - name: "with tag and --notes-from-tag", + name: "with generate notes from tag", isTTY: false, opts: CreateOptions{ - TagName: "v1.2.3", - Target: "", - Name: "", - Body: "", - BodyProvided: true, - Draft: false, - Prerelease: false, - RepoOverride: "", - Concurrency: 5, - Assets: []*shared.AssetForUpload(nil), - GenerateNotes: false, - NotesFromTag: true, + TagName: "v1.2.3", + BodyProvided: true, + Concurrency: 5, + Assets: []*shared.AssetForUpload(nil), + NotesFromTag: true, + }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "some tag message") }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - reg.Register(httpmock.GraphQL("RepositoryFindRef"), - httpmock.StringResponse(`{"data":{"repository":{"ref": {"id": "tag id"}}}}`)) - reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.StatusStringResponse(201, `{ + reg.Register( + httpmock.GraphQL("RepositoryFindRef"), + httpmock.StringResponse(`{"data":{"repository":{"ref": {"id": "tag id"}}}}`), + ) + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/releases"), + httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", "upload_url": "https://api.github.com/assets/upload", "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" - }`)) + }`, func(payload map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "draft": false, + "prerelease": false, + "body": "some tag message", + }, payload) + })) }, wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", wantStderr: "", }, { - name: "with tag and --notes-from-tag and --notes", + name: "with generate notes from tag and notes provided", isTTY: false, opts: CreateOptions{ - TagName: "v1.2.3", - Target: "", - Name: "", - Body: "Notes from --notes here", - BodyProvided: true, - Draft: false, - Prerelease: false, - RepoOverride: "", - Concurrency: 5, - Assets: []*shared.AssetForUpload(nil), - GenerateNotes: false, - NotesFromTag: true, + TagName: "v1.2.3", + Body: "some notes here", + BodyProvided: true, + Concurrency: 5, + Assets: []*shared.AssetForUpload(nil), + NotesFromTag: true, + }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "some tag message") }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - reg.Register(httpmock.GraphQL("RepositoryFindRef"), - httpmock.StringResponse(`{"data":{"repository":{"ref": {"id": "tag id"}}}}`)) - reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.StatusStringResponse(201, `{ - "url": "https://api.github.com/releases/123", - "upload_url": "https://api.github.com/assets/upload", - "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" - }`)) + reg.Register( + httpmock.GraphQL("RepositoryFindRef"), + httpmock.StringResponse(`{"data":{"repository":{"ref": {"id": "tag id"}}}}`), + ) + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/releases"), + httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(payload map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "draft": false, + "prerelease": false, + "body": "some notes here\nsome tag message", + }, payload) + })) }, wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", wantStderr: "", }, { - name: "tag and --notes-from-tag but tag does not exist in repo", + name: "with generate notes from tag and tag does not exist", isTTY: false, opts: CreateOptions{ - TagName: "v1.2.4", - Target: "", - Name: "", - Body: "", - BodyProvided: true, - Draft: false, - Prerelease: false, - RepoOverride: "", - Concurrency: 5, - Assets: []*shared.AssetForUpload(nil), - GenerateNotes: false, - NotesFromTag: true, + TagName: "v1.2.3", + BodyProvided: true, + Concurrency: 5, + Assets: []*shared.AssetForUpload(nil), + NotesFromTag: true, }, - httpStubs: func(t *testing.T, reg *httpmock.Registry) { - reg.Register(httpmock.GraphQL("RepositoryFindRef"), - httpmock.StringResponse(`{"data":{"repository":{"ref": {"id": ""}}}}`)) + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "") }, - wantStdout: "", - wantStderr: "", - wantErr: "tag v1.2.4 doesn't exist in the repo OWNER/REPO, cannot populate release notes with annotated git tag message using the `--notes-from-tag` flag", + wantErr: "cannot generate release notes from tag v1.2.3 as it does not exist locally", }, } for _, tt := range tests { @@ -1028,6 +1064,12 @@ func Test_createRun(t *testing.T) { tt.opts.GitClient = &git.Client{GitPath: "some/path/git"} + rs, teardown := run.Stub() + defer teardown(t) + if tt.runStubs != nil { + tt.runStubs(rs) + } + err := createRun(&tt.opts) if tt.wantErr != "" { require.EqualError(t, err, tt.wantErr) @@ -1571,34 +1613,34 @@ func Test_createRun_interactive(t *testing.T) { }, } for _, tt := range tests { - ios, _, stdout, stderr := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStdinTTY(true) - ios.SetStderrTTY(true) - tt.opts.IO = ios - - reg := &httpmock.Registry{} - defer reg.Verify(t) - tt.httpStubs(reg) - tt.opts.HttpClient = func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - } - - tt.opts.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.FromFullName("OWNER/REPO") - } - - tt.opts.Config = func() (config.Config, error) { - return config.NewBlankConfig(), nil - } - - tt.opts.Edit = func(_, _, val string, _ io.Reader, _, _ io.Writer) (string, error) { - return val, nil - } - - tt.opts.GitClient = &git.Client{GitPath: "some/path/git"} - t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + tt.opts.IO = ios + + reg := &httpmock.Registry{} + defer reg.Verify(t) + tt.httpStubs(reg) + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("OWNER/REPO") + } + + tt.opts.Config = func() (config.Config, error) { + return config.NewBlankConfig(), nil + } + + tt.opts.Edit = func(_, _, val string, _ io.Reader, _, _ io.Writer) (string, error) { + return val, nil + } + + tt.opts.GitClient = &git.Client{GitPath: "some/path/git"} + pm := prompter.NewMockPrompter(t) if tt.prompterStubs != nil { tt.prompterStubs(t, pm) diff --git a/pkg/httpmock/stub.go b/pkg/httpmock/stub.go index 9332ac9dd..147d36fc8 100644 --- a/pkg/httpmock/stub.go +++ b/pkg/httpmock/stub.go @@ -181,6 +181,7 @@ func RESTPayload(responseStatus int, responseBody string, cb func(payload map[st return httpResponse(responseStatus, req, bytes.NewBufferString(responseBody)), nil } } + func GraphQLMutation(body string, cb func(map[string]interface{})) Responder { return func(req *http.Request) (*http.Response, error) { var bodyData struct {