Regression in v3.14.3 using -1 max value

[disq]

v3.14.3 introduced an issue where progressbar.Finish() behaves differently, when it's initialized with an -1 max value. (v3.14.2...v3.14.3)

Reverting #182 seems to fix the issue.

Reproduction:

package progressbar

import (
	"strings"
	"testing"
	"time"
)

func expectBuffer(t *testing.T, buf *strings.Builder, expect string) {
	t.Helper()
	current := strings.TrimSpace(buf.String())
	if current != expect {
		r := strings.NewReplacer("\r", "\\R", "\n", "\\N")
		current, expect = r.Replace(current), r.Replace(expect)
		t.Fatalf("Render mismatch\nResult: '%s'\nExpect: '%s'\n", current, expect)
	}
}

func TestRegression(t *testing.T) {
	buf := strings.Builder{}

	bar := NewOptions(-1,
		OptionSetWriter(&buf),
		OptionSetDescription("Syncing resources..."),
		OptionSetItsString("resources"),
		OptionShowIts(),
		OptionSetElapsedTime(true),
		OptionShowCount(),
	)
	bar.Reset()
	time.Sleep(1 * time.Second)
	expectBuffer(t, &buf, "")
	bar.Add(5)
	expectBuffer(t, &buf, "- Syncing resources... (5/-, 5 resources/s) [1s]")
	time.Sleep(1 * time.Second)
	bar.Add(5)
	expectBuffer(t, &buf, "- Syncing resources... (5/-, 5 resources/s) [1s] \r                                                 \r\r| Syncing resources... (10/-, 5 resources/s) [2s]")
	time.Sleep(1 * time.Second)
	bar.Finish()
	expectBuffer(t, &buf, "- Syncing resources... (5/-, 5 resources/s) [1s] \r                                                 \r\r| Syncing resources... (10/-, 5 resources/s) [2s] \r                                                  \r\r- Syncing resources... (10/-, 3 resources/s) [3s]")
}

func TestRegressionClearOnFinish(t *testing.T) {
	buf := strings.Builder{}

	bar := NewOptions(-1,
		OptionSetWriter(&buf),
		OptionSetDescription("Syncing resources..."),
		OptionSetItsString("resources"),
		OptionShowIts(),
		OptionSetElapsedTime(true),
		OptionShowCount(),
		OptionClearOnFinish(),
	)
	bar.Reset()
	time.Sleep(1 * time.Second)
	expectBuffer(t, &buf, "")
	bar.Add(5)
	expectBuffer(t, &buf, "- Syncing resources... (5/-, 5 resources/s) [1s]")
	time.Sleep(1 * time.Second)
	bar.Add(5)
	expectBuffer(t, &buf, "- Syncing resources... (5/-, 5 resources/s) [1s] \r                                                 \r\r| Syncing resources... (10/-, 5 resources/s) [2s]")
	time.Sleep(1 * time.Second)
	bar.Finish()
	expectBuffer(t, &buf, "- Syncing resources... (5/-, 5 resources/s) [1s] \r                                                 \r\r| Syncing resources... (10/-, 5 resources/s) [2s]")
}

Passes in v3.14.2, results in v3.14.3:

=== RUN   TestRegression
    new_test.go:40: Render mismatch
        Result: '- Syncing resources... (5/-, 5 resources/s) [1s] \R                                                 \R\R| Syncing resources... (10/-, 5 resources/s) [2s] \R                                                  \R\R- Syncing resources... (40/-, 13 resources/s) [3s]'
        Expect: '- Syncing resources... (5/-, 5 resources/s) [1s] \R                                                 \R\R| Syncing resources... (10/-, 5 resources/s) [2s] \R                                                  \R\R- Syncing resources... (10/-, 3 resources/s) [3s]'
--- FAIL: TestRegression (3.00s)

=== RUN   TestRegressionClearOnFinish
    new_test.go:65: Render mismatch
        Result: '- Syncing resources... (5/-, 5 resources/s) [1s] \R                                                 \R\R| Syncing resources... (10/-, 5 resources/s) [2s] \R                                                  \R\R- Syncing resources... (40/-, 13 resources/s) [3s]'
        Expect: '- Syncing resources... (5/-, 5 resources/s) [1s] \R                                                 \R\R| Syncing resources... (10/-, 5 resources/s) [2s]'
--- FAIL: TestRegressionClearOnFinish (3.00s)

FAIL

[black-night-heron]

Thank you for bringing this regression to my attention. I apologize for the oversight regarding the spinner scenario.

When initializing a spinner, its max is set to the width, which is why the total resources show as 40 after Finish() (the default width).

Regarding the failure to clear on finish, when Add(num) is called, the spinner sets currentNum to (currentNum + num) % max, resulting in 0. Consequently, the spinner is not considered finished and thus is not cleared.

I will submit a fix to revert the changes for the spinner type to restore the expected behavior.

[disq]

Fixed in v3.14.4, thanks!