From 908513f97f5e311ae71a2dc69d9778513e3afbeb Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Fri, 31 Jan 2025 16:42:50 -0500 Subject: [PATCH 01/14] Initial working implementation --- pkg/cmd/repo/autolink/autolink.go | 3 + pkg/cmd/repo/autolink/delete/delete.go | 101 +++++++++++++++++++++++++ pkg/cmd/repo/autolink/delete/http.go | 37 +++++++++ 3 files changed, 141 insertions(+) create mode 100644 pkg/cmd/repo/autolink/delete/delete.go create mode 100644 pkg/cmd/repo/autolink/delete/http.go diff --git a/pkg/cmd/repo/autolink/autolink.go b/pkg/cmd/repo/autolink/autolink.go index 09b766b85..e6c43748b 100644 --- a/pkg/cmd/repo/autolink/autolink.go +++ b/pkg/cmd/repo/autolink/autolink.go @@ -3,8 +3,10 @@ package autolink import ( "github.com/MakeNowJust/heredoc" cmdCreate "github.com/cli/cli/v2/pkg/cmd/repo/autolink/create" + cmdDelete "github.com/cli/cli/v2/pkg/cmd/repo/autolink/delete" cmdList "github.com/cli/cli/v2/pkg/cmd/repo/autolink/list" cmdView "github.com/cli/cli/v2/pkg/cmd/repo/autolink/view" + "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" ) @@ -26,6 +28,7 @@ func NewCmdAutolink(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(cmdList.NewCmdList(f, nil)) cmd.AddCommand(cmdCreate.NewCmdCreate(f, nil)) cmd.AddCommand(cmdView.NewCmdView(f, nil)) + cmd.AddCommand(cmdDelete.NewCmdDelete(f, nil)) return cmd } diff --git a/pkg/cmd/repo/autolink/delete/delete.go b/pkg/cmd/repo/autolink/delete/delete.go new file mode 100644 index 000000000..9cdaaf669 --- /dev/null +++ b/pkg/cmd/repo/autolink/delete/delete.go @@ -0,0 +1,101 @@ +package delete + +import ( + "fmt" + + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/pkg/cmd/repo/autolink/view" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +type deleteOptions struct { + BaseRepo func() (ghrepo.Interface, error) + Browser browser.Browser + AutolinkDeleteClient AutolinkDeleteClient + AutolinkViewClient view.AutolinkViewClient + IO *iostreams.IOStreams + + ID string + Confirmed bool + Prompter prompter.Prompter +} + +type AutolinkDeleteClient interface { + Delete(repo ghrepo.Interface, id string) error +} + +func NewCmdDelete(f *cmdutil.Factory, runF func(*deleteOptions) error) *cobra.Command { + opts := &deleteOptions{ + Browser: f.Browser, + IO: f.IOStreams, + Prompter: f.Prompter, + } + + cmd := &cobra.Command{ + Use: "delete ", + Short: "Delete an autolink reference", + Long: "Delete an autolink reference for a repository.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + opts.BaseRepo = f.BaseRepo + + httpClient, err := f.HttpClient() + if err != nil { + return err + } + + opts.AutolinkDeleteClient = &AutolinkDeleter{HTTPClient: httpClient} + opts.AutolinkViewClient = &view.AutolinkViewer{HTTPClient: httpClient} + opts.ID = args[0] + + opts.Confirmed = cmd.Flags().Changed("yes") + + if runF != nil { + return runF(opts) + } + + return deleteRun(opts) + }, + } + + cmd.Flags().BoolVar(&opts.Confirmed, "yes", false, "Confirm deletion without prompting") + + return cmd +} + +func deleteRun(opts *deleteOptions) error { + repo, err := opts.BaseRepo() + if err != nil { + return err + } + + out := opts.IO.Out + cs := opts.IO.ColorScheme() + + autolink, err := opts.AutolinkViewClient.View(repo, opts.ID) + + if err != nil { + return fmt.Errorf("%s %w", cs.Red("error deleting autolink:"), err) + } + + if !opts.Confirmed { + fmt.Fprintf(out, "Autolink %s has key prefix %s.", cs.Cyan(opts.ID), autolink.KeyPrefix) + + if err := opts.Prompter.ConfirmDeletion(autolink.KeyPrefix); err != nil { + return err + } + } + + err = opts.AutolinkDeleteClient.Delete(repo, opts.ID) + if err != nil { + return err + } + + fmt.Fprintf(out, "%s Autolink %s deleted from %s\n", cs.SuccessIcon(), cs.Cyan(opts.ID), cs.Bold(ghrepo.FullName(repo))) + + return nil +} diff --git a/pkg/cmd/repo/autolink/delete/http.go b/pkg/cmd/repo/autolink/delete/http.go new file mode 100644 index 000000000..d6bc53e84 --- /dev/null +++ b/pkg/cmd/repo/autolink/delete/http.go @@ -0,0 +1,37 @@ +package delete + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghinstance" + "github.com/cli/cli/v2/internal/ghrepo" +) + +type AutolinkDeleter struct { + HTTPClient *http.Client +} + +func (a *AutolinkDeleter) Delete(repo ghrepo.Interface, id string) error { + path := fmt.Sprintf("repos/%s/%s/autolinks/%s", repo.RepoOwner(), repo.RepoName(), id) + url := ghinstance.RESTPrefix(repo.RepoHost()) + path + req, err := http.NewRequest(http.MethodDelete, url, nil) + if err != nil { + return err + } + + resp, err := a.HTTPClient.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusNotFound { + return fmt.Errorf("error deleting autolink: HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/%s)", path) + } else if resp.StatusCode > 299 { + return api.HandleHTTPError(resp) + } + + return nil +} From e226a79dc56f95dd9a5c7d2cb0c0dbf15a11d302 Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Sat, 1 Feb 2025 15:46:09 -0500 Subject: [PATCH 02/14] Autolink delete tests --- go.mod | 1 + go.sum | 6 + pkg/cmd/repo/autolink/delete/delete_test.go | 257 ++++++++++++++++++++ pkg/cmd/repo/autolink/delete/http_test.go | 1 + pkg/cmd/repo/autolink/view/view_test.go | 16 +- 5 files changed, 273 insertions(+), 8 deletions(-) create mode 100644 pkg/cmd/repo/autolink/delete/delete_test.go create mode 100644 pkg/cmd/repo/autolink/delete/http_test.go diff --git a/go.mod b/go.mod index 38837b883..9dd32099f 100644 --- a/go.mod +++ b/go.mod @@ -54,6 +54,7 @@ require ( google.golang.org/protobuf v1.36.4 gopkg.in/h2non/gock.v1 v1.1.2 gopkg.in/yaml.v3 v3.0.1 + gotest.tools/v3 v3.0.3 ) require ( diff --git a/go.sum b/go.sum index 883ac72ce..4e7b719c8 100644 --- a/go.sum +++ b/go.sum @@ -206,6 +206,7 @@ github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM= github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/google/certificate-transparency-go v1.3.1 h1:akbcTfQg0iZlANZLn0L9xOeWtyCIdeoYhKrqi5iH3Go= github.com/google/certificate-transparency-go v1.3.1/go.mod h1:gg+UQlx6caKEDQ9EElFOujyxEQEfOiQzAt6782Bvi8k= +github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-containerregistry v0.20.3 h1:oNx7IdTI936V8CQRveCjaxOiegWwvM7kqkbXTpyiovI= @@ -360,6 +361,7 @@ github.com/pelletier/go-toml/v2 v2.2.2 h1:aYUidT7k73Pcl9nb2gScu7NSrKCSHIDE89b3+6 github.com/pelletier/go-toml/v2 v2.2.2/go.mod h1:1t835xjRzz80PqgE6HHgN2JOsmgYu/h4qDAS4n929Rs= github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU= +github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -433,6 +435,7 @@ github.com/spf13/cast v1.7.0 h1:ntdiHjuueXFgm5nzDRdOS4yfT43P5Fnud6DH50rz/7w= github.com/spf13/cast v1.7.0/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo= github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= +github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.19.0 h1:RWq5SEjt8o25SROyN3z2OrDB9l7RPd3lwTWU8EcEdcI= @@ -508,6 +511,7 @@ golang.org/x/exp v0.0.0-20240325151524-a685a6edb6d8/go.mod h1:CQ1k9gNrJ50XIzaKCR golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4= golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= +golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= @@ -544,11 +548,13 @@ golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= golang.org/x/time v0.9.0 h1:EsRrnYcQiGH+5FfbgvV4AP7qEZstoyrHB0DzarOQ4ZY= golang.org/x/time v0.9.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20190624222133-a101b041ded4/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.29.0 h1:Xx0h3TtM9rzQpQuR4dKLrdglAmCEN5Oi+P74JdhdzXE= golang.org/x/tools v0.29.0/go.mod h1:KMQVMRsVxU6nHCFXrBPhDB8XncLNLM0lIy/F14RP588= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.216.0 h1:xnEHy+xWFrtYInWPy8OdGFsyIfWJjtVnO39g7pz2BFY= google.golang.org/api v0.216.0/go.mod h1:K9wzQMvWi47Z9IU7OgdOofvZuw75Ge3PPITImZR/UyI= google.golang.org/genproto v0.0.0-20241118233622-e639e219e697 h1:ToEetK57OidYuqD4Q5w+vfEnPvPpuTwedCNVohYJfNk= diff --git a/pkg/cmd/repo/autolink/delete/delete_test.go b/pkg/cmd/repo/autolink/delete/delete_test.go new file mode 100644 index 000000000..99233fb1e --- /dev/null +++ b/pkg/cmd/repo/autolink/delete/delete_test.go @@ -0,0 +1,257 @@ +package delete + +import ( + "bytes" + "errors" + "net/http" + "testing" + + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared" + "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" +) + +func TestNewCmdDelete(t *testing.T) { + tests := []struct { + name string + input string + output deleteOptions + wantErr bool + errMsg string + }{ + { + name: "no argument", + input: "", + wantErr: true, + errMsg: "accepts 1 arg(s), received 0", + }, + { + name: "id provided", + input: "123", + output: deleteOptions{ID: "123"}, + }, + { + name: "yes flag", + input: "123 --yes", + output: deleteOptions{ID: "123", Confirmed: true}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + f := &cmdutil.Factory{ + IOStreams: ios, + } + f.HttpClient = func() (*http.Client, error) { + return &http.Client{}, nil + } + + argv, err := shlex.Split(tt.input) + require.NoError(t, err) + + var gotOpts *deleteOptions + cmd := NewCmdDelete(f, func(opts *deleteOptions) error { + gotOpts = opts + return nil + }) + + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + if tt.wantErr { + require.EqualError(t, err, tt.errMsg) + } else { + require.NoError(t, err) + assert.Equal(t, tt.output.ID, gotOpts.ID) + assert.Equal(t, tt.output.Confirmed, gotOpts.Confirmed) + } + }) + } +} + +type stubAutolinkDeleter struct { + err error +} + +func (d *stubAutolinkDeleter) Delete(repo ghrepo.Interface, id string) error { + return d.err +} + +type stubAutolinkViewer struct { + autolink *shared.Autolink + err error +} + +func (g stubAutolinkViewer) View(repo ghrepo.Interface, id string) (*shared.Autolink, error) { + return g.autolink, g.err +} + +var errTestPrompt = errors.New("prompt error") +var errTestAutolinkClientView = errors.New("autolink client view error") +var errTestAutolinkClientDelete = errors.New("autolink client delete error") + +func TestDeleteRun(t *testing.T) { + tests := []struct { + name string + opts *deleteOptions + stubDeleter stubAutolinkDeleter + stubViewer stubAutolinkViewer + prompterStubs func(*prompter.PrompterMock) + + wantStdout string + wantStderr string + expectedErr error + expectedErrMsg string + }{ + { + name: "delete", + opts: &deleteOptions{ + ID: "123", + }, + stubViewer: stubAutolinkViewer{ + autolink: &shared.Autolink{ + ID: 123, + KeyPrefix: "TICKET-", + URLTemplate: "https://example.com/TICKET?query=", + IsAlphanumeric: true, + }, + }, + stubDeleter: stubAutolinkDeleter{ + err: nil, + }, + prompterStubs: func(pm *prompter.PrompterMock) { + pm.ConfirmDeletionFunc = func(_ string) error { + return nil + } + }, + wantStdout: "Autolink 123 has key prefix TICKET-.✓ Autolink 123 deleted from OWNER/REPO\n", + }, + { + name: "delete with confirm flag", + opts: &deleteOptions{ + ID: "123", + Confirmed: true, + }, + stubViewer: stubAutolinkViewer{ + autolink: &shared.Autolink{ + ID: 123, + KeyPrefix: "TICKET-", + URLTemplate: "https://example.com/TICKET?query=", + IsAlphanumeric: true, + }, + }, + stubDeleter: stubAutolinkDeleter{ + err: nil, + }, + wantStdout: "✓ Autolink 123 deleted from OWNER/REPO\n", + }, + { + name: "confirmation fails", + opts: &deleteOptions{ + ID: "123", + }, + stubViewer: stubAutolinkViewer{ + autolink: &shared.Autolink{ + ID: 123, + KeyPrefix: "TICKET-", + URLTemplate: "https://example.com/TICKET?query=", + IsAlphanumeric: true, + }, + }, + stubDeleter: stubAutolinkDeleter{ + err: nil, + }, + prompterStubs: func(pm *prompter.PrompterMock) { + pm.ConfirmDeletionFunc = func(_ string) error { + return errTestPrompt + } + }, + expectedErr: errTestPrompt, + expectedErrMsg: errTestPrompt.Error(), + wantStdout: "Autolink 123 has key prefix TICKET-.", + }, + { + name: "view error", + opts: &deleteOptions{ + ID: "123", + }, + stubViewer: stubAutolinkViewer{ + err: errTestAutolinkClientView, + }, + stubDeleter: stubAutolinkDeleter{ + err: nil, + }, + expectedErr: errTestAutolinkClientView, + expectedErrMsg: "error deleting autolink: autolink client view error", + }, + { + name: "delete error", + opts: &deleteOptions{ + ID: "123", + }, + stubViewer: stubAutolinkViewer{ + autolink: &shared.Autolink{ + ID: 123, + KeyPrefix: "TICKET-", + URLTemplate: "https://example.com/TICKET?query=", + IsAlphanumeric: true, + }, + }, + stubDeleter: stubAutolinkDeleter{ + err: errTestAutolinkClientDelete, + }, + prompterStubs: func(pm *prompter.PrompterMock) { + pm.ConfirmDeletionFunc = func(_ string) error { + return nil + } + }, + expectedErr: errTestAutolinkClientDelete, + expectedErrMsg: errTestAutolinkClientDelete.Error(), + wantStdout: "Autolink 123 has key prefix TICKET-.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + + opts := tt.opts + opts.IO = ios + opts.Browser = &browser.Stub{} + + opts.IO = ios + opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil } + + opts.AutolinkDeleteClient = &tt.stubDeleter + opts.AutolinkViewClient = &tt.stubViewer + + pm := &prompter.PrompterMock{} + if tt.prompterStubs != nil { + tt.prompterStubs(pm) + } + tt.opts.Prompter = pm + + err := deleteRun(opts) + + if tt.expectedErr != nil { + require.Error(t, err, "expected error but got none") + assert.ErrorIs(t, err, tt.expectedErr, "unexpected error") + assert.Equal(t, tt.expectedErrMsg, err.Error(), "unexpected error message") + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.wantStdout, stdout.String(), "unexpected stdout") + assert.Equal(t, tt.wantStderr, stderr.String(), "unexpected stderr") + }) + } +} diff --git a/pkg/cmd/repo/autolink/delete/http_test.go b/pkg/cmd/repo/autolink/delete/http_test.go new file mode 100644 index 000000000..a67e08acc --- /dev/null +++ b/pkg/cmd/repo/autolink/delete/http_test.go @@ -0,0 +1 @@ +package delete diff --git a/pkg/cmd/repo/autolink/view/view_test.go b/pkg/cmd/repo/autolink/view/view_test.go index bc0a1bf7f..4192822ea 100644 --- a/pkg/cmd/repo/autolink/view/view_test.go +++ b/pkg/cmd/repo/autolink/view/view_test.go @@ -49,13 +49,12 @@ func TestNewCmdView(t *testing.T) { { name: "json flag", input: "123 --json id", - output: viewOptions{}, + output: viewOptions{ID: "123"}, wantExporter: true, }, { name: "invalid json flag", input: "123 --json invalid", - output: viewOptions{}, wantErr: true, errMsg: "Unknown JSON field: \"invalid\"\nAvailable fields:\n id\n isAlphanumeric\n keyPrefix\n urlTemplate", }, @@ -91,17 +90,18 @@ func TestNewCmdView(t *testing.T) { } else { require.NoError(t, err) assert.Equal(t, tt.wantExporter, gotOpts.Exporter != nil) + assert.Equal(t, tt.output.ID, gotOpts.ID) } }) } } -type stubAutoLinkViewer struct { +type stubAutolinkViewer struct { autolink *shared.Autolink err error } -func (g stubAutoLinkViewer) View(repo ghrepo.Interface, id string) (*shared.Autolink, error) { +func (g stubAutolinkViewer) View(repo ghrepo.Interface, id string) (*shared.Autolink, error) { return g.autolink, g.err } @@ -115,7 +115,7 @@ func TestViewRun(t *testing.T) { tests := []struct { name string opts *viewOptions - stubViewer stubAutoLinkViewer + stubViewer stubAutolinkViewer expectedErr error wantStdout string }{ @@ -124,7 +124,7 @@ func TestViewRun(t *testing.T) { opts: &viewOptions{ ID: "1", }, - stubViewer: stubAutoLinkViewer{ + stubViewer: stubAutolinkViewer{ autolink: &shared.Autolink{ ID: 1, KeyPrefix: "TICKET-", @@ -150,7 +150,7 @@ func TestViewRun(t *testing.T) { return exporter }(), }, - stubViewer: stubAutoLinkViewer{ + stubViewer: stubAutolinkViewer{ autolink: &shared.Autolink{ ID: 1, KeyPrefix: "TICKET-", @@ -163,7 +163,7 @@ func TestViewRun(t *testing.T) { { name: "client error", opts: &viewOptions{}, - stubViewer: stubAutoLinkViewer{ + stubViewer: stubAutolinkViewer{ autolink: nil, err: testAutolinkClientViewError{}, }, From 5fea94ed9e2c9b7f7ee7789339308bc1d42b6b90 Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Sat, 1 Feb 2025 15:48:58 -0500 Subject: [PATCH 03/14] Revert change to deps --- go.mod | 1 - go.sum | 6 ------ 2 files changed, 7 deletions(-) diff --git a/go.mod b/go.mod index 9dd32099f..38837b883 100644 --- a/go.mod +++ b/go.mod @@ -54,7 +54,6 @@ require ( google.golang.org/protobuf v1.36.4 gopkg.in/h2non/gock.v1 v1.1.2 gopkg.in/yaml.v3 v3.0.1 - gotest.tools/v3 v3.0.3 ) require ( diff --git a/go.sum b/go.sum index 4e7b719c8..883ac72ce 100644 --- a/go.sum +++ b/go.sum @@ -206,7 +206,6 @@ github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM= github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/google/certificate-transparency-go v1.3.1 h1:akbcTfQg0iZlANZLn0L9xOeWtyCIdeoYhKrqi5iH3Go= github.com/google/certificate-transparency-go v1.3.1/go.mod h1:gg+UQlx6caKEDQ9EElFOujyxEQEfOiQzAt6782Bvi8k= -github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-containerregistry v0.20.3 h1:oNx7IdTI936V8CQRveCjaxOiegWwvM7kqkbXTpyiovI= @@ -361,7 +360,6 @@ github.com/pelletier/go-toml/v2 v2.2.2 h1:aYUidT7k73Pcl9nb2gScu7NSrKCSHIDE89b3+6 github.com/pelletier/go-toml/v2 v2.2.2/go.mod h1:1t835xjRzz80PqgE6HHgN2JOsmgYu/h4qDAS4n929Rs= github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU= -github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -435,7 +433,6 @@ github.com/spf13/cast v1.7.0 h1:ntdiHjuueXFgm5nzDRdOS4yfT43P5Fnud6DH50rz/7w= github.com/spf13/cast v1.7.0/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo= github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= -github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.19.0 h1:RWq5SEjt8o25SROyN3z2OrDB9l7RPd3lwTWU8EcEdcI= @@ -511,7 +508,6 @@ golang.org/x/exp v0.0.0-20240325151524-a685a6edb6d8/go.mod h1:CQ1k9gNrJ50XIzaKCR golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4= golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= -golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= @@ -548,13 +544,11 @@ golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= golang.org/x/time v0.9.0 h1:EsRrnYcQiGH+5FfbgvV4AP7qEZstoyrHB0DzarOQ4ZY= golang.org/x/time v0.9.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.0.0-20190624222133-a101b041ded4/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.29.0 h1:Xx0h3TtM9rzQpQuR4dKLrdglAmCEN5Oi+P74JdhdzXE= golang.org/x/tools v0.29.0/go.mod h1:KMQVMRsVxU6nHCFXrBPhDB8XncLNLM0lIy/F14RP588= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.216.0 h1:xnEHy+xWFrtYInWPy8OdGFsyIfWJjtVnO39g7pz2BFY= google.golang.org/api v0.216.0/go.mod h1:K9wzQMvWi47Z9IU7OgdOofvZuw75Ge3PPITImZR/UyI= google.golang.org/genproto v0.0.0-20241118233622-e639e219e697 h1:ToEetK57OidYuqD4Q5w+vfEnPvPpuTwedCNVohYJfNk= From fc5a6d1c3faed07267dbfa7cf4e5df577fdbbe00 Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Sun, 2 Feb 2025 20:09:49 -0500 Subject: [PATCH 04/14] Autolink delete http tests --- pkg/cmd/repo/autolink/delete/http_test.go | 68 +++++++++++++++++++++++ pkg/cmd/repo/autolink/view/http_test.go | 10 ++-- 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/repo/autolink/delete/http_test.go b/pkg/cmd/repo/autolink/delete/http_test.go index a67e08acc..928e561c7 100644 --- a/pkg/cmd/repo/autolink/delete/http_test.go +++ b/pkg/cmd/repo/autolink/delete/http_test.go @@ -1 +1,69 @@ package delete + +import ( + "fmt" + "net/http" + "testing" + + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/stretchr/testify/require" +) + +func TestAutolinkDeleter_Delete(t *testing.T) { + repo := ghrepo.New("OWNER", "REPO") + + tests := []struct { + name string + id string + stubStatus int + stubRespJSON string + + expectErr bool + expectedErrMsg string + }{ + { + name: "204 successful delete", + id: "123", + stubStatus: http.StatusNoContent, + }, + { + name: "404 repo or autolink not found", + id: "123", + stubStatus: http.StatusNotFound, + stubRespJSON: `{ + "message": "Not Found", + "documentation_url": "https://docs.github.com/rest/repos/autolinks#get-an-autolink-reference-of-a-repository", + "status": "404" + }`, + expectErr: true, + expectedErrMsg: "error deleting autolink: HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/repos/OWNER/REPO/autolinks/123)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + reg.Register( + httpmock.REST( + http.MethodDelete, + fmt.Sprintf("repos/%s/%s/autolinks/%s", repo.RepoOwner(), repo.RepoName(), tt.id), + ), + httpmock.StatusStringResponse(tt.stubStatus, tt.stubRespJSON), + ) + defer reg.Verify(t) + + autolinkDeleter := &AutolinkDeleter{ + HTTPClient: &http.Client{Transport: reg}, + } + + err := autolinkDeleter.Delete(repo, tt.id) + + if tt.expectErr { + require.EqualError(t, err, tt.expectedErrMsg) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/pkg/cmd/repo/autolink/view/http_test.go b/pkg/cmd/repo/autolink/view/http_test.go index 5bfe9369f..c47418404 100644 --- a/pkg/cmd/repo/autolink/view/http_test.go +++ b/pkg/cmd/repo/autolink/view/http_test.go @@ -28,7 +28,7 @@ func TestAutolinkViewer_View(t *testing.T) { { name: "200 successful alphanumeric view", id: "123", - stubStatus: 200, + stubStatus: http.StatusOK, stubRespJSON: `{ "id": 123, "key_prefix": "TICKET-", @@ -45,7 +45,7 @@ func TestAutolinkViewer_View(t *testing.T) { { name: "200 successful numeric view", id: "123", - stubStatus: 200, + stubStatus: http.StatusOK, stubRespJSON: `{ "id": 123, "key_prefix": "TICKET-", @@ -62,7 +62,7 @@ func TestAutolinkViewer_View(t *testing.T) { { name: "404 repo or autolink not found", id: "123", - stubStatus: 404, + stubStatus: http.StatusNotFound, stubRespJSON: `{ "message": "Not Found", "documentation_url": "https://docs.github.com/rest/repos/autolinks#get-an-autolink-reference-of-a-repository", @@ -85,11 +85,11 @@ func TestAutolinkViewer_View(t *testing.T) { ) defer reg.Verify(t) - autolinkCreator := &AutolinkViewer{ + autolinkViewer := &AutolinkViewer{ HTTPClient: &http.Client{Transport: reg}, } - autolink, err := autolinkCreator.View(repo, tt.id) + autolink, err := autolinkViewer.View(repo, tt.id) if tt.expectErr { require.EqualError(t, err, tt.expectedErrMsg) From dcc1efa2bd402c0d68f9b9d6aed9ebea0f782917 Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Sun, 2 Feb 2025 20:22:16 -0500 Subject: [PATCH 05/14] clean up --- pkg/cmd/repo/autolink/create/http.go | 4 ---- pkg/cmd/repo/autolink/delete/delete_test.go | 2 +- pkg/cmd/repo/autolink/view/http.go | 2 +- pkg/cmd/repo/autolink/view/http_test.go | 2 +- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/repo/autolink/create/http.go b/pkg/cmd/repo/autolink/create/http.go index d7ee940b9..5f187319f 100644 --- a/pkg/cmd/repo/autolink/create/http.go +++ b/pkg/cmd/repo/autolink/create/http.go @@ -45,10 +45,6 @@ func (a *AutolinkCreator) Create(repo ghrepo.Interface, request AutolinkCreateRe defer resp.Body.Close() - // if resp.StatusCode != http.StatusCreated { - // return nil, api.HandleHTTPError(resp) - // } - err = handleAutolinkCreateError(resp) if err != nil { diff --git a/pkg/cmd/repo/autolink/delete/delete_test.go b/pkg/cmd/repo/autolink/delete/delete_test.go index 99233fb1e..6dee277f8 100644 --- a/pkg/cmd/repo/autolink/delete/delete_test.go +++ b/pkg/cmd/repo/autolink/delete/delete_test.go @@ -133,7 +133,7 @@ func TestDeleteRun(t *testing.T) { return nil } }, - wantStdout: "Autolink 123 has key prefix TICKET-.✓ Autolink 123 deleted from OWNER/REPO\n", + wantStdout: "Autolink 123 has key prefix TICKET-.✓Autolink 123 deleted from OWNER/REPO\n", }, { name: "delete with confirm flag", diff --git a/pkg/cmd/repo/autolink/view/http.go b/pkg/cmd/repo/autolink/view/http.go index 8dd6dc12d..cc5638613 100644 --- a/pkg/cmd/repo/autolink/view/http.go +++ b/pkg/cmd/repo/autolink/view/http.go @@ -30,7 +30,7 @@ func (a *AutolinkViewer) View(repo ghrepo.Interface, id string) (*shared.Autolin defer resp.Body.Close() if resp.StatusCode == http.StatusNotFound { - return nil, fmt.Errorf("HTTP 404: Either no autolink with this ID exists for this repository or you are missing admin rights to the repository. (https://api.github.com/%s)", path) + return nil, fmt.Errorf("HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/%s)", path) } else if resp.StatusCode > 299 { return nil, api.HandleHTTPError(resp) } diff --git a/pkg/cmd/repo/autolink/view/http_test.go b/pkg/cmd/repo/autolink/view/http_test.go index c47418404..b792c9750 100644 --- a/pkg/cmd/repo/autolink/view/http_test.go +++ b/pkg/cmd/repo/autolink/view/http_test.go @@ -69,7 +69,7 @@ func TestAutolinkViewer_View(t *testing.T) { "status": "404" }`, expectErr: true, - expectedErrMsg: "HTTP 404: Either no autolink with this ID exists for this repository or you are missing admin rights to the repository. (https://api.github.com/repos/OWNER/REPO/autolinks/123)", + expectedErrMsg: "HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/repos/OWNER/REPO/autolinks/123)", }, } From 0274999880ae9a70056e727565344ca21adba550 Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Sun, 2 Feb 2025 20:22:51 -0500 Subject: [PATCH 06/14] whitespace --- pkg/cmd/repo/autolink/autolink.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/cmd/repo/autolink/autolink.go b/pkg/cmd/repo/autolink/autolink.go index e6c43748b..b4f7c6839 100644 --- a/pkg/cmd/repo/autolink/autolink.go +++ b/pkg/cmd/repo/autolink/autolink.go @@ -6,7 +6,6 @@ import ( cmdDelete "github.com/cli/cli/v2/pkg/cmd/repo/autolink/delete" cmdList "github.com/cli/cli/v2/pkg/cmd/repo/autolink/list" cmdView "github.com/cli/cli/v2/pkg/cmd/repo/autolink/view" - "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" ) From 7325944040b16eb71fb89a85f93dc50b2ba68be2 Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Sun, 2 Feb 2025 20:31:12 -0500 Subject: [PATCH 07/14] More consistency --- pkg/cmd/repo/autolink/create/create.go | 2 +- pkg/cmd/repo/autolink/delete/delete_test.go | 2 +- pkg/cmd/repo/autolink/list/list.go | 16 ++++++++-------- pkg/cmd/repo/autolink/view/view.go | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/repo/autolink/create/create.go b/pkg/cmd/repo/autolink/create/create.go index aad71a941..f06ad5cf9 100644 --- a/pkg/cmd/repo/autolink/create/create.go +++ b/pkg/cmd/repo/autolink/create/create.go @@ -112,7 +112,7 @@ func createRun(opts *createOptions) error { "%s Created repository autolink %s on %s\n", cs.SuccessIconWithColor(cs.Green), cs.Cyanf("%d", autolink.ID), - ghrepo.FullName(repo)) + cs.Bold(ghrepo.FullName(repo))) return nil } diff --git a/pkg/cmd/repo/autolink/delete/delete_test.go b/pkg/cmd/repo/autolink/delete/delete_test.go index 6dee277f8..99233fb1e 100644 --- a/pkg/cmd/repo/autolink/delete/delete_test.go +++ b/pkg/cmd/repo/autolink/delete/delete_test.go @@ -133,7 +133,7 @@ func TestDeleteRun(t *testing.T) { return nil } }, - wantStdout: "Autolink 123 has key prefix TICKET-.✓Autolink 123 deleted from OWNER/REPO\n", + wantStdout: "Autolink 123 has key prefix TICKET-.✓ Autolink 123 deleted from OWNER/REPO\n", }, { name: "delete with confirm flag", diff --git a/pkg/cmd/repo/autolink/list/list.go b/pkg/cmd/repo/autolink/list/list.go index 402479c44..7b10d558f 100644 --- a/pkg/cmd/repo/autolink/list/list.go +++ b/pkg/cmd/repo/autolink/list/list.go @@ -89,8 +89,14 @@ func listRun(opts *listOptions) error { return err } + cs := opts.IO.ColorScheme() + if len(autolinks) == 0 { - return cmdutil.NewNoResultsError(fmt.Sprintf("no autolinks found in %s", ghrepo.FullName(repo))) + return cmdutil.NewNoResultsError( + fmt.Sprintf( + "no autolinks found in %s", + cs.Bold(ghrepo.FullName(repo))), + ) } if opts.Exporter != nil { @@ -98,14 +104,12 @@ func listRun(opts *listOptions) error { } if opts.IO.IsStdoutTTY() { - title := listHeader(ghrepo.FullName(repo), len(autolinks)) + title := fmt.Sprintf("Showing %s in %s", text.Pluralize(len(autolinks), "autolink reference"), cs.Bold(ghrepo.FullName(repo))) fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } tp := tableprinter.New(opts.IO, tableprinter.WithHeader("ID", "KEY PREFIX", "URL TEMPLATE", "ALPHANUMERIC")) - cs := opts.IO.ColorScheme() - for _, autolink := range autolinks { tp.AddField(cs.Cyanf("%d", autolink.ID)) tp.AddField(autolink.KeyPrefix) @@ -116,7 +120,3 @@ func listRun(opts *listOptions) error { return tp.Render() } - -func listHeader(repoName string, count int) string { - return fmt.Sprintf("Showing %s in %s", text.Pluralize(count, "autolink reference"), repoName) -} diff --git a/pkg/cmd/repo/autolink/view/view.go b/pkg/cmd/repo/autolink/view/view.go index 0b212aece..f146cd25b 100644 --- a/pkg/cmd/repo/autolink/view/view.go +++ b/pkg/cmd/repo/autolink/view/view.go @@ -78,7 +78,7 @@ func viewRun(opts *viewOptions) error { return opts.Exporter.Write(opts.IO, autolink) } - fmt.Fprintf(out, "Autolink in %s\n\n", ghrepo.FullName(repo)) + fmt.Fprintf(out, "Autolink in %s\n\n", cs.Bold(ghrepo.FullName(repo))) fmt.Fprint(out, cs.Bold("ID: ")) fmt.Fprintln(out, cs.Cyanf("%d", autolink.ID)) From f907ad9f97ac745d09246d85b3f7e09b0f53975c Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Sun, 2 Feb 2025 20:50:37 -0500 Subject: [PATCH 08/14] refac and some spacing in output --- pkg/cmd/repo/autolink/delete/delete.go | 6 ++++-- pkg/cmd/repo/autolink/delete/delete_test.go | 10 +++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/repo/autolink/delete/delete.go b/pkg/cmd/repo/autolink/delete/delete.go index 9cdaaf669..70c479c2f 100644 --- a/pkg/cmd/repo/autolink/delete/delete.go +++ b/pkg/cmd/repo/autolink/delete/delete.go @@ -83,9 +83,11 @@ func deleteRun(opts *deleteOptions) error { } if !opts.Confirmed { - fmt.Fprintf(out, "Autolink %s has key prefix %s.", cs.Cyan(opts.ID), autolink.KeyPrefix) + fmt.Fprintf(out, "Autolink %s has key prefix %s.\n", cs.Cyan(opts.ID), autolink.KeyPrefix) - if err := opts.Prompter.ConfirmDeletion(autolink.KeyPrefix); err != nil { + err := opts.Prompter.ConfirmDeletion(autolink.KeyPrefix) + + if err != nil { return err } } diff --git a/pkg/cmd/repo/autolink/delete/delete_test.go b/pkg/cmd/repo/autolink/delete/delete_test.go index 99233fb1e..73b7e3c77 100644 --- a/pkg/cmd/repo/autolink/delete/delete_test.go +++ b/pkg/cmd/repo/autolink/delete/delete_test.go @@ -6,6 +6,7 @@ import ( "net/http" "testing" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" @@ -133,7 +134,10 @@ func TestDeleteRun(t *testing.T) { return nil } }, - wantStdout: "Autolink 123 has key prefix TICKET-.✓ Autolink 123 deleted from OWNER/REPO\n", + wantStdout: heredoc.Doc(` + Autolink 123 has key prefix TICKET-. + ✓ Autolink 123 deleted from OWNER/REPO + `), }, { name: "delete with confirm flag", @@ -177,7 +181,7 @@ func TestDeleteRun(t *testing.T) { }, expectedErr: errTestPrompt, expectedErrMsg: errTestPrompt.Error(), - wantStdout: "Autolink 123 has key prefix TICKET-.", + wantStdout: "Autolink 123 has key prefix TICKET-.\n", }, { name: "view error", @@ -216,7 +220,7 @@ func TestDeleteRun(t *testing.T) { }, expectedErr: errTestAutolinkClientDelete, expectedErrMsg: errTestAutolinkClientDelete.Error(), - wantStdout: "Autolink 123 has key prefix TICKET-.", + wantStdout: "Autolink 123 has key prefix TICKET-.\n", }, } From 0b3efede35f7f121ce9c0b78a194c151699e44cf Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Sun, 2 Feb 2025 21:12:49 -0500 Subject: [PATCH 09/14] Use http constants --- pkg/cmd/repo/autolink/list/http_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/repo/autolink/list/http_test.go b/pkg/cmd/repo/autolink/list/http_test.go index 65289c419..065444a89 100644 --- a/pkg/cmd/repo/autolink/list/http_test.go +++ b/pkg/cmd/repo/autolink/list/http_test.go @@ -23,7 +23,7 @@ func TestAutolinkLister_List(t *testing.T) { name: "no autolinks", repo: ghrepo.New("OWNER", "REPO"), resp: []shared.Autolink{}, - status: 200, + status: http.StatusOK, }, { name: "two autolinks", @@ -42,12 +42,12 @@ func TestAutolinkLister_List(t *testing.T) { URLTemplate: "https://example2.com", }, }, - status: 200, + status: http.StatusOK, }, { name: "http error", repo: ghrepo.New("OWNER", "REPO"), - status: 404, + status: http.StatusNotFound, }, } @@ -64,7 +64,7 @@ func TestAutolinkLister_List(t *testing.T) { HTTPClient: &http.Client{Transport: reg}, } autolinks, err := autolinkLister.List(tt.repo) - if tt.status == 404 { + if tt.status == http.StatusNotFound { require.Error(t, err) assert.Equal(t, "error getting autolinks: HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/repos/OWNER/REPO/autolinks)", err.Error()) } else { From 021557912d08ce7d168c08694d2bd67b33e9b38e Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Sun, 2 Feb 2025 21:34:03 -0500 Subject: [PATCH 10/14] AuoLink -> Autolink --- pkg/cmd/repo/autolink/create/http_test.go | 2 +- pkg/cmd/repo/autolink/list/list_test.go | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/repo/autolink/create/http_test.go b/pkg/cmd/repo/autolink/create/http_test.go index 9ef5e0da5..f5e2f5365 100644 --- a/pkg/cmd/repo/autolink/create/http_test.go +++ b/pkg/cmd/repo/autolink/create/http_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestAutoLinkCreator_Create(t *testing.T) { +func TestAutolinkCreator_Create(t *testing.T) { repo := ghrepo.New("OWNER", "REPO") tests := []struct { diff --git a/pkg/cmd/repo/autolink/list/list_test.go b/pkg/cmd/repo/autolink/list/list_test.go index 1e4d73ab8..81acfbbdf 100644 --- a/pkg/cmd/repo/autolink/list/list_test.go +++ b/pkg/cmd/repo/autolink/list/list_test.go @@ -96,12 +96,12 @@ func TestNewCmdList(t *testing.T) { } } -type stubAutoLinkLister struct { +type stubAutolinkLister struct { autolinks []shared.Autolink err error } -func (g stubAutoLinkLister) List(repo ghrepo.Interface) ([]shared.Autolink, error) { +func (g stubAutolinkLister) List(repo ghrepo.Interface) ([]shared.Autolink, error) { return g.autolinks, g.err } @@ -116,7 +116,7 @@ func TestListRun(t *testing.T) { name string opts *listOptions isTTY bool - stubLister stubAutoLinkLister + stubLister stubAutolinkLister expectedErr error wantStdout string wantStderr string @@ -125,7 +125,7 @@ func TestListRun(t *testing.T) { name: "list tty", opts: &listOptions{}, isTTY: true, - stubLister: stubAutoLinkLister{ + stubLister: stubAutolinkLister{ autolinks: []shared.Autolink{ { ID: 1, @@ -161,7 +161,7 @@ func TestListRun(t *testing.T) { }(), }, isTTY: true, - stubLister: stubAutoLinkLister{ + stubLister: stubAutolinkLister{ autolinks: []shared.Autolink{ { ID: 1, @@ -184,7 +184,7 @@ func TestListRun(t *testing.T) { name: "list non-tty", opts: &listOptions{}, isTTY: false, - stubLister: stubAutoLinkLister{ + stubLister: stubAutolinkLister{ autolinks: []shared.Autolink{ { ID: 1, @@ -210,7 +210,7 @@ func TestListRun(t *testing.T) { name: "no results", opts: &listOptions{}, isTTY: true, - stubLister: stubAutoLinkLister{ + stubLister: stubAutolinkLister{ autolinks: []shared.Autolink{}, }, expectedErr: cmdutil.NewNoResultsError("no autolinks found in OWNER/REPO"), @@ -220,7 +220,7 @@ func TestListRun(t *testing.T) { name: "client error", opts: &listOptions{}, isTTY: true, - stubLister: stubAutoLinkLister{ + stubLister: stubAutolinkLister{ autolinks: []shared.Autolink{}, err: testAutolinkClientListError{}, }, From a1ae68b4f4794b838e64bf7e860abccdbfa638c5 Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Sun, 2 Feb 2025 21:37:34 -0500 Subject: [PATCH 11/14] Formatting --- pkg/cmd/repo/autolink/list/list.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/repo/autolink/list/list.go b/pkg/cmd/repo/autolink/list/list.go index 7b10d558f..fb726d369 100644 --- a/pkg/cmd/repo/autolink/list/list.go +++ b/pkg/cmd/repo/autolink/list/list.go @@ -104,7 +104,11 @@ func listRun(opts *listOptions) error { } if opts.IO.IsStdoutTTY() { - title := fmt.Sprintf("Showing %s in %s", text.Pluralize(len(autolinks), "autolink reference"), cs.Bold(ghrepo.FullName(repo))) + title := fmt.Sprintf( + "Showing %s in %s", + text.Pluralize(len(autolinks), "autolink reference"), + cs.Bold(ghrepo.FullName(repo)), + ) fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } From d5adafd3fb04eed62c09c0fe507f35c2292540ba Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Sun, 2 Feb 2025 21:46:56 -0500 Subject: [PATCH 12/14] Rename staggler --- pkg/cmd/repo/autolink/create/create_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/repo/autolink/create/create_test.go b/pkg/cmd/repo/autolink/create/create_test.go index 477d28da9..fe357f2ee 100644 --- a/pkg/cmd/repo/autolink/create/create_test.go +++ b/pkg/cmd/repo/autolink/create/create_test.go @@ -92,11 +92,11 @@ func TestNewCmdCreate(t *testing.T) { } } -type stubAutoLinkCreator struct { +type stubAutolinkCreator struct { err error } -func (g stubAutoLinkCreator) Create(repo ghrepo.Interface, request AutolinkCreateRequest) (*shared.Autolink, error) { +func (g stubAutolinkCreator) Create(repo ghrepo.Interface, request AutolinkCreateRequest) (*shared.Autolink, error) { if g.err != nil { return nil, g.err } @@ -119,7 +119,7 @@ func TestCreateRun(t *testing.T) { tests := []struct { name string opts *createOptions - stubCreator stubAutoLinkCreator + stubCreator stubAutolinkCreator expectedErr error errMsg string wantStdout string @@ -131,7 +131,7 @@ func TestCreateRun(t *testing.T) { KeyPrefix: "TICKET-", URLTemplate: "https://example.com/TICKET?query=", }, - stubCreator: stubAutoLinkCreator{}, + stubCreator: stubAutolinkCreator{}, wantStdout: "✓ Created repository autolink 1 on OWNER/REPO\n", }, { @@ -141,7 +141,7 @@ func TestCreateRun(t *testing.T) { URLTemplate: "https://example.com/TICKET?query=", Numeric: true, }, - stubCreator: stubAutoLinkCreator{}, + stubCreator: stubAutolinkCreator{}, wantStdout: "✓ Created repository autolink 1 on OWNER/REPO\n", }, { @@ -150,7 +150,7 @@ func TestCreateRun(t *testing.T) { KeyPrefix: "TICKET-", URLTemplate: "https://example.com/TICKET?query=", }, - stubCreator: stubAutoLinkCreator{err: testAutolinkClientCreateError{}}, + stubCreator: stubAutolinkCreator{err: testAutolinkClientCreateError{}}, expectedErr: testAutolinkClientCreateError{}, errMsg: fmt.Sprint("error creating autolink: ", testAutolinkClientCreateError{}.Error()), }, From 9eaaf4451630e9f8dd056ea95826bbb27e195ced Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Fri, 7 Feb 2025 16:27:04 -0500 Subject: [PATCH 13/14] Handle non-TTY behavior --- pkg/cmd/repo/autolink/delete/delete.go | 10 +- pkg/cmd/repo/autolink/delete/delete_test.go | 100 +++++++++++++++++--- 2 files changed, 94 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/repo/autolink/delete/delete.go b/pkg/cmd/repo/autolink/delete/delete.go index 70c479c2f..e5cd874fb 100644 --- a/pkg/cmd/repo/autolink/delete/delete.go +++ b/pkg/cmd/repo/autolink/delete/delete.go @@ -52,7 +52,9 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*deleteOptions) error) *cobra.Co opts.AutolinkViewClient = &view.AutolinkViewer{HTTPClient: httpClient} opts.ID = args[0] - opts.Confirmed = cmd.Flags().Changed("yes") + if !opts.IO.CanPrompt() && !opts.Confirmed { + return cmdutil.FlagErrorf("--yes required when not running interactively") + } if runF != nil { return runF(opts) @@ -82,7 +84,7 @@ func deleteRun(opts *deleteOptions) error { return fmt.Errorf("%s %w", cs.Red("error deleting autolink:"), err) } - if !opts.Confirmed { + if opts.IO.CanPrompt() && !opts.Confirmed { fmt.Fprintf(out, "Autolink %s has key prefix %s.\n", cs.Cyan(opts.ID), autolink.KeyPrefix) err := opts.Prompter.ConfirmDeletion(autolink.KeyPrefix) @@ -97,7 +99,9 @@ func deleteRun(opts *deleteOptions) error { return err } - fmt.Fprintf(out, "%s Autolink %s deleted from %s\n", cs.SuccessIcon(), cs.Cyan(opts.ID), cs.Bold(ghrepo.FullName(repo))) + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(out, "%s Autolink %s deleted from %s\n", cs.SuccessIcon(), cs.Cyan(opts.ID), cs.Bold(ghrepo.FullName(repo))) + } return nil } diff --git a/pkg/cmd/repo/autolink/delete/delete_test.go b/pkg/cmd/repo/autolink/delete/delete_test.go index 73b7e3c77..7606ebe33 100644 --- a/pkg/cmd/repo/autolink/delete/delete_test.go +++ b/pkg/cmd/repo/autolink/delete/delete_test.go @@ -23,29 +23,51 @@ func TestNewCmdDelete(t *testing.T) { name string input string output deleteOptions + isTTY bool wantErr bool errMsg string }{ { name: "no argument", input: "", + isTTY: true, wantErr: true, errMsg: "accepts 1 arg(s), received 0", }, { name: "id provided", input: "123", + isTTY: true, output: deleteOptions{ID: "123"}, }, { name: "yes flag", input: "123 --yes", + isTTY: true, output: deleteOptions{ID: "123", Confirmed: true}, }, + { + name: "non-TTY", + input: "123 --yes", + isTTY: false, + output: deleteOptions{ID: "123", Confirmed: true}, + }, + { + name: "non-TTY missing yes flag", + input: "123", + isTTY: false, + wantErr: true, + errMsg: "--yes required when not running interactively", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ios, _, _, _ := iostreams.Test() + + ios.SetStdinTTY(tt.isTTY) + ios.SetStdoutTTY(tt.isTTY) + ios.SetStderrTTY(tt.isTTY) + f := &cmdutil.Factory{ IOStreams: ios, } @@ -104,12 +126,12 @@ func TestDeleteRun(t *testing.T) { tests := []struct { name string opts *deleteOptions + isTTY bool stubDeleter stubAutolinkDeleter stubViewer stubAutolinkViewer prompterStubs func(*prompter.PrompterMock) wantStdout string - wantStderr string expectedErr error expectedErrMsg string }{ @@ -118,6 +140,7 @@ func TestDeleteRun(t *testing.T) { opts: &deleteOptions{ ID: "123", }, + isTTY: true, stubViewer: stubAutolinkViewer{ autolink: &shared.Autolink{ ID: 123, @@ -145,6 +168,7 @@ func TestDeleteRun(t *testing.T) { ID: "123", Confirmed: true, }, + isTTY: true, stubViewer: stubAutolinkViewer{ autolink: &shared.Autolink{ ID: 123, @@ -153,16 +177,15 @@ func TestDeleteRun(t *testing.T) { IsAlphanumeric: true, }, }, - stubDeleter: stubAutolinkDeleter{ - err: nil, - }, - wantStdout: "✓ Autolink 123 deleted from OWNER/REPO\n", + stubDeleter: stubAutolinkDeleter{}, + wantStdout: "✓ Autolink 123 deleted from OWNER/REPO\n", }, { name: "confirmation fails", opts: &deleteOptions{ ID: "123", }, + isTTY: true, stubViewer: stubAutolinkViewer{ autolink: &shared.Autolink{ ID: 123, @@ -171,9 +194,7 @@ func TestDeleteRun(t *testing.T) { IsAlphanumeric: true, }, }, - stubDeleter: stubAutolinkDeleter{ - err: nil, - }, + stubDeleter: stubAutolinkDeleter{}, prompterStubs: func(pm *prompter.PrompterMock) { pm.ConfirmDeletionFunc = func(_ string) error { return errTestPrompt @@ -188,12 +209,11 @@ func TestDeleteRun(t *testing.T) { opts: &deleteOptions{ ID: "123", }, + isTTY: true, stubViewer: stubAutolinkViewer{ err: errTestAutolinkClientView, }, - stubDeleter: stubAutolinkDeleter{ - err: nil, - }, + stubDeleter: stubAutolinkDeleter{}, expectedErr: errTestAutolinkClientView, expectedErrMsg: "error deleting autolink: autolink client view error", }, @@ -202,6 +222,7 @@ func TestDeleteRun(t *testing.T) { opts: &deleteOptions{ ID: "123", }, + isTTY: true, stubViewer: stubAutolinkViewer{ autolink: &shared.Autolink{ ID: 123, @@ -222,11 +243,65 @@ func TestDeleteRun(t *testing.T) { expectedErrMsg: errTestAutolinkClientDelete.Error(), wantStdout: "Autolink 123 has key prefix TICKET-.\n", }, + { + name: "no TTY", + opts: &deleteOptions{ + ID: "123", + Confirmed: true, + }, + isTTY: false, + stubViewer: stubAutolinkViewer{ + autolink: &shared.Autolink{ + ID: 123, + KeyPrefix: "TICKET-", + URLTemplate: "https://example.com/TICKET?query=", + IsAlphanumeric: true, + }}, + stubDeleter: stubAutolinkDeleter{}, + wantStdout: "", + }, + { + name: "no TTY view error", + opts: &deleteOptions{ + ID: "123", + }, + isTTY: false, + stubViewer: stubAutolinkViewer{ + err: errTestAutolinkClientView, + }, + stubDeleter: stubAutolinkDeleter{}, + expectedErr: errTestAutolinkClientView, + expectedErrMsg: "error deleting autolink: autolink client view error", + }, + { + name: "no TTY delete error", + opts: &deleteOptions{ + ID: "123", + Confirmed: true, + }, + isTTY: false, + stubViewer: stubAutolinkViewer{ + autolink: &shared.Autolink{ + ID: 123, + KeyPrefix: "TICKET-", + URLTemplate: "https://example.com/TICKET?query=", + IsAlphanumeric: true, + }, + }, + stubDeleter: stubAutolinkDeleter{ + err: errTestAutolinkClientDelete, + }, + expectedErr: errTestAutolinkClientDelete, + expectedErrMsg: errTestAutolinkClientDelete.Error(), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ios, _, stdout, stderr := iostreams.Test() + ios, _, stdout, _ := iostreams.Test() + + ios.SetStdinTTY(tt.isTTY) + ios.SetStdoutTTY(tt.isTTY) opts := tt.opts opts.IO = ios @@ -255,7 +330,6 @@ func TestDeleteRun(t *testing.T) { } assert.Equal(t, tt.wantStdout, stdout.String(), "unexpected stdout") - assert.Equal(t, tt.wantStderr, stderr.String(), "unexpected stderr") }) } } From 8e2be7326b5fe966aeef4697c8f17d6ab3fc42c4 Mon Sep 17 00:00:00 2001 From: Michael Hoffman Date: Fri, 7 Feb 2025 16:51:19 -0500 Subject: [PATCH 14/14] Improve http error test cases --- pkg/cmd/repo/autolink/delete/http_test.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/repo/autolink/delete/http_test.go b/pkg/cmd/repo/autolink/delete/http_test.go index 928e561c7..a2676178d 100644 --- a/pkg/cmd/repo/autolink/delete/http_test.go +++ b/pkg/cmd/repo/autolink/delete/http_test.go @@ -28,17 +28,21 @@ func TestAutolinkDeleter_Delete(t *testing.T) { stubStatus: http.StatusNoContent, }, { - name: "404 repo or autolink not found", - id: "123", - stubStatus: http.StatusNotFound, - stubRespJSON: `{ - "message": "Not Found", - "documentation_url": "https://docs.github.com/rest/repos/autolinks#get-an-autolink-reference-of-a-repository", - "status": "404" - }`, + name: "404 repo or autolink not found", + id: "123", + stubStatus: http.StatusNotFound, + stubRespJSON: `{}`, // API response not used in output expectErr: true, expectedErrMsg: "error deleting autolink: HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/repos/OWNER/REPO/autolinks/123)", }, + { + name: "500 unexpected error", + id: "123", + stubRespJSON: `{"messsage": "arbitrary error"}`, + stubStatus: http.StatusInternalServerError, + expectErr: true, + expectedErrMsg: "HTTP 500 (https://api.github.com/repos/OWNER/REPO/autolinks/123)", + }, } for _, tt := range tests { @@ -49,7 +53,7 @@ func TestAutolinkDeleter_Delete(t *testing.T) { http.MethodDelete, fmt.Sprintf("repos/%s/%s/autolinks/%s", repo.RepoOwner(), repo.RepoName(), tt.id), ), - httpmock.StatusStringResponse(tt.stubStatus, tt.stubRespJSON), + httpmock.StatusJSONResponse(tt.stubStatus, tt.stubRespJSON), ) defer reg.Verify(t)