PR comments

cli/module_build.go

@@ -1108,7 +1108,7 @@ func reloadModuleActionInner(
 		{ID: "resource", Message: "Adding resource...", CompletedMsg: "Resource added", IndentLevel: 1},
 	}
 
-	pm := NewProgressManager(allSteps)
+	pm := NewProgressManager(allSteps, WithProgressOutput(!args.NoProgress))
 	defer pm.Stop()
 
 	var needsRestart bool

cli/progress_manager.go

@@ -40,10 +40,21 @@ type ProgressManager struct {
 	stepMap        map[string]*Step
 	currentSpinner *pterm.SpinnerPrinter // Active child spinner (IndentLevel > 0)
 	mu             sync.Mutex
+	disabled       bool
+}
+
+// ProgressManagerOption allows customizing ProgressManager behavior at creation time.
+type ProgressManagerOption func(*ProgressManager)
+
+// WithProgressOutput enables or disables terminal output for a ProgressManager.
+func WithProgressOutput(enabled bool) ProgressManagerOption {
+	return func(pm *ProgressManager) {
+		pm.disabled = !enabled
+	}
 }
 
 // NewProgressManager creates a new ProgressManager with all steps registered upfront.
-func NewProgressManager(steps []*Step) *ProgressManager {
+func NewProgressManager(steps []*Step, opts ...ProgressManagerOption) *ProgressManager {
 	// Customize spinner style globally
 	pterm.Success.Prefix = pterm.Prefix{
 		Text:  "✓",
@@ -73,6 +84,10 @@ func NewProgressManager(steps []*Step) *ProgressManager {
 		currentSpinner: nil,
 	}
 
+	for _, opt := range opts {
+		opt(pm)
+	}
+
 	return pm
 }
 
@@ -101,6 +116,10 @@ func (pm *ProgressManager) Start(stepID string) error {
 	step.Status = StepRunning
 	step.startTime = time.Now() // Record start time
 
+	if pm.disabled {
+		return nil
+	}
+
 	// If this is a parent step (IndentLevel == 0), print it as a static "in progress" indicator
 	if step.IndentLevel == 0 {
 		// Use ellipsis to indicate parent is in progress (with extra space for alignment)
@@ -163,6 +182,10 @@ func (pm *ProgressManager) Complete(stepID string) error {
 
 	prefix := getPrefix(step)
 
+	if pm.disabled {
+		return nil
+	}
+
 	// If this is a parent step (IndentLevel == 0), update the static header
 	if step.IndentLevel == 0 {
 		// Count how many child lines were printed after the parent
@@ -225,6 +248,10 @@ func (pm *ProgressManager) CompleteWithMessage(stepID, message string) error {
 
 	prefix := getPrefix(step)
 
+	if pm.disabled {
+		return nil
+	}
+
 	// If this is the currently active spinner, stop it and mark success
 	if pm.currentSpinner != nil {
 		pm.currentSpinner.Success(" " + prefix + message + elapsed)
@@ -247,24 +274,12 @@ func (pm *ProgressManager) Fail(stepID string, err error) error {
 		return fmt.Errorf("step %q not found", stepID)
 	}
 
-	step.Status = StepFailed
-
 	msg := step.FailedMsg
 	if msg == "" {
 		msg = fmt.Sprintf("%s: %v", step.Message, err)
 	}
 
-	prefix := getPrefix(step)
-
-	// If this is the currently active spinner, stop it and mark failure
-	if pm.currentSpinner != nil {
-		pm.currentSpinner.Fail(" " + prefix + msg)
-		pm.currentSpinner = nil
-	} else {
-		// If no spinner is active, just print the error message
-		pterm.Error.Println(" " + prefix + msg)
-	}
-
+	pm.failWithMessageLocked(step, msg)
 	return nil
 }
 
@@ -278,8 +293,19 @@ func (pm *ProgressManager) FailWithMessage(stepID, message string) error {
 		return fmt.Errorf("step %q not found", stepID)
 	}
 
+	pm.failWithMessageLocked(step, message)
+	return nil
+}
+
+// failWithMessageLocked is the shared implementation for failing a step.
+// It assumes the lock is already held by the caller.
+func (pm *ProgressManager) failWithMessageLocked(step *Step, message string) {
 	step.Status = StepFailed
 
+	if pm.disabled {
+		return
+	}
+
 	prefix := getPrefix(step)
 
 	// If this is the currently active spinner, stop it and mark failure
@@ -290,15 +316,17 @@ func (pm *ProgressManager) FailWithMessage(stepID, message string) error {
 		// If no spinner is active, just print the error message
 		pterm.Error.Println(" " + prefix + message)
 	}
-
-	return nil
 }
 
 // UpdateText updates the text of the currently active spinner (for progress updates).
 func (pm *ProgressManager) UpdateText(text string) {
 	pm.mu.Lock()
 	defer pm.mu.Unlock()
 
+	if pm.disabled {
+		return
+	}
+
 	if pm.currentSpinner != nil {
 		pm.currentSpinner.UpdateText(text)
 	}
@@ -309,6 +337,10 @@ func (pm *ProgressManager) Stop() {
 	pm.mu.Lock()
 	defer pm.mu.Unlock()
 
+	if pm.disabled {
+		return
+	}
+
 	if pm.currentSpinner != nil {
 		_ = pm.currentSpinner.Stop() //nolint:errcheck
 		pm.currentSpinner = nil

cli/progress_manager_test.go

@@ -5,6 +5,8 @@ import (
 	"sync"
 	"testing"
 	"time"
+
+	"go.viam.com/test"
 )
 
 func TestNewProgressManager(t *testing.T) {
@@ -105,6 +107,47 @@ func TestStartChildStep(t *testing.T) {
 	}
 }
 
+func TestProgressManagerWithOutputDisabled(t *testing.T) {
+	steps := []*Step{
+		{ID: "parent", Message: "Parent", IndentLevel: 0},
+		{ID: "child", Message: "Child", IndentLevel: 1},
+	}
+
+	pm := NewProgressManager(steps, WithProgressOutput(false))
+	defer pm.Stop()
+
+	err := pm.Start("parent")
+	if err != nil {
+		t.Fatalf("Failed to start parent step: %v", err)
+	}
+
+	if pm.currentSpinner != nil {
+		t.Fatal("Expected no spinner when output is disabled for parent step")
+	}
+
+	err = pm.Start("child")
+	if err != nil {
+		t.Fatalf("Failed to start child step: %v", err)
+	}
+
+	if pm.currentSpinner != nil {
+		t.Fatal("Expected no spinner when output is disabled for child step")
+	}
+
+	err = pm.Complete("child")
+	if err != nil {
+		t.Fatalf("Failed to complete child step: %v", err)
+	}
+
+	err = pm.Complete("parent")
+	if err != nil {
+		t.Fatalf("Failed to complete parent step: %v", err)
+	}
+
+	test.That(t, pm.stepMap["parent"].Status, test.ShouldEqual, StepCompleted)
+	test.That(t, pm.stepMap["child"].Status, test.ShouldEqual, StepCompleted)
+}
+
 func TestStartInvalidStep(t *testing.T) {
 	steps := []*Step{
 		{ID: "valid", Message: "Valid step", IndentLevel: 0},
@@ -243,9 +286,7 @@ func TestCompleteWithElapsedTime(t *testing.T) {
 	}
 
 	elapsed := time.Since(step.startTime)
-	if elapsed < 0 {
-		t.Error("Expected positive elapsed time")
-	}
+	test.That(t, elapsed, test.ShouldBeGreaterThanOrEqualTo, 10*time.Millisecond)
 }
 
 func TestCompleteWithMessage(t *testing.T) {
@@ -375,30 +416,6 @@ func TestFailWithoutCustomMessage(t *testing.T) {
 	}
 }
 
-func TestUpdateText(t *testing.T) {
-	steps := []*Step{
-		{ID: "child", Message: "Child step", IndentLevel: 1},
-	}
-
-	pm := NewProgressManager(steps)
-	defer pm.Stop() // Clean up any active spinners
-
-	err := pm.Start("child")
-	if err != nil {
-		t.Fatalf("Failed to start child step: %v", err)
-	}
-
-	if pm.currentSpinner == nil {
-		t.Fatal("Expected currentSpinner to be set")
-	}
-
-	newText := "Updated child step"
-	pm.UpdateText(newText)
-
-	// We can't easily verify the text was updated since pterm doesn't expose it,
-	// but we can verify no error occurred
-}
-
 func TestStop(t *testing.T) {
 	steps := []*Step{
 		{ID: "child", Message: "Child step", IndentLevel: 1},