Skip to content

Commit

Permalink
cmd: make sure there are no crashes in no api data is returned
Browse files Browse the repository at this point in the history
Make sure that snap refresh/install does not crash if a change has no
api data. This is not happening in usual interactions with the store,
but it does in some cases when we use the fakestore, which is breaking
some integration tests.
  • Loading branch information
alfonsosanchezbeato committed Jul 9, 2024
1 parent c59a5f6 commit 35964cf
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 4 deletions.
12 changes: 8 additions & 4 deletions cmd/snap/cmd_snap_op.go
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,8 @@ func (x *cmdInstall) installMany(names []string, opts *client.SnapOptions) error
return err
}

if changedSnaps.hasChanges() {
// changedSnaps might be nil in some operations with the fakestore
if changedSnaps != nil && changedSnaps.hasChanges() {
if err := showDone(x.client, chg, changedSnaps, "install", opts, x.getEscapes()); err != nil {
return err
}
Expand All @@ -740,8 +741,10 @@ func (x *cmdInstall) installMany(names []string, opts *client.SnapOptions) error

// show skipped
seen := make(map[string]bool)
for _, name := range changedSnaps.names {
seen[name] = true
if changedSnaps != nil {
for _, name := range changedSnaps.names {
seen[name] = true
}
}
for _, name := range names {
if !seen[name] {
Expand Down Expand Up @@ -846,7 +849,8 @@ func (x *cmdRefresh) refreshMany(snaps []string, opts *client.SnapOptions) error
return err
}

if changedSnaps.hasChanges() {
// changedSnaps might be nil in some operations with the fakestore
if changedSnaps != nil && changedSnaps.hasChanges() {
return showDone(x.client, chg, changedSnaps, "refresh", opts, x.getEscapes())
}

Expand Down
83 changes: 83 additions & 0 deletions cmd/snap/cmd_snap_op_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2837,6 +2837,89 @@ func (s *SnapOpSuite) TestInstallMany(c *check.C) {
c.Check(n, check.Equals, total)
}

func (s *SnapOpSuite) TestInstallManyNoChanges(c *check.C) {
total := 3
n := 0
s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) {
switch n {
case 0:
c.Check(r.URL.Path, check.Equals, "/v2/snaps")
c.Check(DecodedRequestBody(c, r), check.DeepEquals, map[string]interface{}{
"action": "install",
"snaps": []interface{}{"one", "two"},
"transaction": string(client.TransactionPerSnap),
})

c.Check(r.Method, check.Equals, "POST")
w.WriteHeader(202)
fmt.Fprintln(w, `{"type":"async", "change": "42", "status-code": 202}`)
case 1:
c.Check(r.Method, check.Equals, "GET")
c.Check(r.URL.Path, check.Equals, "/v2/changes/42")
fmt.Fprintln(w, `{"type": "sync", "result": {"status": "Doing"}}`)
case 2:
c.Check(r.Method, check.Equals, "GET")
c.Check(r.URL.Path, check.Equals, "/v2/changes/42")
fmt.Fprintln(w, `{"type": "sync", "result": {"ready": true, "status": "Done", "data": {}}}`)
default:
c.Fatalf("expected to get %d requests, now on %d", total, n+1)
}

n++
})

rest, err := snap.Parser(snap.Client()).ParseArgs([]string{"install", "one", "two"})
c.Assert(err, check.IsNil)
c.Assert(rest, check.DeepEquals, []string{})
// note that (stable) is omitted
c.Check(s.Stdout(), check.Matches, `(?sm).*one already installed`)
c.Check(s.Stdout(), check.Matches, `(?sm).*two already installed`)
c.Check(s.Stderr(), check.Equals, "")
// ensure that the fake server api was actually hit
c.Check(n, check.Equals, total)
}

func (s *SnapOpSuite) TestRefreshManyNoChanges(c *check.C) {
total := 3
n := 0
s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) {
switch n {
case 0:
c.Check(r.URL.Path, check.Equals, "/v2/snaps")
c.Check(DecodedRequestBody(c, r), check.DeepEquals, map[string]interface{}{
"action": "refresh",
"snaps": []interface{}{"one", "two"},
"transaction": string(client.TransactionPerSnap),
})

c.Check(r.Method, check.Equals, "POST")
w.WriteHeader(202)
fmt.Fprintln(w, `{"type":"async", "change": "42", "status-code": 202}`)
case 1:
c.Check(r.Method, check.Equals, "GET")
c.Check(r.URL.Path, check.Equals, "/v2/changes/42")
fmt.Fprintln(w, `{"type": "sync", "result": {"status": "Doing"}}`)
case 2:
c.Check(r.Method, check.Equals, "GET")
c.Check(r.URL.Path, check.Equals, "/v2/changes/42")
fmt.Fprintln(w, `{"type": "sync", "result": {"ready": true, "status": "Done", "data": {}}}`)
default:
c.Fatalf("expected to get %d requests, now on %d", total, n+1)
}

n++
})

rest, err := snap.Parser(snap.Client()).ParseArgs([]string{"refresh", "one", "two"})
c.Assert(err, check.IsNil)
c.Assert(rest, check.DeepEquals, []string{})
// note that (stable) is omitted
c.Check(s.Stdout(), check.Equals, "")
c.Check(s.Stderr(), check.Equals, "All snaps up to date.\n")
// ensure that the fake server api was actually hit
c.Check(n, check.Equals, total)
}

func (s *SnapOpSuite) TestInstallZeroEmpty(c *check.C) {
_, err := snap.Parser(snap.Client()).ParseArgs([]string{"install"})
c.Assert(err, check.Not(check.IsNil))
Expand Down

0 comments on commit 35964cf

Please sign in to comment.