Skip to content

Commit 8ebec37

Browse files
121watts121watts
committed
fix(ci): avoid repeated diagnose commands
1 parent 180881c commit 8ebec37

2 files changed

Lines changed: 229 additions & 21 deletions

File tree

‎pkg/cmd/ci/diagnose.go‎

Lines changed: 165 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ import (
1515

1616
var ciDiagnose = api.CIGetFailureDiagnosis
1717

18+
const (
19+
diagnoseTextWidth = 96
20+
maxGroupEvidenceLines = 5
21+
)
22+
1823
func NewCmdDiagnose() *cobra.Command {
1924
var (
2025
orgID string
@@ -188,7 +193,7 @@ func printDiagnosisContext(w io.Writer, context *civ1.FailureDiagnosisContext) {
188193
if context.GetRef() != "" {
189194
fmt.Fprintf(w, " @ %s", context.GetRef())
190195
}
191-
if context.GetSha() != "" {
196+
if context.GetSha() != "" && context.GetSha() != context.GetRef() {
192197
fmt.Fprintf(w, " (%s)", context.GetSha())
193198
}
194199
fmt.Fprintln(w)
@@ -289,7 +294,6 @@ func printFocusedDiagnosis(w io.Writer, resp *civ1.FailureDiagnosis, renderer di
289294
printGroupedDiagnosis(w, resp, renderer)
290295
return
291296
}
292-
printNextCommands(w, renderer.commands(resp.GetNextCommands(), resp.GetCommandCapabilities(), true), "Next commands")
293297
printSummaryUnavailableNote(w, resp.GetCommandCapabilities())
294298
printBoundsSummary(w, resp)
295299
}
@@ -307,27 +311,32 @@ func printGroupedDiagnosis(w io.Writer, resp *civ1.FailureDiagnosis, renderer di
307311
fmt.Fprintf(w, "Failure groups: %d\n", len(resp.GetFailureGroups()))
308312
}
309313
for i, group := range resp.GetFailureGroups() {
310-
fmt.Fprintf(w, "\nGroup %d: %d failure", i+1, group.GetCount())
311-
if group.GetCount() != 1 {
312-
fmt.Fprint(w, "s")
314+
fmt.Fprintf(w, "\nGroup %d: %s\n", i+1, firstNonEmpty(group.GetErrorMessage(), "failure group"))
315+
fmt.Fprintf(w, " %d %s\n", group.GetCount(), pluralize("failure", int(group.GetCount())))
316+
if group.GetErrorMessageTruncated() {
317+
fmt.Fprintf(w, " Error truncated%s\n", truncatedSuffix(true, group.GetErrorMessageOriginalLength()))
313318
}
314-
if group.GetSource() != "" {
315-
fmt.Fprintf(w, " from %s", group.GetSource())
316-
}
317-
fmt.Fprintln(w)
318-
if group.GetErrorMessage() != "" {
319-
fmt.Fprintf(w, " Error: %s%s\n", group.GetErrorMessage(), truncatedSuffix(group.GetErrorMessageTruncated(), group.GetErrorMessageOriginalLength()))
319+
representativeError := commonRepresentativeError(group)
320+
showRepresentativeErrors := true
321+
if representativeError != "" && representativeError != group.GetErrorMessage() {
322+
fmt.Fprintf(w, " Where: %s\n", representativeError)
323+
showRepresentativeErrors = false
324+
} else if representativeError == group.GetErrorMessage() {
325+
showRepresentativeErrors = false
320326
}
321327
if group.GetDiagnosis() != "" {
322-
fmt.Fprintf(w, " Diagnosis: %s\n", group.GetDiagnosis())
328+
fmt.Fprintln(w)
329+
printWrappedSection(w, "Diagnosis", group.GetDiagnosis(), " ")
323330
}
324331
if group.GetPossibleFix() != "" {
325-
fmt.Fprintf(w, " Possible fix: %s\n", group.GetPossibleFix())
332+
fmt.Fprintln(w)
333+
printWrappedSection(w, "Possible fix", group.GetPossibleFix(), " ")
326334
}
327335
if len(group.GetRepresentatives()) > 0 {
328-
fmt.Fprintln(w, " Representative attempts:")
336+
fmt.Fprintln(w)
337+
fmt.Fprintln(w, " Attempts:")
329338
for _, representative := range group.GetRepresentatives() {
330-
printRepresentativeAttempt(w, resp.GetOrgId(), representative, resp.GetCommandCapabilities(), renderer, " ")
339+
printCompactRepresentativeAttempt(w, resp.GetOrgId(), representative, resp.GetCommandCapabilities(), renderer, " ", showRepresentativeErrors)
331340
}
332341
}
333342
if group.GetOmittedRepresentativeCount() > 0 {
@@ -338,12 +347,49 @@ func printGroupedDiagnosis(w io.Writer, resp *civ1.FailureDiagnosis, renderer di
338347
len(group.GetRepresentatives())+int(group.GetOmittedRepresentativeCount()),
339348
)
340349
}
350+
printGroupEvidence(w, group, " ")
341351
}
342-
printNextCommands(w, renderer.commands(resp.GetNextCommands(), resp.GetCommandCapabilities(), true), "Next commands")
343352
printSummaryUnavailableNote(w, resp.GetCommandCapabilities())
344353
printBoundsSummary(w, resp)
345354
}
346355

356+
func printCompactRepresentativeAttempt(w io.Writer, orgID string, representative *civ1.RepresentativeAttempt, capabilities *civ1.FailureDiagnosisCommandCapabilities, renderer diagnosisCommandRenderer, indent string, showError bool) {
357+
fmt.Fprintf(w, "%s- #%d %s", indent, representative.GetAttempt(), representative.GetAttemptId())
358+
if representative.GetJobKey() != "" || representative.GetJobDisplayName() != "" {
359+
fmt.Fprintf(w, " %s", firstNonEmpty(representative.GetJobDisplayName(), representative.GetJobKey()))
360+
}
361+
if representative.GetAttemptStatus() != "" {
362+
fmt.Fprintf(w, " (%s)", representative.GetAttemptStatus())
363+
}
364+
fmt.Fprintln(w)
365+
if showError && representative.GetErrorMessage() != "" {
366+
fmt.Fprintf(w, "%s Error: %s%s\n", indent, representative.GetErrorMessage(), truncatedSuffix(representative.GetErrorMessageTruncated(), representative.GetErrorMessageOriginalLength()))
367+
}
368+
for _, command := range renderer.commands(representative.GetNextCommands(), capabilities, true) {
369+
fmt.Fprintf(w, "%s %s: %s\n", indent, firstNonEmpty(command.Label, "Command"), command.Command)
370+
}
371+
if orgID != "" && representative.GetWorkflowId() != "" && representative.GetJobId() != "" && representative.GetAttemptId() != "" {
372+
fmt.Fprintf(w, "%s View: %s\n", indent, statusAttemptViewURL(orgID, representative.GetWorkflowId(), representative.GetJobId(), representative.GetAttemptId()))
373+
}
374+
}
375+
376+
func commonRepresentativeError(group *civ1.FailureGroup) string {
377+
var common string
378+
for _, representative := range group.GetRepresentatives() {
379+
if representative.GetErrorMessage() == "" {
380+
continue
381+
}
382+
if common == "" {
383+
common = representative.GetErrorMessage()
384+
continue
385+
}
386+
if representative.GetErrorMessage() != common {
387+
return ""
388+
}
389+
}
390+
return common
391+
}
392+
347393
func printRepresentativeAttempt(w io.Writer, orgID string, representative *civ1.RepresentativeAttempt, capabilities *civ1.FailureDiagnosisCommandCapabilities, renderer diagnosisCommandRenderer, indent string) {
348394
fmt.Fprintf(w, "%sAttempt #%d %s", indent, representative.GetAttempt(), representative.GetAttemptId())
349395
if representative.GetJobKey() != "" || representative.GetJobDisplayName() != "" {
@@ -380,6 +426,66 @@ func printRepresentativeAttempt(w io.Writer, orgID string, representative *civ1.
380426
}
381427
}
382428

429+
func printGroupEvidence(w io.Writer, group *civ1.FailureGroup, indent string) {
430+
type evidenceLine struct {
431+
prefix string
432+
content string
433+
truncated bool
434+
originalLength uint32
435+
}
436+
437+
seen := map[string]struct{}{}
438+
lines := make([]evidenceLine, 0, maxGroupEvidenceLines)
439+
for _, representative := range group.GetRepresentatives() {
440+
for _, line := range representative.GetRelevantLines() {
441+
if isGenericEvidenceLine(line.GetContent()) {
442+
continue
443+
}
444+
prefix := fmt.Sprintf("%d", line.GetLineNumber())
445+
if line.GetStepId() != "" {
446+
prefix = line.GetStepId() + ":" + prefix
447+
}
448+
key := normalizeEvidenceContent(line.GetContent())
449+
if _, ok := seen[key]; ok {
450+
continue
451+
}
452+
seen[key] = struct{}{}
453+
lines = append(lines, evidenceLine{
454+
prefix: prefix,
455+
content: line.GetContent(),
456+
truncated: line.GetContentTruncated(),
457+
originalLength: line.GetContentOriginalLength(),
458+
})
459+
if len(lines) == maxGroupEvidenceLines {
460+
break
461+
}
462+
}
463+
if len(lines) == maxGroupEvidenceLines {
464+
break
465+
}
466+
}
467+
if len(lines) == 0 {
468+
return
469+
}
470+
471+
fmt.Fprintln(w)
472+
fmt.Fprintf(w, "%sEvidence:\n", indent)
473+
for _, line := range lines {
474+
fmt.Fprintf(w, "%s - %s: %s%s\n", indent, line.prefix, line.content, truncatedSuffix(line.truncated, line.originalLength))
475+
}
476+
}
477+
478+
func normalizeEvidenceContent(content string) string {
479+
return strings.Join(strings.Fields(content), " ")
480+
}
481+
482+
func isGenericEvidenceLine(content string) bool {
483+
normalized := strings.ToLower(normalizeEvidenceContent(content))
484+
return strings.HasPrefix(normalized, "##[error]script exited with code ") ||
485+
strings.Contains(normalized, "err_pnpm_recursive_run_first_fail") ||
486+
strings.Contains(normalized, "elifecycle command failed")
487+
}
488+
383489
func printNextCommands(w io.Writer, commands []diagnoseCommandJSON, title string) {
384490
if len(commands) == 0 {
385491
return
@@ -431,6 +537,49 @@ func truncatedSuffix(truncated bool, originalLength uint32) string {
431537
return fmt.Sprintf(" (truncated from %d chars)", originalLength)
432538
}
433539

540+
func printWrappedSection(w io.Writer, title, text, indent string) {
541+
fmt.Fprintf(w, "%s%s:\n", indent, title)
542+
printWrappedText(w, text, indent+" ", diagnoseTextWidth)
543+
}
544+
545+
func printWrappedText(w io.Writer, text, indent string, width int) {
546+
available := width - len(indent)
547+
if available < 20 {
548+
available = 20
549+
}
550+
for _, paragraph := range strings.Split(text, "\n") {
551+
words := strings.Fields(paragraph)
552+
if len(words) == 0 {
553+
fmt.Fprintln(w, indent)
554+
continue
555+
}
556+
557+
line := ""
558+
for _, word := range words {
559+
if line == "" {
560+
line = word
561+
continue
562+
}
563+
if len(line)+1+len(word) > available {
564+
fmt.Fprintln(w, indent+line)
565+
line = word
566+
continue
567+
}
568+
line += " " + word
569+
}
570+
if line != "" {
571+
fmt.Fprintln(w, indent+line)
572+
}
573+
}
574+
}
575+
576+
func pluralize(singular string, count int) string {
577+
if count == 1 {
578+
return singular
579+
}
580+
return singular + "s"
581+
}
582+
434583
type diagnoseJSONDocument struct {
435584
OrgID string `json:"org_id"`
436585
State string `json:"state"`

‎pkg/cmd/ci/diagnose_test.go‎

Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,13 @@ func TestDiagnoseHumanGroupedOutputWithOrgQualifiedCommands(t *testing.T) {
4949
"Target: run run-1 (failed)",
5050
"Source: depot/cli @ refs/heads/main (abc123)",
5151
"Failure groups: 2 (1 omitted)",
52-
"Group 1: 3 failures from attempt_error",
53-
"Error: go test ./... failed",
54-
"Diagnosis: Unit tests failed in package pkg/cmd/ci.",
55-
"Possible fix: Fix the failing assertion and rerun tests.",
56-
"Attempt #2 att-1 for ci.yml:test (failed)",
52+
"Group 1: go test ./... failed",
53+
"3 failures",
54+
"Diagnosis:\n Unit tests failed in package pkg/cmd/ci.",
55+
"Possible fix:\n Fix the failing assertion and rerun tests.",
56+
"Attempts:",
57+
"- #2 att-1 ci.yml:test (failed)",
58+
"Evidence:",
5759
"build:42: expected true, got false",
5860
"Logs: depot ci logs att-1 --org org-123",
5961
"Summary: depot ci summary att-1 --org org-123",
@@ -66,6 +68,15 @@ func TestDiagnoseHumanGroupedOutputWithOrgQualifiedCommands(t *testing.T) {
6668
t.Fatalf("diagnose output missing %q:\n%s", want, stdout)
6769
}
6870
}
71+
if count := strings.Count(stdout, "Unit tests failed in package pkg/cmd/ci."); count != 1 {
72+
t.Fatalf("diagnosis rendered %d times, want group-level only:\n%s", count, stdout)
73+
}
74+
if count := strings.Count(stdout, "Fix the failing assertion and rerun tests."); count != 1 {
75+
t.Fatalf("possible fix rendered %d times, want group-level only:\n%s", count, stdout)
76+
}
77+
if count := strings.Count(stdout, "go test ./... failed"); count != 1 {
78+
t.Fatalf("group error rendered %d times, want group heading only:\n%s", count, stdout)
79+
}
6980
}
7081

7182
func TestDiagnoseRepresentativeSamplingDoesNotPrintGenericTruncationFooter(t *testing.T) {
@@ -128,6 +139,54 @@ func TestDiagnoseRepresentativeSamplingStillPrintsRealTruncationFooter(t *testin
128139
}
129140
}
130141

142+
func TestDiagnoseGroupedOutputDoesNotRepeatRepresentativeCommandsFooter(t *testing.T) {
143+
restoreDiagnoseAPI(t)
144+
145+
ciDiagnose = func(ctx context.Context, token, orgID string, req *civ1.GetFailureDiagnosisRequest) (*civ1.FailureDiagnosis, error) {
146+
resp := groupedDiagnosisResponse(true)
147+
resp.NextCommands = []*civ1.DrillDownCommand{logsCommand("att-1"), summaryCommand("att-1")}
148+
return resp, nil
149+
}
150+
151+
stdout, _, err := executeDiagnoseTextCommand([]string{"--org", "org-123", "--token", "token-123", "run-1"})
152+
if err != nil {
153+
t.Fatal(err)
154+
}
155+
if strings.Contains(stdout, "Next commands:") {
156+
t.Fatalf("grouped output repeated representative commands in footer:\n%s", stdout)
157+
}
158+
if count := strings.Count(stdout, "depot ci logs att-1 --org org-123"); count != 1 {
159+
t.Fatalf("logs command rendered %d times, want once:\n%s", count, stdout)
160+
}
161+
if count := strings.Count(stdout, "depot ci summary att-1 --org org-123"); count != 1 {
162+
t.Fatalf("summary command rendered %d times, want once:\n%s", count, stdout)
163+
}
164+
}
165+
166+
func TestDiagnoseFocusedOutputDoesNotRepeatRepresentativeCommandsFooter(t *testing.T) {
167+
restoreDiagnoseAPI(t)
168+
169+
ciDiagnose = func(ctx context.Context, token, orgID string, req *civ1.GetFailureDiagnosisRequest) (*civ1.FailureDiagnosis, error) {
170+
resp := focusedDiagnosisResponse(true)
171+
resp.NextCommands = []*civ1.DrillDownCommand{logsCommand("att-1"), summaryCommand("att-1")}
172+
return resp, nil
173+
}
174+
175+
stdout, _, err := executeDiagnoseTextCommand([]string{"--org", "org-123", "--token", "token-123", "--type", "attempt", "att-1"})
176+
if err != nil {
177+
t.Fatal(err)
178+
}
179+
if strings.Contains(stdout, "Next commands:") {
180+
t.Fatalf("focused output repeated representative commands in footer:\n%s", stdout)
181+
}
182+
if count := strings.Count(stdout, "depot ci logs att-1 --org org-123"); count != 1 {
183+
t.Fatalf("logs command rendered %d times, want once:\n%s", count, stdout)
184+
}
185+
if count := strings.Count(stdout, "depot ci summary att-1 --org org-123"); count != 1 {
186+
t.Fatalf("summary command rendered %d times, want once:\n%s", count, stdout)
187+
}
188+
}
189+
131190
func TestDiagnoseJSONOutputIsCLINormalized(t *testing.T) {
132191
restoreDiagnoseAPI(t)
133192

0 commit comments

Comments
 (0)