From cf9b02fdc694566df84130cb7eaaa90c700658ee Mon Sep 17 00:00:00 2001 From: Michael Rykov Date: Thu, 25 Nov 2021 16:35:37 +0800 Subject: [PATCH] =?UTF-8?q?Support=20for=20=E2=80=9Cyank=20PACKAGE@VERSION?= =?UTF-8?q?=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cli/git_test.go | 24 ++++++---- cli/yank.go | 41 +++++++++++------ cli/yank_test.go | 79 +++++++++++++++++++++++++++++++- go.mod | 4 +- go.sum | 7 +-- internal/testutil/api.go | 98 ++++++++++++++++++++++------------------ 6 files changed, 181 insertions(+), 72 deletions(-) diff --git a/cli/git_test.go b/cli/git_test.go index 9efcaaf..84cdfd4 100644 --- a/cli/git_test.go +++ b/cli/git_test.go @@ -105,11 +105,13 @@ func TestGitDestroyCommandSuccess(t *testing.T) { // Destroy without reset path := "/git/repos/me/repo-name" - serverDestroy := testutil.APIServerCustom(t, "DELETE", path, func(w http.ResponseWriter, r *http.Request) { - if r.URL.Query().Has("reset") { - t.Errorf("Has extraneous reset=1 URL query") - } - w.Write([]byte("{}")) + serverDestroy := testutil.APIServerCustom(t, func(mux *http.ServeMux) { + mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Has("reset") { + t.Errorf("Has extraneous reset=1 URL query") + } + w.Write([]byte("{}")) + }) }) defer serverDestroy.Close() @@ -128,11 +130,13 @@ func TestGitDestroyCommandSuccess(t *testing.T) { } // Reset without destroying repo - serverReset := testutil.APIServerCustom(t, "DELETE", path, func(w http.ResponseWriter, r *http.Request) { - if q := r.URL.Query(); q.Get("reset") != "1" { - t.Errorf("Missing reset=1 URL query") - } - w.Write([]byte("{}")) + serverReset := testutil.APIServerCustom(t, func(mux *http.ServeMux) { + mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { + if q := r.URL.Query(); q.Get("reset") != "1" { + t.Errorf("Missing reset=1 URL query") + } + w.Write([]byte("{}")) + }) }) defer serverReset.Close() diff --git a/cli/yank.go b/cli/yank.go index a14d71e..c67796b 100644 --- a/cli/yank.go +++ b/cli/yank.go @@ -2,9 +2,11 @@ package cli import ( "github.com/gemfury/cli/internal/ctx" + "github.com/hashicorp/go-multierror" "github.com/spf13/cobra" "fmt" + "strings" ) // NewCmdYank generates the Cobra command for "yank" @@ -12,22 +14,15 @@ func NewCmdYank() *cobra.Command { var versionFlag string yankCmd := &cobra.Command{ - Use: "yank PACKAGE VERSION", + Use: "yank PACKAGE@VERSION", Short: "Remove a package version", RunE: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { return fmt.Errorf("Please specify at least one package") } - var pkg string = args[0] - var ver string = "" - - if versionFlag != "" { - ver = versionFlag - } else if len(args) > 1 { - ver = args[1] - } else { - return fmt.Errorf("No version specified") + if versionFlag != "" && len(args) > 1 { + return fmt.Errorf("Use PACKAGE@VERSION for multiple yanks") } cc := cmd.Context() @@ -37,12 +32,32 @@ func NewCmdYank() *cobra.Command { return err } - err = c.Yank(cc, pkg, ver) - if err == nil { + var multiErr *multierror.Error + for _, pkg := range args { + var ver string = "" + + if versionFlag != "" { + ver = versionFlag + } else if at := strings.LastIndex(pkg, "@"); at > 0 { + pkg, ver = pkg[0:at], pkg[at+1:] + } + + if pkg == "" || ver == "" { + err := fmt.Errorf("Invalid package/version specified") + multiErr = multierror.Append(multiErr, err) + continue + } + + err = c.Yank(cc, pkg, ver) + if err != nil { + multiErr = multierror.Append(multiErr, err) + continue + } + term.Printf("Removed package %q version %q\n", pkg, ver) } - return err + return multiErr.Unwrap() }, } diff --git a/cli/yank_test.go b/cli/yank_test.go index 9805661..c91bb4f 100644 --- a/cli/yank_test.go +++ b/cli/yank_test.go @@ -5,13 +5,15 @@ import ( "github.com/gemfury/cli/internal/ctx" "github.com/gemfury/cli/internal/testutil" "github.com/gemfury/cli/pkg/terminal" + + "net/http" "strings" "testing" ) // ==== YANK ==== -func TestYankCommandSuccess(t *testing.T) { +func TestYankCommandOnePackage(t *testing.T) { auth := terminal.TestAuther("user", "abc123", nil) term := terminal.NewForTest() @@ -24,6 +26,7 @@ func TestYankCommandSuccess(t *testing.T) { flags := ctx.GlobalFlags(cc) flags.Endpoint = server.URL + // Removing using version flag err := runCommandNoErr(cc, []string{"yank", "foo", "-v", "0.0.1"}) if err != nil { t.Fatal(err) @@ -33,6 +36,80 @@ func TestYankCommandSuccess(t *testing.T) { if outStr := string(term.OutBytes()); !strings.HasSuffix(outStr, exp) { t.Errorf("Expected output to include %q, got %q", exp, outStr) } + + // Removing using PACKAGE@VERSION + err = runCommandNoErr(cc, []string{"yank", "foo@0.0.1"}) + if err != nil { + t.Fatal(err) + } + + exp = "Removed package \"foo\" version \"0.0.1\"\n" + if outStr := string(term.OutBytes()); !strings.HasSuffix(outStr, exp) { + t.Errorf("Expected output to include %q, got %q", exp, outStr) + } + + // Fail if no version specified + err = runCommandNoErr(cc, []string{"yank", "foo"}) + if err == nil || !strings.Contains(err.Error(), "Invalid package/version") { + t.Errorf("Expected invalid error, got %q", err) + } +} + +func TestYankCommandMultiPackage(t *testing.T) { + auth := terminal.TestAuther("user", "abc123", nil) + term := terminal.NewForTest() + + // Fire up test server + server := testutil.APIServerCustom(t, func(mux *http.ServeMux) { + mux.HandleFunc("/packages/foo/versions/0.0.1", func(w http.ResponseWriter, r *http.Request) { + if method := r.Method; method != "DELETE" { + t.Errorf("Invalid request: %s %s", method, r.URL.Path) + } + w.Write([]byte("{}")) + }) + mux.HandleFunc("/packages/foo/versions/0.0.2", func(w http.ResponseWriter, r *http.Request) { + if method := r.Method; method != "DELETE" { + t.Errorf("Invalid request: %s %s", method, r.URL.Path) + } + http.NotFound(w, r) + }) + }) + + defer server.Close() + + cc := cli.TestContext(term, auth) + flags := ctx.GlobalFlags(cc) + flags.Endpoint = server.URL + + // Expected successful output + exp := "Removed package \"foo\" version \"0.0.1\"\n" + + // Failure for multiple packages without version + err := runCommandNoErr(cc, []string{"yank", "foo", "bar"}) + if err == nil || !strings.Contains(err.Error(), "Invalid package/version") { + t.Errorf("Expected invalid error, got %q", err) + } + + // Failure for multiple packages with version flag + err = runCommandNoErr(cc, []string{"yank", "foo", "bar", "-v", "0.0.1"}) + if err == nil || !strings.Contains(err.Error(), "Use PACKAGE@VERSION") { + t.Errorf("Expected invalid error, got %q", err) + } + + // Partial failure for multiple packages + err = runCommandNoErr(cc, []string{"yank", "foo@0.0.1", "foo@0.0.2"}) + if err == nil || !strings.Contains(err.Error(), "Doesn't look like this exists") { + t.Errorf("Expected invalid error, got %q", err) + } + if outStr := string(term.OutBytes()); !strings.Contains(outStr, exp) { + t.Errorf("Expected output to include %q, got %q", exp, outStr) + } + + // Success all around (reusing the same test package URL) + err = runCommandNoErr(cc, []string{"yank", "foo@0.0.1", "foo@0.0.1"}) + if outStr := string(term.OutBytes()); !strings.HasSuffix(outStr, exp) { + t.Errorf("Expected output to include %q, got %q", exp, outStr) + } } func TestYankCommandUnauthorized(t *testing.T) { diff --git a/go.mod b/go.mod index 0c26171..ec18217 100644 --- a/go.mod +++ b/go.mod @@ -20,9 +20,9 @@ require ( github.com/fatih/color v1.13.0 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect github.com/inconshreveable/mousetrap v1.0.0 // indirect - github.com/mattn/go-colorable v0.1.11 // indirect + github.com/mattn/go-colorable v0.1.12 // indirect github.com/mattn/go-isatty v0.0.14 // indirect github.com/mattn/go-runewidth v0.0.13 // indirect github.com/rivo/uniseg v0.2.0 // indirect - golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6 // indirect + golang.org/x/sys v0.0.0-20211124211545-fe61309f8881 // indirect ) diff --git a/go.sum b/go.sum index 5ca44d1..c477b9b 100644 --- a/go.sum +++ b/go.sum @@ -196,8 +196,8 @@ github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaO github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.8/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= -github.com/mattn/go-colorable v0.1.11 h1:nQ+aFkoE2TMGc0b68U2OKSexC+eq46+XwZzWXHRmPYs= -github.com/mattn/go-colorable v0.1.11/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= +github.com/mattn/go-colorable v0.1.12 h1:jF+Du6AlPIjs2BiUiQlKOX0rt3SujHxPnksPKZbaA40= +github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= @@ -420,8 +420,9 @@ golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6 h1:foEbQz/B0Oz6YIqu/69kfXPYeFQAuuMYFkjaqXzl5Wo= golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20211124211545-fe61309f8881 h1:TyHqChC80pFkXWraUUf6RuB5IqFdQieMLwwCJokV2pc= +golang.org/x/sys v0.0.0-20211124211545-fe61309f8881/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/internal/testutil/api.go b/internal/testutil/api.go index ef40ae0..774f7e0 100644 --- a/internal/testutil/api.go +++ b/internal/testutil/api.go @@ -21,66 +21,76 @@ const loginResponse = `{ }` func APIServer(t *testing.T, method, path, resp string, code int) *httptest.Server { - return APIServerCustom(t, method, path, func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(code) - w.Write([]byte(resp)) + return APIServerCustom(t, func(mux *http.ServeMux) { + mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { + t.Logf("API Request: %s %s", r.Method, r.URL.String()) + + if r.Method != method { + w.WriteHeader(http.StatusNotImplemented) + return + } + + w.WriteHeader(code) + w.Write([]byte(resp)) + }) }) } // Allow responses to be paginated forward. Page param is just a string of "p" characters to // simplify implementation (without parsing), and prevent parsing page number as an integer func APIServerPaginated(t *testing.T, method, path string, resps []string, code int) *httptest.Server { - return APIServerCustom(t, method, path, func(w http.ResponseWriter, r *http.Request) { + return APIServerCustom(t, func(mux *http.ServeMux) { + mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { + t.Logf("API Request: %s %s", r.Method, r.URL.String()) - // Page from JSON body or query - pageReq := api.PaginationRequest{} - page := len(r.URL.Query().Get("page")) - if err := json.NewDecoder(r.Body).Decode(&pageReq); err == nil && pageReq.Page != "" { - page = len(pageReq.Page) - } + if r.Method != method { + w.WriteHeader(http.StatusNotImplemented) + return + } - // Out of bounds empty response - if page > len(resps) { - w.WriteHeader(code) - w.Write([]byte("[]")) - return - } + // Page from JSON body or query + pageReq := api.PaginationRequest{} + page := len(r.URL.Query().Get("page")) + if err := json.NewDecoder(r.Body).Decode(&pageReq); err == nil && pageReq.Page != "" { + page = len(pageReq.Page) + } - // Populate "Link" header - if page < len(resps)-1 { - newURL := *r.URL // Copy incoming URL - newURL.Scheme, newURL.Host = "", "" + // Out of bounds empty response + if page > len(resps) { + w.WriteHeader(code) + w.Write([]byte("[]")) + return + } - query := newURL.Query() - query.Set("page", strings.Repeat("p", page+1)) - newURL.RawQuery = query.Encode() - linkStr := linkheader.Links{ - {URL: newURL.String(), Rel: "next"}, - }.String() + // Populate "Link" header + if page < len(resps)-1 { + newURL := *r.URL // Copy incoming URL + newURL.Scheme, newURL.Host = "", "" - t.Logf("Next page Link: %s", linkStr) - w.Header().Set("Link", linkStr) - } + query := newURL.Query() + query.Set("page", strings.Repeat("p", page+1)) + newURL.RawQuery = query.Encode() + linkStr := linkheader.Links{ + {URL: newURL.String(), Rel: "next"}, + }.String() + + t.Logf("Next page Link: %s", linkStr) + w.Header().Set("Link", linkStr) + } - w.WriteHeader(code) - w.Write([]byte(resps[page])) + w.WriteHeader(code) + w.Write([]byte(resps[page])) + }) }) } -func APIServerCustom(t *testing.T, method, path string, hf http.HandlerFunc) *httptest.Server { +func APIServerCustom(t *testing.T, custom func(*http.ServeMux)) *httptest.Server { h := http.NewServeMux() - h.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { - t.Logf("API Request: %s %s", r.Method, r.URL.String()) - - if r.Method != method { - w.WriteHeader(http.StatusNotImplemented) - return - } - - hf(w, r) - }) + // Add custom path handlers + custom(h) + // Default handler for auth h.HandleFunc("/login", func(w http.ResponseWriter, r *http.Request) { if r.Method != "POST" { w.WriteHeader(http.StatusNotImplemented) @@ -88,7 +98,9 @@ func APIServerCustom(t *testing.T, method, path string, hf http.HandlerFunc) *ht w.Write([]byte(loginResponse)) }) - if path != "/" { + // Check if mux has a handler for "/" + rootRequest := httptest.NewRequest("GET", "/", nil) + if _, pattern := h.Handler(rootRequest); pattern == "" { h.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { t.Errorf("Unexpected: %s %s", r.Method, r.URL.String()) http.NotFound(w, r)