Refactor ProgressBar for easier testing
Refactors ProgressBar to be an interface with two implementations,
realProgressBar (the existing impl) & StubProgressBar (no-op for
unsupported terminals and testing.
The terminal character check is moved into the constructor to allow
selecting whether to return a realProgressBar or a
StubProgressBar. Callers no longer need to perform the check
themselves, and are guaranteed to get a ProgressBar back, so can avoid
later nil checks.
This change bypasses the need to write a dependency injection wrapper
for the `term` package for testing fuzz/main.go
Bug: 344014313
Change-Id: I8e4d5342ce4f42750b3179730a72ab6a4c7ace1d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/258094
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@google.com>
Reviewed-by: Brian Sheedy <bsheedy@google.com>
diff --git a/tools/src/cmd/fuzz/main.go b/tools/src/cmd/fuzz/main.go
index c758833..6e2e2ef 100644
--- a/tools/src/cmd/fuzz/main.go
+++ b/tools/src/cmd/fuzz/main.go
@@ -45,7 +45,6 @@
"dawn.googlesource.com/dawn/tools/src/glob"
"dawn.googlesource.com/dawn/tools/src/oswrapper"
"dawn.googlesource.com/dawn/tools/src/progressbar"
- "dawn.googlesource.com/dawn/tools/src/term"
"dawn.googlesource.com/dawn/tools/src/transform"
"dawn.googlesource.com/dawn/tools/src/utils"
)
@@ -73,17 +72,18 @@
)
type cmdConfig struct {
- verbose bool
- dump bool
- fuzzMode FuzzMode
- cmdMode TaskMode // meta-task being requested by the user, may require running multiple tasks internally
- filter string
- inputs string
- build string
- out string
- numProcesses int
- osWrapper oswrapper.OSWrapper
- execWrapper execwrapper.ExecWrapper
+ verbose bool
+ dump bool
+ fuzzMode FuzzMode
+ cmdMode TaskMode // meta-task being requested by the user, may require running multiple tasks internally
+ filter string
+ inputs string
+ build string
+ out string
+ numProcesses int
+ osWrapper oswrapper.OSWrapper
+ execWrapper execwrapper.ExecWrapper
+ progressBuilder progressbar.Build
}
func showUsage() {
@@ -106,6 +106,7 @@
c := cmdConfig{}
c.osWrapper = oswrapper.GetRealOSWrapper()
c.execWrapper = execwrapper.CreateRealExecWrapper()
+ c.progressBuilder = progressbar.New
flag.Usage = showUsage
@@ -308,24 +309,19 @@
remaining := transform.SliceToChan(files)
- var pb *progressbar.ProgressBar
- if term.CanUseAnsiEscapeSequences() {
- pb = progressbar.New(os.Stdout, nil)
- defer pb.Stop()
- }
+ pb := t.progressBuilder(os.Stdout, nil)
+ defer pb.Stop()
var numDone uint32
routine := func() error {
for file := range remaining {
atomic.AddUint32(&numDone, 1)
- if pb != nil {
- pb.Update(progressbar.Status{
- Total: len(files),
- Segments: []progressbar.Segment{
- {Count: int(atomic.LoadUint32(&numDone))},
- },
- })
- }
+ pb.Update(progressbar.Status{
+ Total: len(files),
+ Segments: []progressbar.Segment{
+ {Count: int(atomic.LoadUint32(&numDone))},
+ },
+ })
if out, err := t.execWrapper.Command(t.fuzzer, file).RunWithCombinedOutput(); err != nil {
_, fuzzer := filepath.Split(t.fuzzer)
diff --git a/tools/src/progressbar/progressbar.go b/tools/src/progressbar/progressbar.go
index cbd0c21..4e21aa3 100644
--- a/tools/src/progressbar/progressbar.go
+++ b/tools/src/progressbar/progressbar.go
@@ -25,7 +25,7 @@
// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-// Package progressbar provides functions for drawing unicode progress bars to
+// Package progressbar provides functions for drawing Unicode progress bars to
// the terminal
package progressbar
@@ -36,6 +36,8 @@
"math"
"strings"
"time"
+
+ "dawn.googlesource.com/dawn/tools/src/term"
)
// Defaults for the Config
@@ -80,19 +82,34 @@
Segments []Segment
}
-// ProgressBar returns a string with an ANSI-colored progress bar, providing
-// realtime information about the status of the CTS run.
-// Note: We'll want to skip this if !isatty or if we're running on windows.
-type ProgressBar struct {
- Config
- out io.Writer
- c chan Status
+// ProgressBar is an interface for progress bars.
+type ProgressBar interface {
+ // Update updates the ProgressBar with the given status
+ Update(s Status)
+ // Stop stops drawing the progress bar.
+ // Once called, the ProgressBar must not be used.
+ Stop()
}
+type Build func(out io.Writer, cfg *Config) ProgressBar
+
// New returns a new ProgressBar that streams output to out.
+// Selects between realProgressBar and StubProgressBar automatically depending
+// on if the output will be displayed in the terminal correctly.
// Call ProgressBar.Stop() once finished.
-func New(out io.Writer, cfg *Config) *ProgressBar {
- p := &ProgressBar{out: out, c: make(chan Status, 64)}
+func New(out io.Writer, cfg *Config) ProgressBar {
+ if !term.CanUseAnsiEscapeSequences() {
+ return NewStubProgressBar(out, cfg)
+ }
+
+ return newRealProgressBar(out, cfg)
+}
+
+// NewRealProgressBar constructs and returns a realProgressBar.
+// Does not check if the terminal supports the needed escape characters, which
+// is why it is not exposed outside the package, use New() instead.
+func newRealProgressBar(out io.Writer, cfg *Config) ProgressBar {
+ p := &realProgressBar{out: out, c: make(chan Status, 64)}
if cfg != nil {
p.Config = *cfg
} else {
@@ -125,14 +142,36 @@
return p
}
-// Update updates the ProgressBar with the given status
-func (p *ProgressBar) Update(s Status) {
+// NewStubProgressBar returns a new StubProgressBar, used in tests to guarantee
+// the type of ProgressBar returned.
+// Note: In production code, use New() instead.
+func NewStubProgressBar(_ io.Writer, _ *Config) ProgressBar {
+ return &StubProgressBar{}
+}
+
+// StubProgressBar is a ProgressBar that does nothing, used in environments that
+// don't support the control characters needed or when testing to avoid spamming
+// output.
+type StubProgressBar struct{}
+
+func (p *StubProgressBar) Update(s Status) {}
+
+func (p *StubProgressBar) Stop() {}
+
+// realProgressBar writes out a string with an ANSI-colored progress bar,
+// providing realtime information about the status of the CTS run.
+// Note: Does not work if !isatty or if running on windows.
+type realProgressBar struct {
+ Config
+ out io.Writer
+ c chan Status
+}
+
+func (p *realProgressBar) Update(s Status) {
p.c <- s
}
-// Stop stops drawing the progress bar.
-// Once called, the ProgressBar must not be used.
-func (p *ProgressBar) Stop() {
+func (p *realProgressBar) Stop() {
close(p.c)
}
