chore: Expanding lll coverage to worktrees (#5861)

Author: yhakbar

.golangci.yml

@@ -114,7 +114,7 @@ linters:
       # trying to get this merged in.
       - linters:
           - lll
-        path-except: '^(internal/awshelper/|internal/cas/|internal/cli/commands/(backend/(delete|migrate)|catalog/tui/command|exec|find|help|list|stack)/|internal/cloner/|internal/configbridge/|internal/engine/|internal/errorconfig/|internal/errors/|internal/experiment/|internal/gcphelper/|internal/git/|internal/prepare/|internal/runner/(common|graph|run/creds/providers/(amazonsts|externalcmd))/|internal/stacks/(generate|output)/|internal/tf/cache/(controllers|middleware)/|internal/tips/|internal/vfs/|pkg/log/(format/placeholders|writer)/)'
+        path-except: '^(internal/awshelper/|internal/cas/|internal/cli/commands/(backend/(delete|migrate)|catalog/tui/command|exec|find|help|list|stack)/|internal/cloner/|internal/configbridge/|internal/engine/|internal/errorconfig/|internal/errors/|internal/experiment/|internal/gcphelper/|internal/git/|internal/prepare/|internal/runner/(common|graph|run/creds/providers/(amazonsts|externalcmd))/|internal/stacks/(generate|output)/|internal/tf/cache/(controllers|middleware)/|internal/tips/|internal/vfs/|internal/worktrees/|pkg/log/(format/placeholders|writer)/)'
     paths:
       - docs
       - _ci

internal/worktrees/worktrees.go

@@ -380,114 +380,118 @@ func NewWorktrees(
 	repoCommit := gitRunner.GetHeadCommit(ctx)
 
 	// Wrap entire worktree creation process with telemetry
-	traceErr := filter.TraceGitWorktreesCreate(ctx, workingDir, len(gitRefs), repoRemote, repoBranch, repoCommit, func(ctx context.Context) error {
-		var (
-			errs []error
-			mu   sync.Mutex
-		)
+	traceErr := filter.TraceGitWorktreesCreate(
+		ctx, workingDir, len(gitRefs), repoRemote, repoBranch, repoCommit,
+		func(ctx context.Context) error {
+			var (
+				errs []error
+				mu   sync.Mutex
+			)
 
-		expressionsToDiffs := make(map[*filter.GitExpression]*git.Diffs, len(gitExpressions))
+			expressionsToDiffs := make(map[*filter.GitExpression]*git.Diffs, len(gitExpressions))
 
-		gitCmdGroup, gitCmdCtx := errgroup.WithContext(ctx)
-		// Cap concurrent git commands to the number of refs, but no more than available CPUs.
-		gitCmdGroup.SetLimit(min(runtime.GOMAXPROCS(0), len(gitRefs)))
+			gitCmdGroup, gitCmdCtx := errgroup.WithContext(ctx)
+			// Cap concurrent git commands to the number of refs, but no more than available CPUs.
+			gitCmdGroup.SetLimit(min(runtime.GOMAXPROCS(0), len(gitRefs)))
 
-		refsToPaths := make(map[string]string, len(gitRefs))
+			refsToPaths := make(map[string]string, len(gitRefs))
+
+			if len(gitRefs) > 0 {
+				gitCmdGroup.Go(func() error {
+					paths, err := createGitWorktrees(gitCmdCtx, l, gitRunner, gitRefs, repoRemote, repoBranch, repoCommit, experiments)
+					if err != nil {
+						mu.Lock()
+
+						errs = append(errs, err)
+
+						mu.Unlock()
+
+						return err
+					}
 
-		if len(gitRefs) > 0 {
-			gitCmdGroup.Go(func() error {
-				paths, err := createGitWorktrees(gitCmdCtx, l, gitRunner, gitRefs, repoRemote, repoBranch, repoCommit, experiments)
-				if err != nil {
 					mu.Lock()
 
-					errs = append(errs, err)
+					maps.Copy(refsToPaths, paths)
 
 					mu.Unlock()
 
-					return err
-				}
+					return nil
+				})
+			}
 
-				mu.Lock()
+			for _, gitExpression := range gitExpressions {
+				gitCmdGroup.Go(func() error {
+					// Wrap git diff with telemetry
+					var diffs *git.Diffs
 
-				maps.Copy(refsToPaths, paths)
+					diffErr := filter.TraceGitDiff(
+						gitCmdCtx, gitExpression.FromRef, gitExpression.ToRef, repoRemote,
+						func(ctx context.Context) error {
+							var err error
 
-				mu.Unlock()
+							diffs, err = gitRunner.Diff(ctx, gitExpression.FromRef, gitExpression.ToRef)
 
-				return nil
-			})
-		}
+							return err
+						})
+					if diffErr != nil {
+						mu.Lock()
 
-		for _, gitExpression := range gitExpressions {
-			gitCmdGroup.Go(func() error {
-				// Wrap git diff with telemetry
-				var diffs *git.Diffs
+						errs = append(errs, diffErr)
 
-				diffErr := filter.TraceGitDiff(gitCmdCtx, gitExpression.FromRef, gitExpression.ToRef, repoRemote, func(ctx context.Context) error {
-					var err error
+						mu.Unlock()
 
-					diffs, err = gitRunner.Diff(ctx, gitExpression.FromRef, gitExpression.ToRef)
+						return nil
+					}
 
-					return err
-				})
-				if diffErr != nil {
 					mu.Lock()
 
-					errs = append(errs, diffErr)
+					expressionsToDiffs[gitExpression] = diffs
 
 					mu.Unlock()
 
 					return nil
-				}
+				})
+			}
 
-				mu.Lock()
+			if err := gitCmdGroup.Wait(); err != nil {
+				worktrees = &Worktrees{
+					WorktreePairs:      make(map[string]WorktreePair),
+					OriginalWorkingDir: workingDir,
+					gitRunner:          gitRunner,
+				}
+				outerErr = err
 
-				expressionsToDiffs[gitExpression] = diffs
+				return err
+			}
 
-				mu.Unlock()
+			worktreePairs := make(map[string]WorktreePair, len(gitExpressions))
+			for _, gitExpression := range gitExpressions {
+				worktreePairs[gitExpression.String()] = WorktreePair{
+					GitExpression: gitExpression,
+					Diffs:         expressionsToDiffs[gitExpression],
+					FromWorktree:  Worktree{Ref: gitExpression.FromRef, Path: refsToPaths[gitExpression.FromRef]},
+					ToWorktree:    Worktree{Ref: gitExpression.ToRef, Path: refsToPaths[gitExpression.ToRef]},
+				}
 
-				return nil
-			})
-		}
+				// Record telemetry for diff results
+				if diffs := expressionsToDiffs[gitExpression]; diffs != nil {
+					recordDiffTelemetry(ctx, diffs)
+				}
+			}
 
-		if err := gitCmdGroup.Wait(); err != nil {
 			worktrees = &Worktrees{
-				WorktreePairs:      make(map[string]WorktreePair),
+				WorktreePairs:      worktreePairs,
 				OriginalWorkingDir: workingDir,
 				gitRunner:          gitRunner,
 			}
-			outerErr = err
-
-			return err
-		}
 
-		worktreePairs := make(map[string]WorktreePair, len(gitExpressions))
-		for _, gitExpression := range gitExpressions {
-			worktreePairs[gitExpression.String()] = WorktreePair{
-				GitExpression: gitExpression,
-				Diffs:         expressionsToDiffs[gitExpression],
-				FromWorktree:  Worktree{Ref: gitExpression.FromRef, Path: refsToPaths[gitExpression.FromRef]},
-				ToWorktree:    Worktree{Ref: gitExpression.ToRef, Path: refsToPaths[gitExpression.ToRef]},
+			if len(errs) > 0 {
+				outerErr = errors.Join(errs...)
+				return outerErr
 			}
 
-			// Record telemetry for diff results
-			if diffs := expressionsToDiffs[gitExpression]; diffs != nil {
-				recordDiffTelemetry(ctx, diffs)
-			}
-		}
-
-		worktrees = &Worktrees{
-			WorktreePairs:      worktreePairs,
-			OriginalWorkingDir: workingDir,
-			gitRunner:          gitRunner,
-		}
-
-		if len(errs) > 0 {
-			outerErr = errors.Join(errs...)
-			return outerErr
-		}
-
-		return nil
-	})
+			return nil
+		})
 
 	if traceErr != nil && outerErr == nil {
 		l.Warnf("telemetry trace error during worktree creation: %v", traceErr)
@@ -599,18 +603,20 @@ func createGitWorktrees(
 		}
 
 		// Wrap individual worktree creation with telemetry including repo info
-		err = filter.TraceGitWorktreeCreate(ctx, ref, tmpDir, repoRemote, repoBranch, repoCommit, func(ctx context.Context) error {
-			if slowReporting {
-				return util.NotifyIfSlow(ctx, l, util.SpinnerWriter(), time.Second, util.SlowNotifyMsg{
-					Spinner: fmt.Sprintf("Creating Git worktree for reference %s...", ref),
-					Done:    "Created Git worktree for reference " + ref,
-				}, func() error {
-					return gitRunner.CreateDetachedWorktree(ctx, tmpDir, ref)
-				})
-			}
+		err = filter.TraceGitWorktreeCreate(
+			ctx, ref, tmpDir, repoRemote, repoBranch, repoCommit,
+			func(ctx context.Context) error {
+				if slowReporting {
+					return util.NotifyIfSlow(ctx, l, util.SpinnerWriter(), time.Second, util.SlowNotifyMsg{
+						Spinner: fmt.Sprintf("Creating Git worktree for reference %s...", ref),
+						Done:    "Created Git worktree for reference " + ref,
+					}, func() error {
+						return gitRunner.CreateDetachedWorktree(ctx, tmpDir, ref)
+					})
+				}
 
-			return gitRunner.CreateDetachedWorktree(ctx, tmpDir, ref)
-		})
+				return gitRunner.CreateDetachedWorktree(ctx, tmpDir, ref)
+			})
 		if err != nil {
 			if cleanErr := os.RemoveAll(tmpDir); cleanErr != nil {
 				l.Warnf("failed to clean worktree directory %s: %v", tmpDir, cleanErr)