CI: vet + modernize + race; fix progress bar math; move to slack-go/slack

.github/workflows/ci.yml

@@ -5,13 +5,16 @@ on:
     branches: [master]
   pull_request:
 
+permissions:
+  contents: read
+
 jobs:
   test:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v7
 
-      - uses: actions/setup-go@v5
+      - uses: actions/setup-go@v7
         with:
           go-version-file: go.mod
 
@@ -24,5 +27,11 @@ jobs:
             exit 1
           fi
 
+      - name: Vet
+        run: go vet ./...
+
+      - name: Modernize
+        run: go run golang.org/x/tools/go/analysis/passes/modernize/cmd/modernize@v0.52.0 -test ./...
+
       - name: Test
-        run: go test -v ./...
+        run: go test -race -v ./...

go.mod

@@ -2,11 +2,6 @@ module github.com/sfreiberg/progress
 
 go 1.26
 
-require github.com/nlopes/slack v0.5.0
+require github.com/slack-go/slack v0.30.1
 
-require (
-	github.com/gorilla/websocket v1.4.0 // indirect
-	github.com/lusis/slack-test v0.0.0-20190426140909-c40012f20018 // indirect
-	github.com/pkg/errors v0.8.1 // indirect
-	github.com/stretchr/testify v1.12.1 // indirect
-)
+require github.com/gorilla/websocket v1.5.3 // indirect

go.sum

@@ -1,11 +1,9 @@
-github.com/gorilla/websocket v1.4.0 h1:WDFjx/TMzVgy9VdMMQi2K2Emtwi2QcUQsztZ/zLaH/Q=
-github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
-github.com/lusis/slack-test v0.0.0-20190426140909-c40012f20018 h1:MNApn+Z+fIT4NPZopPfCc1obT6aY3SVM6DOctz1A9ZU=
-github.com/lusis/slack-test v0.0.0-20190426140909-c40012f20018/go.mod h1:sFlOUpQL1YcjhFVXhg1CG8ZASEs/Mf1oVb6H75JL/zg=
-github.com/nlopes/slack v0.5.0 h1:NbIae8Kd0NpqaEI3iUrsuS0KbcEDhzhc939jLW5fNm0=
-github.com/nlopes/slack v0.5.0/go.mod h1:jVI4BBK3lSktibKahxBF74txcK2vyvkza1z/+rRnVAM=
-github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
-github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
+github.com/go-test/deep v1.1.1 h1:0r/53hagsehfO4bzD2Pgr/+RgHqhmf+k1Bpse2cTu1U=
+github.com/go-test/deep v1.1.1/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE=
+github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg=
+github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
+github.com/slack-go/slack v0.30.1 h1:q6FjZssqts6hJ+DRWCnnO2YVUh+E3/UNrC0EIO2xci8=
+github.com/slack-go/slack v0.30.1/go.mod h1:radSmEe8GEgE+aK980ZafMR0AQhiZjADptN5k/gOw34=
 github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE=
 github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg=
 go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw=

progress.go

@@ -7,7 +7,7 @@ import (
 	"text/template"
 	"time"
 
-	"github.com/nlopes/slack"
+	"github.com/slack-go/slack"
 )
 
 var (
@@ -69,7 +69,8 @@ func (p *Progress) Update(pos int) error {
 		return ErrMaxPosExceeded
 	}
 
-	pct := int(float32(pos) / float32(p.Opts.TotalUnits) * 100)
+	// Integer math avoids float32 rounding (e.g. 53/100 became 52%).
+	pct := pos * 100 / p.Opts.TotalUnits
 	if pct <= p.lastPct { // We haven't progressed so no need to update slack
 		return nil
 	}
@@ -85,31 +86,36 @@ func (p *Progress) Update(pos int) error {
 			slack.MsgOptionText(msg, false),
 			slack.MsgOptionAsUser(p.Opts.AsUser),
 		}
-		p.channel, p.ts, _, err = p.client.SendMessage(p.channel, msgOpts...)
-		return err
+		channel, ts, _, err := p.client.SendMessage(p.channel, msgOpts...)
+		if err != nil {
+			// Keep the original channel so the next call can retry.
+			return err
+		}
+		p.channel, p.ts, p.lastPct = channel, ts, pct
+		return nil
 	}
 
 	_, ts, _, err := p.client.UpdateMessage(p.channel, p.ts, slack.MsgOptionText(msg, false))
+	if err != nil {
+		// Keep the existing timestamp so a transient failure doesn't make the
+		// next call post a second progress bar.
+		return err
+	}
 	p.ts = ts
 	p.lastPct = pct
