many: implement our own ANSI-escape-using progress indicator #3964

Merged
merged 8 commits into from Oct 10, 2017
@@ -1,3 +1,22 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
@zyga

zyga Sep 27, 2017

Contributor

Thank you!

+/*
+ * Copyright (C) 2017 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
package asserts
import (
@@ -1,3 +1,22 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2017 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
package asserts_test
import (
@@ -71,7 +71,7 @@ func (wmx *waitMixin) wait(cli *client.Client, id string) (*client.Change, error
}
func wait(cli *client.Client, id string) (*client.Change, error) {
- pb := progress.NewTextProgress()
+ pb := progress.MakeProgressBar()
defer func() {
pb.Finished()
}()
@@ -122,7 +122,7 @@ func wait(cli *client.Client, id string) (*client.Change, error) {
case t.ID == lastID:
pb.Set(float64(t.Progress.Done))
default:
- pb.Start(t.Progress.Label, float64(t.Progress.Total))
+ pb.Start(t.Summary, float64(t.Progress.Total))
lastID = t.ID
}
break
@@ -26,7 +26,6 @@ import (
"mime/multipart"
"net/http"
"net/http/httptest"
- "os"
"path/filepath"
"regexp"
"time"
@@ -35,6 +34,9 @@ import (
"github.com/snapcore/snapd/client"
snap "github.com/snapcore/snapd/cmd/snap"
+ "github.com/snapcore/snapd/progress"
+ "github.com/snapcore/snapd/progress/progresstest"
+ "github.com/snapcore/snapd/testutil"
)
type snapOpTestServer struct {
@@ -103,6 +105,8 @@ func (s *SnapOpSuite) TearDownTest(c *check.C) {
}
func (s *SnapOpSuite) TestWait(c *check.C) {
+ meter := &progresstest.Meter{}
+ defer progress.MockMeter(meter)()
restore := snap.MockMaxGoneTime(time.Millisecond)
defer restore()
@@ -111,27 +115,16 @@ func (s *SnapOpSuite) TestWait(c *check.C) {
snap.ClientConfig.BaseURL = server.URL
server.Close()
- d := c.MkDir()
- oldStdout := os.Stdout
- stdout, err := ioutil.TempFile(d, "stdout")
- c.Assert(err, check.IsNil)
- defer func() {
- os.Stdout = oldStdout
- stdout.Close()
- os.Remove(stdout.Name())
- }()
- os.Stdout = stdout
-
cli := snap.Client()
chg, err := snap.Wait(cli, "x")
c.Assert(chg, check.IsNil)
c.Assert(err, check.NotNil)
- buf, err := ioutil.ReadFile(stdout.Name())
- c.Assert(err, check.IsNil)
- c.Check(string(buf), check.Matches, "(?ms).*Waiting for server to restart.*")
+ c.Check(meter.Labels, testutil.Contains, "Waiting for server to restart")
}
func (s *SnapOpSuite) TestWaitRecovers(c *check.C) {
+ meter := &progresstest.Meter{}
+ defer progress.MockMeter(meter)()
restore := snap.MockMaxGoneTime(time.Millisecond)
defer restore()
@@ -144,27 +137,14 @@ func (s *SnapOpSuite) TestWaitRecovers(c *check.C) {
fmt.Fprintln(w, `{"type": "sync", "result": {"ready": true, "status": "Done"}}`)
})
- d := c.MkDir()
- oldStdout := os.Stdout
- stdout, err := ioutil.TempFile(d, "stdout")
- c.Assert(err, check.IsNil)
- defer func() {
- os.Stdout = oldStdout
- stdout.Close()
- os.Remove(stdout.Name())
- }()
- os.Stdout = stdout
-
cli := snap.Client()
chg, err := snap.Wait(cli, "x")
// we got the change
c.Assert(chg, check.NotNil)
c.Assert(err, check.IsNil)
- buf, err := ioutil.ReadFile(stdout.Name())
- c.Assert(err, check.IsNil)
// but only after recovering
- c.Check(string(buf), check.Matches, "(?ms).*Waiting for server to restart.*")
+ c.Check(meter.Labels, testutil.Contains, "Waiting for server to restart")
}
func (s *SnapOpSuite) TestInstall(c *check.C) {
@@ -21,15 +21,14 @@ package main_test
import (
"fmt"
- "io/ioutil"
"net/http"
- "os"
"time"
. "gopkg.in/check.v1"
snap "github.com/snapcore/snapd/cmd/snap"
- "github.com/snapcore/snapd/testutil"
+ "github.com/snapcore/snapd/progress"
+ "github.com/snapcore/snapd/progress/progresstest"
)
var fmtWatchChangeJSON = `{"type": "sync", "result": {
@@ -42,6 +41,8 @@ var fmtWatchChangeJSON = `{"type": "sync", "result": {
}}`
func (s *SnapSuite) TestCmdWatch(c *C) {
+ meter := &progresstest.Meter{}
+ defer progress.MockMeter(meter)()
restore := snap.MockMaxGoneTime(time.Millisecond)
defer restore()
@@ -64,22 +65,9 @@ func (s *SnapSuite) TestCmdWatch(c *C) {
n++
})
- oldStdout := os.Stdout
- stdout, err := ioutil.TempFile("", "stdout")
- c.Assert(err, IsNil)
- defer func() {
- os.Stdout = oldStdout
- stdout.Close()
- os.Remove(stdout.Name())
- }()
- os.Stdout = stdout
-
- _, err = snap.Parser().ParseArgs([]string{"watch", "42"})
- os.Stdout = oldStdout
+ _, err := snap.Parser().ParseArgs([]string{"watch", "42"})
c.Assert(err, IsNil)
c.Check(n, Equals, 3)
- buf, err := ioutil.ReadFile(stdout.Name())
- c.Assert(err, IsNil)
- c.Check(string(buf), testutil.Contains, "\rmy-snap 0 B / 100.00 KB")
+ c.Check(meter.Values, DeepEquals, []float64{51200})
}
View
@@ -1,3 +1,22 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
@zyga

zyga Sep 27, 2017

Contributor

Man, we missed quite a few!

+
+/*
+ * Copyright (C) 2016 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
package main
import (
View
@@ -796,7 +796,7 @@ func (*licenseData) Error() string {
}
type snapInstruction struct {
- progress.NullProgress
+ progress.NullMeter
Action string `json:"action"`
Channel string `json:"channel"`
Revision snap.Revision `json:"revision"`
@@ -2593,7 +2593,7 @@ func getLogs(c *Command, r *http.Request, user *auth.UserState) Response {
serviceNames[i] = appInfo.ServiceName()
}
- sysd := systemd.New(dirs.GlobalRootDir, &progress.NullProgress{})
+ sysd := systemd.New(dirs.GlobalRootDir, progress.Null)
reader, err := sysd.LogReader(serviceNames, n, follow)
if err != nil {
return InternalError("cannot get logs: %v", err)
View
@@ -278,7 +278,7 @@ func appInfosFor(st *state.State, names []string, opts appInfoOptions) ([]*snap.
func clientAppInfosFromSnapAppInfos(apps []*snap.AppInfo) []client.AppInfo {
// TODO: pass in an actual notifier here instead of null
// (Status doesn't _need_ it, but benefits from it)
- sysd := systemd.New(dirs.GlobalRootDir, &progress.NullProgress{})
+ sysd := systemd.New(dirs.GlobalRootDir, progress.Null)
out := make([]client.AppInfo, len(apps))
for i, app := range apps {
View
@@ -25,7 +25,9 @@ import (
"encoding/json"
"fmt"
"os"
+ "os/signal"
"path/filepath"
+ "syscall"
"github.com/snapcore/snapd/asserts"
"github.com/snapcore/snapd/asserts/snapasserts"
@@ -142,11 +144,24 @@ func (tsto *ToolingStore) DownloadSnap(name string, revision snap.Revision, opts
baseName := filepath.Base(snap.MountFile())
targetFn = filepath.Join(targetDir, baseName)
- pb := progress.NewTextProgress()
+ pb := progress.MakeProgressBar()
+ defer pb.Finished()
+
+ // Intercept sigint
@mvo5

mvo5 Oct 4, 2017

Collaborator

Thanks for this!

+ c := make(chan os.Signal, 3)
+ signal.Notify(c, syscall.SIGINT)
+ go func() {
@mvo5

mvo5 Oct 4, 2017

Collaborator

Should we have the "signal.Reset(...)" from below as a defer here?

@chipaca

chipaca Oct 4, 2017

Member

if image is expected to have any use beyond the one-shot interactive use it currently has then the goroutine needs cleanup and the signal reset, indeed. Otherwise nah :-)

+ <-c
+ pb.Finished()
+ os.Exit(1)
+ }()
+
if err = sto.Download(context.TODO(), name, targetFn, &snap.DownloadInfo, pb, tsto.user); err != nil {
return "", nil, err
}
+ signal.Reset(syscall.SIGINT)
+
return targetFn, snap, nil
}
View
@@ -1,3 +1,22 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2017 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
package osutil_test
import (
@@ -1,3 +1,22 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2016 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
package patch
import (
Oops, something went wrong.