diff --git a/cmd/internal-tools/analyze/end_to_end_test.go b/cmd/internal-tools/analyze/end_to_end_test.go index effe935..9278e42 100644 --- a/cmd/internal-tools/analyze/end_to_end_test.go +++ b/cmd/internal-tools/analyze/end_to_end_test.go @@ -258,3 +258,55 @@ func TestRun_RequiresACampaignDirectory(t *testing.T) { t.Fatal("expected an error with no campaign directories") } } + +// A campaign recorded before actions named their producer cannot say whether +// the login taps of the spec's setup are inside its per-action denominator, and +// the report has to say so where that denominator is read rather than leave the +// reader to date the file. +func TestRun_MarksAnArmWhoseActionsNameNoProducer(t *testing.T) { + root := t.TempDir() + directory := filepath.Join(root, "before-source") + buildFixtureCampaign(t, directory, "before-source", 30, []map[string]any{ + {"seed": 1, "exit_code": 0, "steps": 30, "actions": 28, "duration_millis": 60000}, + {"seed": 2, "exit_code": 0, "steps": 9, "actions": 9, "duration_millis": 60000, + "first_violation_origin_step": 9, "violated_properties": []string{"cartTotalMatches"}}, + }) + + var stdout bytes.Buffer + if err := run([]string{"--json", "-", directory}, &stdout, io.Discard); err != nil { + t.Fatal(err) + } + text := stdout.String() + if !strings.Contains(text, "37 (37 unattributed)") { + t.Errorf("the actions cell does not carry the unattributed count\n%s", text) + } + if !strings.Contains(text, "unknown provenance") { + t.Errorf("the report does not say the denominator's provenance is unknown\n%s", text) + } + + var result analysis + if err := json.Unmarshal([]byte(text[strings.Index(text, "{"):]), &result); err != nil { + t.Fatalf("json: %v", err) + } + if len(result.Arms) != 1 || result.Arms[0].UnattributedActions != 37 { + t.Errorf("arms %+v, want 37 unattributed actions", result.Arms) + } +} + +func TestRun_LeavesAnArmWhoseActionsAllNameAProducerUnmarked(t *testing.T) { + root := t.TempDir() + directory := filepath.Join(root, "sourced") + buildFixtureCampaign(t, directory, "sourced", 30, []map[string]any{ + {"seed": 1, "exit_code": 0, "steps": 30, "actions": 28, "duration_millis": 60000, "unattributed_actions": 0}, + {"seed": 2, "exit_code": 0, "steps": 9, "actions": 9, "duration_millis": 60000, "unattributed_actions": 0, + "first_violation_origin_step": 9, "violated_properties": []string{"cartTotalMatches"}}, + }) + + var stdout bytes.Buffer + if err := run([]string{directory}, &stdout, io.Discard); err != nil { + t.Fatal(err) + } + if strings.Contains(stdout.String(), "unattributed") || strings.Contains(stdout.String(), "unknown provenance") { + t.Errorf("an arm whose every action names a producer was marked\n%s", stdout.String()) + } +} diff --git a/cmd/internal-tools/analyze/report.go b/cmd/internal-tools/analyze/report.go index dfa6773..c47d5aa 100644 --- a/cmd/internal-tools/analyze/report.go +++ b/cmd/internal-tools/analyze/report.go @@ -42,7 +42,7 @@ func writeReport(result analysis, out io.Writer) { add( summary.Arm, strconv.Itoa(summary.TotalSteps), - strconv.Itoa(summary.TotalActions), + formatActions(summary), fmt.Sprintf("%.2f", summary.TotalRunHours), strconv.Itoa(summary.Detections), formatRatio(summary.DefectsPerThousandActions, 2), @@ -64,6 +64,13 @@ func writeReport(result analysis, out io.Writer) { fmt.Fprintf(out, "\n%s excluded %d run(s) as missing data, not as censored observations: %s", summary.Arm, summary.Excluded, strings.Join(parts, ", ")) } + for _, summary := range result.Arms { + if summary.UnattributedActions > 0 { + fmt.Fprintf(out, "\n%s counts %d action(s) of unknown provenance, recorded before an action named its producer: "+ + "its per-action denominator may include the login the spec's setup drove", + summary.Arm, summary.UnattributedActions) + } + } for _, summary := range result.Arms { if summary.EventsHeldAtBudget > 0 { fmt.Fprintf(out, "\n%s held %d violation(s) reported past the budget at %d steps", @@ -168,6 +175,16 @@ func formatMedian(value *float64) string { return strconv.FormatFloat(*value, 'f', -1, 64) } +// formatActions marks a denominator with actions whose producer nothing names, +// because the rate beside it then divides by a count that may include the +// login the spec's setup drove. +func formatActions(summary armSummary) string { + if summary.UnattributedActions == 0 { + return strconv.Itoa(summary.TotalActions) + } + return fmt.Sprintf("%d (%d unattributed)", summary.TotalActions, summary.UnattributedActions) +} + func formatRatio(value *float64, digits int) string { if value == nil { return "n/a"