Skip to content

Commit 9687993

Browse files
authored
fix: flakeguard swallowing panic (#1866)
* fix: placeholder tests reproducing panic swallow * fix: test panic bug
1 parent 2021a44 commit 9687993

File tree

4 files changed

+1500
-36
lines changed

4 files changed

+1500
-36
lines changed

tools/flakeguard/runner/parser/parser.go

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,9 @@ type testProcessingState struct {
108108
temporaryOutputsByRunID map[string][]string // runID -> []string of normal output
109109
panicDetectionMode bool
110110
raceDetectionMode bool
111-
detectedEntries []entry // Raw entries collected during panic/race
112-
key string // Test key (pkg/TestName)
113-
filePath string // File path currently being processed (for logging)
111+
detectedEntries []rawEventData // Raw entries collected during panic/race
112+
key string // Test key (pkg/TestName)
113+
filePath string // File path currently being processed (for logging)
114114
}
115115

116116
// parseTestResults orchestrates the multi-pass parsing approach.
@@ -257,6 +257,20 @@ func (p *defaultParser) processEventsPerTest(eventsByTest map[string][]rawEventD
257257
p.processEvent(state, rawEv)
258258
}
259259

260+
// If after processing all events, and these are still true
261+
// it means there were no terminal actions (pass/fail/skip) after a panic or race.
262+
if state.panicDetectionMode || state.raceDetectionMode {
263+
firstDetected := state.detectedEntries[0]
264+
terminalEvent := entry{
265+
Action: "fail",
266+
Test: firstDetected.Event.Test,
267+
Package: firstDetected.Event.Package,
268+
Output: "",
269+
Elapsed: 0.0, // No elapsed time
270+
}
271+
p.handlePanicRaceTermination(state, terminalEvent, firstDetected.RunID)
272+
}
273+
260274
p.finalizeOutputs(state, cfg)
261275
result.Runs = len(state.processedRunIDs)
262276
processedTestDetails[key] = result
@@ -271,13 +285,28 @@ func (p *defaultParser) processEvent(state *testProcessingState, rawEv rawEventD
271285

272286
// 1. Handle Output / Panic/Race Start Detection
273287
if event.Output != "" {
274-
panicRaceStarted := p.handleOutputEvent(state, event, runID)
275-
if panicRaceStarted || state.panicDetectionMode || state.raceDetectionMode {
276-
if state.panicDetectionMode || state.raceDetectionMode {
277-
state.detectedEntries = append(state.detectedEntries, event)
278-
}
288+
// already detected panic or race, collect non-empty outputs
289+
if state.panicDetectionMode || state.raceDetectionMode {
290+
state.detectedEntries = append(state.detectedEntries, rawEv)
291+
return
292+
}
293+
294+
if panicStarted := startPanicRe.MatchString(event.Output); panicStarted {
295+
state.panicDetectionMode = true
296+
state.detectedEntries = append(state.detectedEntries, rawEv)
297+
return
298+
}
299+
300+
if raceStarted := startRaceRe.MatchString(event.Output); raceStarted {
301+
state.raceDetectionMode = true
302+
state.detectedEntries = append(state.detectedEntries, rawEv)
279303
return
280304
}
305+
306+
if state.temporaryOutputsByRunID[runID] == nil {
307+
state.temporaryOutputsByRunID[runID] = []string{}
308+
}
309+
state.temporaryOutputsByRunID[runID] = append(state.temporaryOutputsByRunID[runID], event.Output)
281310
}
282311

283312
// 2. Handle Panic/Race Termination
@@ -290,31 +319,6 @@ func (p *defaultParser) processEvent(state *testProcessingState, rawEv rawEventD
290319
}
291320
}
292321

293-
// handleOutputEvent handles output collection and panic/race start detection.
294-
// Returns true if panic/race mode started.
295-
func (p *defaultParser) handleOutputEvent(state *testProcessingState, event entry, runID string) (panicRaceStarted bool) {
296-
if state.panicDetectionMode || state.raceDetectionMode {
297-
return false
298-
}
299-
300-
if startPanicRe.MatchString(event.Output) {
301-
state.detectedEntries = append(state.detectedEntries, event)
302-
state.panicDetectionMode = true
303-
return true
304-
}
305-
if startRaceRe.MatchString(event.Output) {
306-
state.detectedEntries = append(state.detectedEntries, event)
307-
state.raceDetectionMode = true
308-
return true
309-
}
310-
311-
if state.temporaryOutputsByRunID[runID] == nil {
312-
state.temporaryOutputsByRunID[runID] = []string{}
313-
}
314-
state.temporaryOutputsByRunID[runID] = append(state.temporaryOutputsByRunID[runID], event.Output)
315-
return false
316-
}
317-
318322
// handlePanicRaceTermination processes the end of a panic/race block.
319323
func (p *defaultParser) handlePanicRaceTermination(state *testProcessingState, event entry, runID string) {
320324
terminalAction := event.Action == "pass" || event.Action == "fail" || event.Action == "skip"
@@ -324,12 +328,12 @@ func (p *defaultParser) handlePanicRaceTermination(state *testProcessingState, e
324328

325329
var outputs []string
326330
for _, de := range state.detectedEntries {
327-
outputs = append(outputs, de.Output)
331+
outputs = append(outputs, de.Event.Output)
328332
}
329333
outputStr := strings.Join(outputs, "\n")
330334
currentPackage := event.Package
331335
if currentPackage == "" && len(state.detectedEntries) > 0 {
332-
currentPackage = state.detectedEntries[0].Package
336+
currentPackage = state.detectedEntries[0].Event.Package
333337
}
334338

335339
attributedTestName := event.Test
@@ -378,7 +382,7 @@ func (p *defaultParser) handlePanicRaceTermination(state *testProcessingState, e
378382
}
379383

380384
// Reset state
381-
state.detectedEntries = []entry{}
385+
state.detectedEntries = []rawEventData{}
382386
state.panicDetectionMode = false
383387
state.raceDetectionMode = false
384388
}

tools/flakeguard/runner/parser/parser_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,3 +1075,85 @@ func TestParseFiles_IgnoreParentFailures(t *testing.T) {
10751075
})
10761076
}
10771077
}
1078+
1079+
func TestParseFiles_WithPanicEventFile(t *testing.T) {
1080+
t.Parallel()
1081+
1082+
parser := NewParser()
1083+
filePath := "testdata/events-with-panic.json"
1084+
results, _, err := parser.ParseFiles([]string{filePath}, "run", 1, Config{})
1085+
1086+
if err != nil {
1087+
t.Fatalf("Failed to parse event file: %v", err)
1088+
}
1089+
1090+
assert.Equal(t, 6, len(results), "Expected 6 test results from file.")
1091+
1092+
// Map test names to expected values for easier assertions
1093+
expected := map[string]struct {
1094+
pkg string
1095+
pkgPanic bool
1096+
panic bool
1097+
passRatio float64
1098+
runs int
1099+
failures int
1100+
successes int
1101+
skipped bool
1102+
skips int
1103+
}{
1104+
"Test_EventHandlerStateSync": {
1105+
pkg: "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/capabilities/workflows/syncer", pkgPanic: true, panic: false, passRatio: 1, runs: 1, failures: 0, successes: 1, skipped: false, skips: 0,
1106+
},
1107+
"Test_InitialStateSync": {
1108+
pkg: "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/capabilities/workflows/syncer", pkgPanic: true, panic: false, passRatio: 1, runs: 1, failures: 0, successes: 1, skipped: false, skips: 0,
1109+
},
1110+
"Test_RegistrySyncer_SkipsEventsNotBelongingToDON": {
1111+
pkg: "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/capabilities/workflows/syncer", pkgPanic: true, panic: false, passRatio: 1, runs: 1, failures: 0, successes: 1, skipped: false, skips: 0,
1112+
},
1113+
"Test_RegistrySyncer_WorkflowRegistered_InitiallyActivated": {
1114+
pkg: "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/capabilities/workflows/syncer", pkgPanic: true, panic: true, passRatio: 0, runs: 1, failures: 1, successes: 0, skipped: false, skips: 0,
1115+
},
1116+
"Test_RegistrySyncer_WorkflowRegistered_InitiallyPaused": {
1117+
pkg: "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/capabilities/workflows/syncer", pkgPanic: true, panic: false, passRatio: 1, runs: 1, failures: 0, successes: 1, skipped: false, skips: 0,
1118+
},
1119+
"Test_SecretsWorker": {
1120+
pkg: "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/capabilities/workflows/syncer", pkgPanic: false, panic: false, passRatio: 1, runs: 0, failures: 0, successes: 0, skipped: true, skips: 1,
1121+
},
1122+
}
1123+
1124+
for _, r := range results {
1125+
exp, ok := expected[r.TestName]
1126+
require.True(t, ok, "Unexpected test name: %s", r.TestName)
1127+
assert.Equal(t, exp.pkg, r.TestPackage, "Package mismatch for %s", r.TestName)
1128+
assert.Equal(t, exp.pkgPanic, r.PackagePanic, "PackagePanic mismatch for %s", r.TestName)
1129+
assert.Equal(t, exp.panic, r.Panic, "Panic mismatch for %s", r.TestName)
1130+
assert.InDelta(t, exp.passRatio, r.PassRatio, 0.001, "PassRatio mismatch for %s", r.TestName)
1131+
assert.Equal(t, exp.runs, r.Runs, "Runs mismatch for %s", r.TestName)
1132+
assert.Equal(t, exp.failures, r.Failures, "Failures mismatch for %s", r.TestName)
1133+
assert.Equal(t, exp.successes, r.Successes, "Successes mismatch for %s", r.TestName)
1134+
assert.Equal(t, exp.skipped, r.Skipped, "Skipped mismatch for %s", r.TestName)
1135+
assert.Equal(t, exp.skips, r.Skips, "Skips count mismatch for %s", r.TestName)
1136+
}
1137+
1138+
1139+
}
1140+
1141+
func TestParseFiles_WithPanicEventFileSpecific(t *testing.T) {
1142+
t.Parallel()
1143+
1144+
parser := NewParser()
1145+
filePath := "testdata/events-with-panic-single.json"
1146+
1147+
results, _, err := parser.ParseFiles([]string{filePath}, "run", 1, Config{})
1148+
if err != nil {
1149+
t.Fatalf("Failed to parse event file: %v", err)
1150+
}
1151+
assert.Equal(t, 1, len(results), "Expected 6 test results from file.")
1152+
1153+
testResult := results[0]
1154+
1155+
assert.True(t, testResult.Panic, "Expected test result to be marked as panic")
1156+
assert.True(t, testResult.PackagePanic, "Expected test result to be marked as panic")
1157+
assert.Equal(t, "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/capabilities/workflows/syncer", testResult.TestPackage, "Test package mismatch")
1158+
assert.Equal(t, "Test_RegistrySyncer_WorkflowRegistered_InitiallyActivated", testResult.TestName, "Test name mismatch")
1159+
}

0 commit comments

Comments
 (0)