Skip to content

Commit 7f8fa09

Browse files
authored
Fix race attribution for Flakeguard (#1758)
1 parent 7db1c58 commit 7f8fa09

File tree

6 files changed

+188
-194
lines changed

6 files changed

+188
-194
lines changed

tools/flakeguard/cmd/get_gh_artifact_link.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"strings"
88
"time"
99

10-
"github.com/google/go-github/v67/github"
10+
"github.com/google/go-github/v70/github"
1111
"github.com/rs/zerolog/log"
1212
"github.com/spf13/cobra"
1313
"golang.org/x/oauth2"

tools/flakeguard/go.mod

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
module github.com/smartcontractkit/chainlink-testing-framework/tools/flakeguard
22

3-
go 1.24.1
3+
go 1.24.2
44

55
require (
66
github.com/andygrunwald/go-jira v1.16.0
77
github.com/briandowns/spinner v1.23.2
88
github.com/charmbracelet/bubbletea v1.3.4
99
github.com/charmbracelet/lipgloss v1.1.0
1010
github.com/go-resty/resty/v2 v2.16.5
11-
github.com/google/go-github/v67 v67.0.0
11+
github.com/google/go-github/v70 v70.0.0
1212
github.com/google/uuid v1.6.0
1313
github.com/rs/zerolog v1.34.0
1414
github.com/spf13/cobra v1.9.1
@@ -28,7 +28,6 @@ require (
2828
github.com/fatih/color v1.7.0 // indirect
2929
github.com/fatih/structs v1.1.0 // indirect
3030
github.com/golang-jwt/jwt/v4 v4.4.2 // indirect
31-
github.com/google/go-cmp v0.7.0 // indirect
3231
github.com/google/go-querystring v1.1.0 // indirect
3332
github.com/inconshreveable/mousetrap v1.1.0 // indirect
3433
github.com/lucasb-eyer/go-colorful v1.2.0 // indirect

tools/flakeguard/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
3535
github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
3636
github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
3737
github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU=
38-
github.com/google/go-github/v67 v67.0.0 h1:g11NDAmfaBaCO8qYdI9fsmbaRipHNWRIU/2YGvlh4rg=
39-
github.com/google/go-github/v67 v67.0.0/go.mod h1:zH3K7BxjFndr9QSeFibx4lTKkYS3K9nDanoI1NjaOtY=
38+
github.com/google/go-github/v70 v70.0.0 h1:/tqCp5KPrcvqCc7vIvYyFYTiCGrYvaWoYMGHSQbo55o=
39+
github.com/google/go-github/v70 v70.0.0/go.mod h1:xBUZgo8MI3lUL/hwxl3hlceJW1U8MVnXP3zUyI+rhQY=
4040
github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=
4141
github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU=
4242
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=

tools/flakeguard/runner/example_test_package/example_tests_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func TestFail(t *testing.T) {
2020

2121
func TestFailLargeOutput(t *testing.T) {
2222
t.Parallel()
23-
for i := 0; i < 1000; i++ {
23+
for range 1000 {
2424
t.Log("This is a log line")
2525
}
2626
t.Fatal("This test always fails")

tools/flakeguard/runner/runner.go

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -375,12 +375,13 @@ func (r *Runner) parseTestResults(runPrefix string, runCount int) ([]reports.Tes
375375
}
376376
}
377377

378+
// TODO: An argument could be made to check for entryLine.Action != "output" instead, but need to check edge cases
378379
if (panicDetectionMode || raceDetectionMode) && entryLine.Action == "fail" { // End of panic or race output
380+
var outputs []string
381+
for _, entry := range detectedEntries {
382+
outputs = append(outputs, entry.Output)
383+
}
379384
if panicDetectionMode {
380-
var outputs []string
381-
for _, entry := range detectedEntries {
382-
outputs = append(outputs, entry.Output)
383-
}
384385
panicTest, timeout, err := attributePanicToTest(outputs)
385386
if err != nil {
386387
log.Error().Msg("Unable to attribute panic to a test")
@@ -419,9 +420,11 @@ func (r *Runner) parseTestResults(runPrefix string, runCount int) ([]reports.Tes
419420
}
420421
}
421422
} else if raceDetectionMode {
422-
raceTest, err := attributeRaceToTest(entryLine.Package, detectedEntries)
423+
raceTest, err := attributeRaceToTest(outputs)
423424
if err != nil {
424-
return nil, err
425+
log.Warn().Msg("Unable to attribute race to a test")
426+
fmt.Println(err.Error())
427+
raceTest = "UnableToAttributeRaceTestPleaseInvestigate"
425428
}
426429
raceTestKey := fmt.Sprintf("%s/%s", entryLine.Package, raceTest)
427430

@@ -614,7 +617,13 @@ func (r *Runner) transformTestOutputFiles(filePaths []string) error {
614617
}
615618

616619
var (
620+
// Regex to extract a valid test function name from a panic message.
621+
// This is the most common situation for test panics, e.g.
622+
// github.com/smartcontractkit/chainlink/deployment/keystone/changeset_test.TestDeployBalanceReader(0xc000583c00)
623+
nestedTestNameRe = regexp.MustCompile(`\.(Test[^\s]+?)(?:\.[^(]+)?\s*\(`)
624+
617625
ErrFailedToAttributePanicToTest = errors.New("failed to attribute panic to test")
626+
ErrFailedToAttributeRaceToTest = errors.New("failed to attribute race to test")
618627
ErrFailedToParseTimeoutDuration = errors.New("failed to parse timeout duration")
619628
ErrFailedToExtractTimeoutDuration = errors.New("failed to extract timeout duration")
620629
ErrDetectedLogAfterCompleteFailedAttribution = errors.New("detected a log after test has completed panic, but failed to properly attribute it")
@@ -626,11 +635,6 @@ var (
626635
// There are a lot of edge cases and strange behavior in Go test output when it comes to panics.
627636
func attributePanicToTest(outputs []string) (test string, timeout bool, err error) {
628637
var (
629-
// Regex to extract a valid test function name from a panic message.
630-
// This is the most common situation for test panics, e.g.
631-
// github.com/smartcontractkit/chainlink/deployment/keystone/changeset_test.TestDeployBalanceReader(0xc000583c00)
632-
nestedTestNameRe = regexp.MustCompile(`\.(Test[^\s]+?)(?:\.[^(]+)?\s*\(`)
633-
634638
// Regex to check if the panic is from a log after a goroutine, e.g.
635639
// panic: Log in goroutine after Test_workflowRegisteredHandler/skips_fetch_if_secrets_url_is_missing has completed: <Log line>
636640
testLogAfterTestRe = regexp.MustCompile(`^panic: Log in goroutine after (Test[^\s]+) has completed:`)
@@ -716,18 +720,16 @@ func attributePanicToTest(outputs []string) (test string, timeout bool, err erro
716720
}
717721

718722
// attributeRaceToTest properly attributes races to the test that caused them.
719-
func attributeRaceToTest(racePackage string, raceEntries []entry) (string, error) {
720-
regexSanitizeRacePackage := filepath.Base(racePackage)
721-
raceAttributionRe := regexp.MustCompile(fmt.Sprintf(`%s\.(Test[^\.\(]+)`, regexSanitizeRacePackage))
722-
entriesOutputs := []string{}
723-
for _, entry := range raceEntries {
724-
entriesOutputs = append(entriesOutputs, entry.Output)
725-
if matches := raceAttributionRe.FindStringSubmatch(entry.Output); len(matches) > 1 {
726-
testName := strings.TrimSpace(matches[1])
727-
return testName, nil
723+
func attributeRaceToTest(outputs []string) (string, error) {
724+
for _, output := range outputs {
725+
match := nestedTestNameRe.FindStringSubmatch(output)
726+
if len(match) > 1 {
727+
return strings.TrimSpace(match[1]), nil
728728
}
729729
}
730-
return "", fmt.Errorf("failed to attribute race to test, using regex: %s on these strings:\n%s", raceAttributionRe.String(), strings.Join(entriesOutputs, ""))
730+
return "", fmt.Errorf("%w, using this output:\n\n%s",
731+
ErrFailedToAttributeRaceToTest, strings.Join(outputs, ""),
732+
)
731733
}
732734

733735
// parseSubTest checks if a test name is a subtest and returns the parent and sub names.

0 commit comments

Comments
 (0)