From 70ef0c5c27a617b79e02c3964233a583ed4e46bf Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Thu, 17 Mar 2022 09:22:58 -0400 Subject: [PATCH 1/5] Don't write to Ui in ResolvePackages if dry run --- cli/internal/run/run.go | 2 +- cli/internal/scope/scope.go | 10 +++++++--- cli/internal/scope/scope_test.go | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 1752b3ce1bb49..58e63f04786ac 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -178,7 +178,7 @@ func (c *RunCommand) Run(args []string) int { return 1 } } - filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), scmInstance, ctx, c.Ui, c.Config.Logger) + filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), scmInstance, ctx, c.Ui, c.Config.Logger, runOptions.dryRunJson || runOptions.dryRun) if err != nil { c.logError(c.Config.Logger, "", fmt.Errorf("failed resolve packages to run %v", err)) } diff --git a/cli/internal/scope/scope.go b/cli/internal/scope/scope.go index c648677c1f59c..fe1f02b5580a7 100644 --- a/cli/internal/scope/scope.go +++ b/cli/internal/scope/scope.go @@ -25,7 +25,7 @@ type Opts struct { GlobalDepPatterns []string } -func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, logger hclog.Logger) (util.Set, error) { +func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, logger hclog.Logger, quiet bool) (util.Set, error) { changedFiles, err := getChangedFiles(opts, scm) if err != nil { return nil, err @@ -111,12 +111,16 @@ func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, } } } - tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) + if !quiet { + tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) + } } else if changedPackages.Len() > 0 { // --since was specified, there are changes, but no scope was specified. // Run the packages that have changed filteredPkgs = changedPackages - tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) + if !quiet { + tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) + } } else if scopePkgs.Len() > 0 { // There was either a global change, or no changes, or no --since flag // There was a --scope flag, run the desired scopes diff --git a/cli/internal/scope/scope_test.go b/cli/internal/scope/scope_test.go index 7244cfa939f1c..a29e5f8ea6250 100644 --- a/cli/internal/scope/scope_test.go +++ b/cli/internal/scope/scope_test.go @@ -261,7 +261,7 @@ func TestResolvePackages(t *testing.T) { PackageNames: packageNames, TopologicalGraph: graph, SCC: scc, - }, tui, logger) + }, tui, logger, false) if err != nil { t.Errorf("expected no error, got %v", err) } From aa551288d181353998f55c528a60da1d1eb1f67e Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Thu, 17 Mar 2022 09:34:10 -0400 Subject: [PATCH 2/5] Remove Ui side effects from ResolvePackages --- cli/internal/run/run.go | 10 ++++++++-- cli/internal/scope/scope.go | 10 +++------- cli/internal/scope/scope_test.go | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 58e63f04786ac..dcc66c0f5869a 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -178,10 +178,11 @@ func (c *RunCommand) Run(args []string) int { return 1 } } - filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), scmInstance, ctx, c.Ui, c.Config.Logger, runOptions.dryRunJson || runOptions.dryRun) + filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), scmInstance, ctx, c.Ui, c.Config.Logger) if err != nil { c.logError(c.Config.Logger, "", fmt.Errorf("failed resolve packages to run %v", err)) } + c.Config.Logger.Debug("global hash", "value", ctx.GlobalHash) c.Config.Logger.Debug("local cache folder", "path", runOptions.cacheFolder) fs.EnsureDir(runOptions.cacheFolder) @@ -329,7 +330,12 @@ func (c *RunCommand) runOperation(g *completeGraph, rs *runSpec, backend *api.La } else { packagesInScope := rs.FilteredPkgs.UnsafeListOfStrings() sort.Strings(packagesInScope) - c.Ui.Output(fmt.Sprintf(ui.Dim("• Packages in scope: %v"), strings.Join(packagesInScope, ", "))) + prettyPackagesInScope := strings.Join(packagesInScope, ", ") + if rs.Opts.since != "" { + c.Ui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), rs.Opts.since, prettyPackagesInScope)) + } else { + c.Ui.Output(fmt.Sprintf(ui.Dim("• Packages in scope: %v"), prettyPackagesInScope)) + } if rs.Opts.stream { c.Ui.Output(fmt.Sprintf("%s %s %s", ui.Dim("• Running"), ui.Dim(ui.Bold(strings.Join(rs.Targets, ", "))), ui.Dim(fmt.Sprintf("in %v packages", rs.FilteredPkgs.Len())))) } diff --git a/cli/internal/scope/scope.go b/cli/internal/scope/scope.go index fe1f02b5580a7..fad604e777b03 100644 --- a/cli/internal/scope/scope.go +++ b/cli/internal/scope/scope.go @@ -25,7 +25,7 @@ type Opts struct { GlobalDepPatterns []string } -func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, logger hclog.Logger, quiet bool) (util.Set, error) { +func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, logger hclog.Logger) (util.Set, error) { changedFiles, err := getChangedFiles(opts, scm) if err != nil { return nil, err @@ -111,16 +111,12 @@ func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, } } } - if !quiet { - tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) - } + } else if changedPackages.Len() > 0 { // --since was specified, there are changes, but no scope was specified. // Run the packages that have changed filteredPkgs = changedPackages - if !quiet { - tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) - } + } else if scopePkgs.Len() > 0 { // There was either a global change, or no changes, or no --since flag // There was a --scope flag, run the desired scopes diff --git a/cli/internal/scope/scope_test.go b/cli/internal/scope/scope_test.go index a29e5f8ea6250..7244cfa939f1c 100644 --- a/cli/internal/scope/scope_test.go +++ b/cli/internal/scope/scope_test.go @@ -261,7 +261,7 @@ func TestResolvePackages(t *testing.T) { PackageNames: packageNames, TopologicalGraph: graph, SCC: scc, - }, tui, logger, false) + }, tui, logger) if err != nil { t.Errorf("expected no error, got %v", err) } From 19a366cb08ad853cb3a3f76526cc24fc10ab8ec6 Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Thu, 17 Mar 2022 10:17:20 -0400 Subject: [PATCH 3/5] Revert "Remove Ui side effects from ResolvePackages" This reverts commit aa551288d181353998f55c528a60da1d1eb1f67e. --- cli/internal/run/run.go | 10 ++-------- cli/internal/scope/scope.go | 10 +++++++--- cli/internal/scope/scope_test.go | 2 +- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index dcc66c0f5869a..58e63f04786ac 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -178,11 +178,10 @@ func (c *RunCommand) Run(args []string) int { return 1 } } - filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), scmInstance, ctx, c.Ui, c.Config.Logger) + filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), scmInstance, ctx, c.Ui, c.Config.Logger, runOptions.dryRunJson || runOptions.dryRun) if err != nil { c.logError(c.Config.Logger, "", fmt.Errorf("failed resolve packages to run %v", err)) } - c.Config.Logger.Debug("global hash", "value", ctx.GlobalHash) c.Config.Logger.Debug("local cache folder", "path", runOptions.cacheFolder) fs.EnsureDir(runOptions.cacheFolder) @@ -330,12 +329,7 @@ func (c *RunCommand) runOperation(g *completeGraph, rs *runSpec, backend *api.La } else { packagesInScope := rs.FilteredPkgs.UnsafeListOfStrings() sort.Strings(packagesInScope) - prettyPackagesInScope := strings.Join(packagesInScope, ", ") - if rs.Opts.since != "" { - c.Ui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), rs.Opts.since, prettyPackagesInScope)) - } else { - c.Ui.Output(fmt.Sprintf(ui.Dim("• Packages in scope: %v"), prettyPackagesInScope)) - } + c.Ui.Output(fmt.Sprintf(ui.Dim("• Packages in scope: %v"), strings.Join(packagesInScope, ", "))) if rs.Opts.stream { c.Ui.Output(fmt.Sprintf("%s %s %s", ui.Dim("• Running"), ui.Dim(ui.Bold(strings.Join(rs.Targets, ", "))), ui.Dim(fmt.Sprintf("in %v packages", rs.FilteredPkgs.Len())))) } diff --git a/cli/internal/scope/scope.go b/cli/internal/scope/scope.go index fad604e777b03..fe1f02b5580a7 100644 --- a/cli/internal/scope/scope.go +++ b/cli/internal/scope/scope.go @@ -25,7 +25,7 @@ type Opts struct { GlobalDepPatterns []string } -func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, logger hclog.Logger) (util.Set, error) { +func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, logger hclog.Logger, quiet bool) (util.Set, error) { changedFiles, err := getChangedFiles(opts, scm) if err != nil { return nil, err @@ -111,12 +111,16 @@ func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, } } } - + if !quiet { + tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) + } } else if changedPackages.Len() > 0 { // --since was specified, there are changes, but no scope was specified. // Run the packages that have changed filteredPkgs = changedPackages - + if !quiet { + tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) + } } else if scopePkgs.Len() > 0 { // There was either a global change, or no changes, or no --since flag // There was a --scope flag, run the desired scopes diff --git a/cli/internal/scope/scope_test.go b/cli/internal/scope/scope_test.go index 7244cfa939f1c..a29e5f8ea6250 100644 --- a/cli/internal/scope/scope_test.go +++ b/cli/internal/scope/scope_test.go @@ -261,7 +261,7 @@ func TestResolvePackages(t *testing.T) { PackageNames: packageNames, TopologicalGraph: graph, SCC: scc, - }, tui, logger) + }, tui, logger, false) if err != nil { t.Errorf("expected no error, got %v", err) } From 1972c1f650754a0fabf59362eec7e9d39defce27 Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Thu, 17 Mar 2022 12:13:45 -0400 Subject: [PATCH 4/5] Remove quiet flag and repetitive output all together --- cli/internal/run/run.go | 2 +- cli/internal/scope/scope.go | 8 +------- cli/internal/scope/scope_test.go | 2 +- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 58e63f04786ac..1752b3ce1bb49 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -178,7 +178,7 @@ func (c *RunCommand) Run(args []string) int { return 1 } } - filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), scmInstance, ctx, c.Ui, c.Config.Logger, runOptions.dryRunJson || runOptions.dryRun) + filteredPkgs, err := scope.ResolvePackages(runOptions.ScopeOpts(), scmInstance, ctx, c.Ui, c.Config.Logger) if err != nil { c.logError(c.Config.Logger, "", fmt.Errorf("failed resolve packages to run %v", err)) } diff --git a/cli/internal/scope/scope.go b/cli/internal/scope/scope.go index fe1f02b5580a7..bd7ee9434e082 100644 --- a/cli/internal/scope/scope.go +++ b/cli/internal/scope/scope.go @@ -25,7 +25,7 @@ type Opts struct { GlobalDepPatterns []string } -func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, logger hclog.Logger, quiet bool) (util.Set, error) { +func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, logger hclog.Logger) (util.Set, error) { changedFiles, err := getChangedFiles(opts, scm) if err != nil { return nil, err @@ -111,16 +111,10 @@ func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui, } } } - if !quiet { - tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s in scope: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) - } } else if changedPackages.Len() > 0 { // --since was specified, there are changes, but no scope was specified. // Run the packages that have changed filteredPkgs = changedPackages - if !quiet { - tui.Output(fmt.Sprintf(ui.Dim("• Packages changed since %s: %s"), opts.Since, strings.Join(filteredPkgs.UnsafeListOfStrings(), ", "))) - } } else if scopePkgs.Len() > 0 { // There was either a global change, or no changes, or no --since flag // There was a --scope flag, run the desired scopes diff --git a/cli/internal/scope/scope_test.go b/cli/internal/scope/scope_test.go index a29e5f8ea6250..7244cfa939f1c 100644 --- a/cli/internal/scope/scope_test.go +++ b/cli/internal/scope/scope_test.go @@ -261,7 +261,7 @@ func TestResolvePackages(t *testing.T) { PackageNames: packageNames, TopologicalGraph: graph, SCC: scc, - }, tui, logger, false) + }, tui, logger) if err != nil { t.Errorf("expected no error, got %v", err) } From d5b77ca6b53c5610b5672e53335952ffc7faf173 Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Thu, 17 Mar 2022 12:19:48 -0400 Subject: [PATCH 5/5] Fix e2e tests for new output --- cli/scripts/e2e/e2e.ts | 27 ++++++--------------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/cli/scripts/e2e/e2e.ts b/cli/scripts/e2e/e2e.ts index d8b3d79733cb1..c766047f23b44 100644 --- a/cli/scripts/e2e/e2e.ts +++ b/cli/scripts/e2e/e2e.ts @@ -103,22 +103,17 @@ function runSmokeTests( ) ); assert.fixture( - `• Packages changed since main: a`, sinceCommandOutputNoCache[0], - "Calculates changed packages (--since)" - ); - assert.fixture( `• Packages in scope: a`, - sinceCommandOutputNoCache[1], "Packages in scope" ); assert.fixture( + sinceCommandOutputNoCache[1], `• Running test in 1 packages`, - sinceCommandOutputNoCache[2], "Runs only in changed packages" ); assert.fixture( - sinceCommandOutputNoCache[3], + sinceCommandOutputNoCache[2], `a:test: cache miss, executing ${getHashFromOutput( sinceCommandOutputNoCache, "a#test" @@ -131,22 +126,17 @@ function runSmokeTests( ); assert.fixture( - `• Packages changed since main: a`, sinceCommandOutput[0], - "Calculates changed packages (--since)" - ); - assert.fixture( `• Packages in scope: a`, - sinceCommandOutput[1], "Packages in scope" ); assert.fixture( + sinceCommandOutput[1], `• Running test in 1 packages`, - sinceCommandOutput[2], "Runs only in changed packages" ); assert.fixture( - sinceCommandOutput[3], + sinceCommandOutput[2], `a:test: cache miss, executing ${getHashFromOutput( sinceCommandOutput, "a#test" @@ -159,23 +149,18 @@ function runSmokeTests( repo.turbo("run", ["test", "--since=main", "--stream"], options) ); assert.equal( - `• Packages changed since main: a`, sinceCommandSecondRunOutput[0], - "Calculates changed packages (--since) after a second run" - ); - assert.equal( `• Packages in scope: a`, - sinceCommandSecondRunOutput[1], "Packages in scope after a second run" ); assert.equal( + sinceCommandSecondRunOutput[1], `• Running test in 1 packages`, - sinceCommandSecondRunOutput[2], "Runs only in changed packages after a second run" ); assert.fixture( - sinceCommandSecondRunOutput[3], + sinceCommandSecondRunOutput[2], `a:test: cache hit, replaying output ${getHashFromOutput( sinceCommandSecondRunOutput, "a#test"