-	return err
+	return nil
 }
 
 func (p *Progress) drawBar(pos int) string {
-	if pos == 0 {
-		return strings.Repeat(p.Opts.Empty, p.Opts.Width)
-	}
-
-	bar := strings.Repeat(p.Opts.Fill, pos/p.Opts.Width)
-	bar += strings.Repeat(p.Opts.Empty, p.Opts.Width-len([]rune(bar)))
-
-	return bar
+	// pos is a percentage; scale it to the width of the bar.
+	filled := min(max(pos*p.Opts.Width/100, 0), p.Opts.Width)
+	return strings.Repeat(p.Opts.Fill, filled) + strings.Repeat(p.Opts.Empty, p.Opts.Width-filled)
 }
 
 func (p *Progress) msg(pos int) (string, error) {
 	msg := &strings.Builder{}
 
-	data := map[string]interface{}{
+	data := map[string]any{
 		"Task":        p.Opts.Task,
 		"ProgBar":     p.drawBar(pos),
 		"Pos":         pos,

progress_internal_test.go

@@ -0,0 +1,131 @@
+package progress
+
+import (
+	"bytes"
+	"io"
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/slack-go/slack"
+)
+
+func TestDrawBar(t *testing.T) {
+	tests := []struct {
+		width, pct int
+		want       string
+	}{
+		{10, 0, "----------"},
+		{10, 55, "#####-----"},
+		{10, 100, "##########"},
+		{20, 50, "##########----------"},
+		{20, 100, "####################"},
+		{5, 100, "#####"}, // used to panic: 100/5 = 20 filled for a width of 5
+		{4, 50, "##--"},
+	}
+
+	for _, tt := range tests {
+		p := &Progress{Opts: &Options{Fill: "#", Empty: "-", Width: tt.width}}
+		if got := p.drawBar(tt.pct); got != tt.want {
+			t.Errorf("drawBar(%d) with width %d = %q, want %q", tt.pct, tt.width, got, tt.want)
+		}
+	}
+}
+
+func TestDrawBarMultiRuneFill(t *testing.T) {
+	// Fill and Empty can be multi-character strings such as Slack emoji
+	// shortcodes; the bar should still be Width segments long.
+	p := &Progress{Opts: &Options{Fill: ":x:", Empty: ":o:", Width: 4}}
+	if got, want := p.drawBar(50), ":x::x::o::o:"; got != want {
+		t.Errorf("drawBar(50) = %q, want %q", got, want)
+	}
+}
+
+// fakeSlack answers Slack Web API calls. Requests to methods listed in fail
+// get an error response.
+type fakeSlack struct {
+	calls []string
+	fail  map[string]bool
+}
+
+func (f *fakeSlack) Do(req *http.Request) (*http.Response, error) {
+	method := req.URL.Path[strings.LastIndex(req.URL.Path, "/")+1:]
+	f.calls = append(f.calls, method)
+
+	body := `{"ok":true,"channel":"C123","ts":"1.000"}`
+	if f.fail[method] {
+		body = `{"ok":false,"error":"internal_error"}`
+	}
+	return &http.Response{
+		StatusCode: http.StatusOK,
+		Header:     http.Header{"Content-Type": {"application/json"}},
+		Body:       io.NopCloser(bytes.NewBufferString(body)),
+		Request:    req,
+	}, nil
+}
+
+func newTestProgress(f *fakeSlack) *Progress {
+	p := New("token", "general", nil)
+	p.client = slack.New("token", slack.OptionHTTPClient(f))
+	return p
+}
+
+func TestUpdateRetriesAfterFailedPost(t *testing.T) {
+	f := &fakeSlack{fail: map[string]bool{"chat.postMessage": true}}
+	p := newTestProgress(f)
+
+	if err := p.Update(10); err == nil {
+		t.Fatal("Update() with a failing chat.postMessage: expected an error")
+	}
+	if p.channel != "general" {
+		t.Errorf("channel = %q after a failed post, want it unchanged", p.channel)
+	}
+
+	f.fail = nil
+	if err := p.Update(10); err != nil {
+		t.Fatalf("Update() retry: %v", err)
+	}
+	if p.channel != "C123" || p.ts != "1.000" {
+		t.Errorf("channel, ts = %q, %q, want C123, 1.000", p.channel, p.ts)
+	}
+}
+
+func TestUpdateKeepsMessageAfterFailedUpdate(t *testing.T) {
+	f := &fakeSlack{}
+	p := newTestProgress(f)
+
+	if err := p.Update(10); err != nil {
+		t.Fatal(err)
+	}
+
+	f.fail = map[string]bool{"chat.update": true}
+	if err := p.Update(20); err == nil {
+		t.Fatal("Update() with a failing chat.update: expected an error")
+	}
+
+	f.fail = nil
+	if err := p.Update(30); err != nil {
+		t.Fatal(err)
+	}
+
+	want := []string{"chat.postMessage", "chat.update", "chat.update"}
+	if strings.Join(f.calls, ",") != strings.Join(want, ",") {
+		t.Errorf("slack calls = %v, want %v (a failed update must not start a new message)", f.calls, want)
+	}
+}
+
+func TestUpdateSkipsUnchangedPercent(t *testing.T) {
+	f := &fakeSlack{}
+	p := newTestProgress(f)
+
+	for _, pos := range []int{10, 10, 20} {
+		if err := p.Update(pos); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	want := []string{"chat.postMessage", "chat.update"}
+	if strings.Join(f.calls, ",") != strings.Join(want, ",") {
+		t.Errorf("slack calls = %v, want %v", f.calls, want)
+	}
+}