mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-02 19:17:10 +00:00
fix(analyze): refuse to compare attributed and unattributed denominators
One arm's actions may include the login the spec's setup drove and the other's cannot, so a per-action rate over the two divides by different things and the tests rank the bookkeeping.
This commit is contained in:
1 parent
67c9de6365
commit
628eb1949a
2 files changed
+88
No files matched your search
@@ -29,6 +29,7 @@ type armSummary struct {
|
||||
ViolationRate *float64 `json:"violation_rate"`
|
||||
TotalSteps int `json:"total_steps"`
|
||||
TotalActions int `json:"total_actions"`
|
||||
UnattributedActions int `json:"unattributed_actions"`
|
||||
TotalRunHours float64 `json:"total_run_hours"`
|
||||
Detections int `json:"detections"`
|
||||
DefectsPerThousandActions *float64 `json:"defects_per_thousand_actions"`
|
||||
@@ -127,6 +128,9 @@ func baseAnalysis(arms []arm, now time.Time) (analysis, []arm, error) {
|
||||
if err := sameBudget(testable); err != nil {
|
||||
return analysis{}, nil, err
|
||||
}
|
||||
if err := sameAttribution(testable); err != nil {
|
||||
return analysis{}, nil, err
|
||||
}
|
||||
if len(testable) >= 2 {
|
||||
names := make([]string, len(testable))
|
||||
groups := make([][]observation, len(testable))
|
||||
@@ -157,6 +161,28 @@ func sameBudget(arms []arm) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// sameAttribution refuses arms whose actions were counted against different
|
||||
// denominators. An arm recorded before an action named its producer counts
|
||||
// whatever the spec's setup dispatched among its actions, and an arm recorded
|
||||
// after leaves the login out, so the same rate over the two divides by
|
||||
// different things and the tests rank a bookkeeping difference. Two arms of the
|
||||
// same unknown provenance are diluted alike and compare; one of each does not.
|
||||
func sameAttribution(arms []arm) error {
|
||||
for index := 1; index < len(arms); index++ {
|
||||
unknown, attributed := arms[0], arms[index]
|
||||
if (unknown.unattributedActions() == 0) == (attributed.unattributedActions() == 0) {
|
||||
continue
|
||||
}
|
||||
if unknown.unattributedActions() == 0 {
|
||||
unknown, attributed = attributed, unknown
|
||||
}
|
||||
return fmt.Errorf("arm %q counts %d action(s) of unknown provenance and arm %q counts none: "+
|
||||
"a denominator that may include the spec's setup cannot be compared against one that excludes it",
|
||||
unknown.Name, unknown.unattributedActions(), attributed.Name)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func countCorrected(pairs []pairwiseResult) int {
|
||||
corrected := 0
|
||||
for _, pair := range pairs {
|
||||
@@ -212,6 +238,7 @@ func summarize(current arm) armSummary {
|
||||
Directories: current.Directories,
|
||||
Recorded: len(current.Runs),
|
||||
MissingSeeds: current.MissingSeeds,
|
||||
UnattributedActions: current.unattributedActions(),
|
||||
}
|
||||
runsPerDefect := map[string]int{}
|
||||
for _, item := range current.Runs {
|
||||
|
||||
@@ -352,6 +352,67 @@ func TestRun_RefusesToCompareArmsCensoredAtDifferentBudgets(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func writeSourcedCampaign(t *testing.T, directory, name string, steps, unattributed int) {
|
||||
t.Helper()
|
||||
const budget, runs, actions = 40, 6, 20
|
||||
seeds := make([]int, 0, runs)
|
||||
records := make([]map[string]any, 0, runs)
|
||||
for seed := 1; seed <= runs; seed++ {
|
||||
seeds = append(seeds, seed)
|
||||
records = append(records, map[string]any{
|
||||
"seed": seed, "exit_code": 0, "steps": steps, "actions": actions,
|
||||
"monotonic_millis": 60_000, "unattributed_actions": unattributed,
|
||||
})
|
||||
}
|
||||
writeCampaign(t, directory, map[string]any{"arm": name, "max_steps": budget, "seeds": seeds}, records)
|
||||
}
|
||||
|
||||
// An arm whose actions name no producer counts whatever the spec's setup
|
||||
// dispatched in the denominator of every per-action rate, and an arm whose
|
||||
// actions name one leaves the login out of it. The two denominators measure
|
||||
// different things, so a test that ranks one arm against the other reads a
|
||||
// difference in what was counted as a difference in what the arms found.
|
||||
func TestRun_RefusesToCompareArmsWhoseActionsWereCountedDifferently(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
arguments []string
|
||||
}{
|
||||
{name: "independent samples"},
|
||||
{name: "paired", arguments: []string{"--paired"}},
|
||||
}
|
||||
for _, test := range cases {
|
||||
root := t.TempDir()
|
||||
attributed := filepath.Join(root, "attributed")
|
||||
legacy := filepath.Join(root, "legacy")
|
||||
writeSourcedCampaign(t, attributed, "attributed", 40, 0)
|
||||
writeSourcedCampaign(t, legacy, "legacy", 30, 20)
|
||||
|
||||
err := run(append(test.arguments, attributed, legacy), io.Discard, io.Discard)
|
||||
if err == nil {
|
||||
t.Fatalf("%s: an arm of unknown provenance was tested against an attributed one without complaint", test.name)
|
||||
}
|
||||
for _, fragment := range []string{"attributed", "legacy", "120", "unknown provenance"} {
|
||||
if !strings.Contains(err.Error(), fragment) {
|
||||
t.Errorf("%s: error %q is missing %q", test.name, err, fragment)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Two arms recorded before actions named their producer are on the same
|
||||
// denominator as each other, diluted the same way, so they compare.
|
||||
func TestRun_ComparesTwoArmsThatBothNameNoProducer(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
first := filepath.Join(root, "first")
|
||||
second := filepath.Join(root, "second")
|
||||
writeSourcedCampaign(t, first, "first", 40, 20)
|
||||
writeSourcedCampaign(t, second, "second", 30, 20)
|
||||
|
||||
if err := run([]string{first, second}, io.Discard, io.Discard); err != nil {
|
||||
t.Fatalf("two arms of the same unknown provenance were refused: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func manyRuns(count, originStep int) []classifiedRun {
|
||||
runs := make([]classifiedRun, 0, count)
|
||||
for index := 0; index < count; index++ {
|
||||
|
||||
Reference in new issue
Block a user