diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e168080..e0de0ad 100644 --- a/.github/workflows/ci.yml +++ b/.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 ./... diff --git a/go.mod b/go.mod index e8db3f0..d0e1922 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 36e0239..3f5200c 100644 --- a/go.sum +++ b/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= diff --git a/progress.go b/progress.go index bf34766..a8b718d 100644 --- a/progress.go +++ b/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, diff --git a/progress_internal_test.go b/progress_internal_test.go new file mode 100644 index 0000000..bd78a86 --- /dev/null +++ b/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) + } +}