Skip to content

Commit

Permalink
Gather Run Summary even when tasks fail (#4524)
Browse files Browse the repository at this point in the history
  • Loading branch information
mehulkar committed Apr 11, 2023
1 parent 98da31e commit bb9ff6f
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 45 deletions.
114 changes: 114 additions & 0 deletions cli/integration_tests/run_summary/error.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
Setup
$ . ${TESTDIR}/../_helpers/setup.sh
$ . ${TESTDIR}/../_helpers/setup_monorepo.sh $(pwd)

$ rm -rf .turbo/runs

Use --filter because otherwise we'll get nondeterministic execution summary depending on
whether the other workspaces had finished executing their task yet. We don't care to validate
that behavior in this test.
$ ${TURBO} run maybefails --filter=my-app --summarize > /dev/null 2>&1
[1]

$ source "$TESTDIR/../_helpers/run-summary-utils.sh"
$ SUMMARY=$(/bin/ls .turbo/runs/*.json | head -n1)

Validate that there was a failed task and exitCode is 1 (which is what we get from npm for the failed task)
$ cat $SUMMARY | jq '.execution'
{
"command": "turbo run maybefails --filter=my-app",
"repoPath": "",
"success": 0,
"failed": 1,
"cached": 0,
"attempted": 1,
"startTime": [0-9]+, (re)
"endTime": [0-9]+, (re)
"exitCode": 1
}

Validate that we got a full task summary for the failed task with an error in .execution
$ echo $(getSummaryTaskId $SUMMARY "my-app#maybefails") | jq
{
"taskId": "my-app#maybefails",
"task": "maybefails",
"package": "my-app",
"hash": "c11710f9379cb7ea",
"inputs": {
"package.json": "6bcf57fd6ff30d1a6f40ad8d8d08e8b940fc7e3b"
},
"hashOfExternalDependencies": "ccab0b28617f1f56",
"cache": {
"local": false,
"remote": false
},
"command": "exit 4",
"cliArguments": [],
"outputs": null,
"excludedOutputs": null,
"logFile": "apps/my-app/.turbo/turbo-maybefails.log",
"directory": "apps/my-app",
"dependencies": [],
"dependents": [],
"resolvedTaskDefinition": {
"outputs": [],
"cache": true,
"dependsOn": [],
"inputs": [],
"outputMode": "full",
"env": [],
"persistent": false
},
"expandedOutputs": [],
"framework": "",
"environmentVariables": {
"configured": [],
"inferred": [],
"global": [
"SOME_ENV_VAR=",
"VERCEL_ANALYTICS_ID="
]
},
"execution": {
"startTime": [0-9]+, (re)
"endTime": [0-9]+, (re)
"error": "command .* npm run maybefails exited \(1\)", (re)
"exitCode": 1
}
}

With --continue flag. We won't validate the whole thing, just execution
Use --force flag so we can be deterministic about cache hits
Don't use --filter here, so we can validate that both tasks attempted to run
$ rm -rf .turbo/runs
$ ${TURBO} run maybefails --summarize --force --continue > /dev/null 2>&1
[1]

$ source "$TESTDIR/../_helpers/run-summary-utils.sh"
$ SUMMARY=$(/bin/ls .turbo/runs/*.json | head -n1)

success should be 1, and attempted should be 2
$ cat $SUMMARY | jq '.execution'
{
"command": "turbo run maybefails --continue",
"repoPath": "",
"success": 1,
"failed": 1,
"cached": 0,
"attempted": 2,
"startTime": [0-9]+, (re)
"endTime": [0-9]+, (re)
"exitCode": 1
}

$ cat $SUMMARY | jq '.tasks | length'
2

# exitCode is 1, because npm will report all errors with exitCode 1
$ getSummaryTaskId $SUMMARY "my-app#maybefails" | jq '.execution'
{
"startTime": [0-9]+, (re)
"endTime": [0-9]+, (re)
"error": "command .* npm run maybefails exited \(1\)", (re)
"exitCode": 1
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ Setup
{
"startTime": [0-9]+, (re)
"endTime": [0-9]+, (re)
"error": null,
"exitCode": 0
}
$ echo $FIRST_APP_BUILD | jq '.cliArguments'
Expand Down Expand Up @@ -89,7 +88,6 @@ Setup
{
"startTime": [0-9]+, (re)
"endTime": [0-9]+, (re)
"error": null,
"exitCode": 0
}

Expand Down
30 changes: 0 additions & 30 deletions cli/integration_tests/run_summary/run_summary_fail.t

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ Check
{
"startTime": [0-9]+, (re)
"endTime": [0-9]+, (re)
"error": null,
"exitCode": 0
}
$ echo $TASK_SUMMARY | jq '.cliArguments'
Expand Down
4 changes: 2 additions & 2 deletions cli/internal/graph/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (g *CompleteGraph) GetPackageTaskVisitor(
taskGraph *dag.AcyclicGraph,
getArgs func(taskID string) []string,
logger hclog.Logger,
visitor func(ctx gocontext.Context, packageTask *nodes.PackageTask, taskSummary *runsummary.TaskSummary) error,
execFunc func(ctx gocontext.Context, packageTask *nodes.PackageTask, taskSummary *runsummary.TaskSummary) error,
) func(taskID string) error {
return func(taskID string) error {
packageName, taskName := util.GetPackageTaskFromId(taskID)
Expand Down Expand Up @@ -140,7 +140,7 @@ func (g *CompleteGraph) GetPackageTaskVisitor(
summary.Dependents = descendents
}

return visitor(ctx, packageTask, summary)
return execFunc(ctx, packageTask, summary)
}
}

Expand Down
8 changes: 5 additions & 3 deletions cli/internal/run/real_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,6 @@ func RealRun(
taskSummaries := []*runsummary.TaskSummary{}
execFunc := func(ctx gocontext.Context, packageTask *nodes.PackageTask, taskSummary *runsummary.TaskSummary) error {
taskExecutionSummary, err := ec.exec(ctx, packageTask)
if err != nil {
return err
}

// taskExecutionSummary will be nil if the task never executed
// (i.e. if the workspace didn't implement the script corresponding to the task)
Expand All @@ -117,6 +114,11 @@ func RealRun(
mu.Unlock()
}

// Return the error when there is one
if err != nil {
return err
}

return nil
}

Expand Down
15 changes: 8 additions & 7 deletions cli/internal/runsummary/execution_summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type executionEvent struct {
// Its current status
Status executionEventName
// Error, only populated for failure statuses
Err error
Err string

exitCode *int
}
Expand Down Expand Up @@ -68,7 +68,7 @@ func (en executionEventName) toString() string {
type TaskExecutionSummary struct {
startAt time.Time // set once
status executionEventName // current status, updated during execution
err error // only populated for failure statuses
err string // only populated for failure statuses
Duration time.Duration // updated during the task execution
exitCode *int // pointer so we can distinguish between 0 and unknown.
}
Expand All @@ -81,10 +81,10 @@ func (ts *TaskExecutionSummary) endTime() time.Time {
// We'll use an anonmyous, private struct for this, so it's not confusingly duplicated
func (ts *TaskExecutionSummary) MarshalJSON() ([]byte, error) {
serializable := struct {
Start int64 `json:"startTime"`
End int64 `json:"endTime"`
Err error `json:"error"`
ExitCode *int `json:"exitCode"`
Start int64 `json:"startTime"`
End int64 `json:"endTime"`
Err string `json:"error,omitempty"`
ExitCode *int `json:"exitCode"`
}{
Start: ts.startAt.UnixMilli(),
End: ts.endTime().UnixMilli(),
Expand Down Expand Up @@ -197,8 +197,9 @@ func (es *executionSummary) run(taskID string) (func(outcome executionEventName,
// when we assign it to the taskExecutionSummary.
exitCode: exitCode,
}

if err != nil {
result.Err = fmt.Errorf("running %v failed: %w", taskID, err)
result.Err = err.Error()
}

// Ignore the return value here
Expand Down

0 comments on commit bb9ff6f

Please sign in to comment.