Skip to content

Commit

Permalink
Filter out summaries of tasks that didn't execute (#4238)
Browse files Browse the repository at this point in the history
We don't want to exclude tasks entirely from the task graph,
because they could be used as virtual wrappers to trigger
downstream tasks (via dependsOn), but we can filter them
out of the task summary to reduce noise. Another reason to
do this, is that the task summary is initialized in "building"
phase, which is a bit presumptuous, and also wrong for tasks
that end up being a no-op because they aren't implemented.
  • Loading branch information
mehulkar authored Mar 17, 2023
1 parent a362571 commit ec3e32e
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 15 deletions.
4 changes: 2 additions & 2 deletions cli/integration_tests/basic_monorepo/cache_state.t
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ Setup

Run a build to get a local cache.
$ ${TURBO} run build --output-logs=none
\xe2\x80\xa2 Packages in scope: my-app, util (esc)
\xe2\x80\xa2 Running build in 2 packages (esc)
\xe2\x80\xa2 Packages in scope: another, my-app, util (esc)
\xe2\x80\xa2 Running build in 3 packages (esc)
\xe2\x80\xa2 Remote caching disabled (esc)

Tasks: 2 successful, 2 total
Expand Down
7 changes: 4 additions & 3 deletions cli/integration_tests/basic_monorepo/dry_run.t
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ Setup
# The first part of the file is Packages in Scope
$ cat tmp-1.txt
Packages in Scope
Name Path
my-app apps/my-app
util packages/util
Name Path
another packages/another
my-app apps/my-app
util packages/util

# Part 2 of the logs are Global Hash INputs
$ cat tmp-2.txt
Expand Down
2 changes: 2 additions & 0 deletions cli/integration_tests/basic_monorepo/infer_pkg.t
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Setup
Run a dry run
$ ${TURBO} build --dry=json | jq .packages
[
"another",
"my-app",
"util"
]
Expand All @@ -19,6 +20,7 @@ Run a dry run in a directory
Ensure we don't infer packages if --cwd is supplied
$ ${TURBO} build --cwd=../.. --dry=json | jq .packages
[
"another",
"my-app",
"util"
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "another",
"scripts": {}
}
4 changes: 2 additions & 2 deletions cli/integration_tests/basic_monorepo/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ Setup

# Bad command
$ ${TURBO} run something
\xe2\x80\xa2 Packages in scope: //, my-app, util (esc)
\xe2\x80\xa2 Running something in 3 packages (esc)
\xe2\x80\xa2 Packages in scope: //, another, my-app, util (esc)
\xe2\x80\xa2 Running something in 4 packages (esc)
\xe2\x80\xa2 Remote caching disabled (esc)
root task something (turbo run build) looks like it invokes turbo and might cause a loop

Expand Down
3 changes: 3 additions & 0 deletions cli/integration_tests/basic_monorepo/run_summary.t
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ Setup
"apps/my-app/.turbo/turbo-build.log"
]

$ cat $(/bin/ls .turbo/runs/*.json | head -n1) | jq '.tasks | map(select(.taskId == "another#build"))'
[]

# Without env var, no summary file is generated
$ rm -rf .turbo/runs
$ ${TURBO} run build > /dev/null
Expand Down
25 changes: 17 additions & 8 deletions cli/internal/run/real_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,18 +94,26 @@ func RealRun(
taskSummaries := []*runsummary.TaskSummary{}
execFunc := func(ctx gocontext.Context, packageTask *nodes.PackageTask, taskSummary *runsummary.TaskSummary) error {
deps := engine.TaskGraph.DownEdges(packageTask.TaskID)
mu.Lock()
taskSummaries = append(taskSummaries, taskSummary)
// don't hold the lock while we run ec.exec
mu.Unlock()

// deps here are passed in to calculate the task hash
taskExecutionSummary, err := ec.exec(ctx, packageTask, deps)
if err != nil {
return err
}
taskSummary.Execution = taskExecutionSummary
taskSummary.ExpandedOutputs = taskHashTracker.GetExpandedOutputs(taskSummary.TaskID)

// taskExecutionSummary will be nil if the task never executed
// (i.e. if the workspace didn't implement the script corresponding to the task)
// We don't need to collect any of the outputs or execution if the task didn't execute.
if taskExecutionSummary != nil {
taskSummary.ExpandedOutputs = taskHashTracker.GetExpandedOutputs(taskSummary.TaskID)
taskSummary.Execution = taskExecutionSummary

// lock since multiple things to be appending to this array at the same time
mu.Lock()
taskSummaries = append(taskSummaries, taskSummary)
// not using defer, just release the lock
mu.Unlock()
}

return nil
}

Expand Down Expand Up @@ -196,7 +204,8 @@ func (ec *execContext) exec(ctx gocontext.Context, packageTask *nodes.PackageTas
if packageTask.Command == "" {
progressLogger.Debug("no task in package, skipping")
progressLogger.Debug("done", "status", "skipped", "duration", time.Since(cmdTime))
return taskExecutionSummary, nil
// Return nil here because there was no execution, so there is no task execution summary
return nil, nil
}

var prefix string
Expand Down

0 comments on commit ec3e32e

Please sign in to comment.