Skip to content

Commit

Permalink
[BCF-2760] Flakey test detection improvements (#11470)
Browse files Browse the repository at this point in the history 
* [BCF-2760] Flakey test detection improvements

* [BCF-2760] Dedupe test and subtest entries
  • Loading branch information
@cedric-cordenier
cedric-cordenier authored Dec 12, 2023
1 parent 120bef7 commit 0c63446
Show file tree
Hide file tree
Showing 4 changed files with 399 additions and 101 deletions.
69 changes: 58 additions & 11 deletions tools/flakeytests/reporter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ import (
"time"
)

const (
messageType_flakeyTest = "flakey_test"
messageType_runReport = "run_report"
messageType_packagePanic = "package_panic"
)

type pushRequest struct {
Streams []stream `json:"streams"`
}
Expand All @@ -20,16 +26,28 @@ type stream struct {
Values [][]string `json:"values"`
}

type BaseMessage struct {
MessageType string `json:"message_type"`
Context
}

type flakeyTest struct {
BaseMessage
Package string `json:"package"`
TestName string `json:"test_name"`
FQTestName string `json:"fq_test_name"`
Context
}

type numFlakes struct {
NumFlakes int `json:"num_flakes"`
Context
type packagePanic struct {
BaseMessage
Package string `json:"package"`
}

type runReport struct {
BaseMessage
NumPackagePanics int `json:"num_package_panics"`
NumFlakes int `json:"num_flakes"`
NumCombined int `json:"num_combined"`
}

type Context struct {
Expand All @@ -48,17 +66,21 @@ type LokiReporter struct {
ctx Context
}

func (l *LokiReporter) createRequest(flakeyTests map[string]map[string]struct{}) (pushRequest, error) {
func (l *LokiReporter) createRequest(report *Report) (pushRequest, error) {
vs := [][]string{}
now := l.now()
nows := fmt.Sprintf("%d", now.UnixNano())
for pkg, tests := range flakeyTests {

for pkg, tests := range report.tests {
for t := range tests {
d, err := json.Marshal(flakeyTest{
BaseMessage: BaseMessage{
MessageType: messageType_flakeyTest,
Context: l.ctx,
},
Package: pkg,
TestName: t,
FQTestName: fmt.Sprintf("%s:%s", pkg, t),
Context: l.ctx,
})
if err != nil {
return pushRequest{}, err
Expand All @@ -67,10 +89,35 @@ func (l *LokiReporter) createRequest(flakeyTests map[string]map[string]struct{})
}
}

// Flakes are store in a map[string][]string, so to count them, we can't just do len(flakeyTests),
// Flakes are stored in a map[string][]string, so to count them, we can't just do len(flakeyTests),
// as that will get us the number of flakey packages, not the number of flakes tests.
// However, we do emit one log line per flakey test above, so use that to count our flakes.
f, err := json.Marshal(numFlakes{NumFlakes: len(vs), Context: l.ctx})
numFlakes := len(vs)

for pkg := range report.packagePanics {
d, err := json.Marshal(packagePanic{
BaseMessage: BaseMessage{
MessageType: messageType_packagePanic,
Context: l.ctx,
},
Package: pkg,
})
if err != nil {
return pushRequest{}, err
}

vs = append(vs, []string{nows, string(d)})
}

f, err := json.Marshal(runReport{
BaseMessage: BaseMessage{
MessageType: messageType_runReport,
Context: l.ctx,
},
NumFlakes: numFlakes,
NumPackagePanics: len(report.packagePanics),
NumCombined: numFlakes + len(report.packagePanics),
})
if err != nil {
return pushRequest{}, nil
}
Expand Down Expand Up @@ -120,8 +167,8 @@ func (l *LokiReporter) makeRequest(pushReq pushRequest) error {
return err
}

func (l *LokiReporter) Report(flakeyTests map[string]map[string]struct{}) error {
pushReq, err := l.createRequest(flakeyTests)
func (l *LokiReporter) Report(report *Report) error {
pushReq, err := l.createRequest(report)
if err != nil {
return err
}
Expand Down
103 changes: 83 additions & 20 deletions tools/flakeytests/reporter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,68 +12,131 @@ import (
func TestMakeRequest_SingleTest(t *testing.T) {
now := time.Now()
ts := fmt.Sprintf("%d", now.UnixNano())
ft := map[string]map[string]struct{}{
"core/assets": map[string]struct{}{
"TestLink": struct{}{},
r := &Report{
tests: map[string]map[string]int{
"core/assets": map[string]int{
"TestLink": 1,
},
},
}
lr := &LokiReporter{auth: "bla", host: "bla", command: "go_core_tests", now: func() time.Time { return now }}
pr, err := lr.createRequest(ft)
pr, err := lr.createRequest(r)
require.NoError(t, err)
assert.Len(t, pr.Streams, 1)
assert.Equal(t, pr.Streams[0].Stream, map[string]string{"command": "go_core_tests", "app": "flakey-test-reporter"})
assert.ElementsMatch(t, pr.Streams[0].Values, [][]string{
{ts, `{"package":"core/assets","test_name":"TestLink","fq_test_name":"core/assets:TestLink","commit_sha":"","repository":"","event_type":""}`},
{ts, `{"num_flakes":1,"commit_sha":"","repository":"","event_type":""}`},
{ts, `{"message_type":"flakey_test","commit_sha":"","repository":"","event_type":"","package":"core/assets","test_name":"TestLink","fq_test_name":"core/assets:TestLink"}`},
{ts, `{"message_type":"run_report","commit_sha":"","repository":"","event_type":"","num_package_panics":0,"num_flakes":1,"num_combined":1}`},
})
}

func TestMakeRequest_MultipleTests(t *testing.T) {
now := time.Now()
ts := fmt.Sprintf("%d", now.UnixNano())
ft := map[string]map[string]struct{}{
"core/assets": map[string]struct{}{
"TestLink": struct{}{},
"TestCore": struct{}{},
r := &Report{
tests: map[string]map[string]int{
"core/assets": map[string]int{
"TestLink": 1,
"TestCore": 1,
},
},
}
lr := &LokiReporter{auth: "bla", host: "bla", command: "go_core_tests", now: func() time.Time { return now }}
pr, err := lr.createRequest(ft)
pr, err := lr.createRequest(r)
require.NoError(t, err)
assert.Len(t, pr.Streams, 1)
assert.Equal(t, pr.Streams[0].Stream, map[string]string{"command": "go_core_tests", "app": "flakey-test-reporter"})

assert.ElementsMatch(t, pr.Streams[0].Values, [][]string{
{ts, `{"package":"core/assets","test_name":"TestLink","fq_test_name":"core/assets:TestLink","commit_sha":"","repository":"","event_type":""}`},
{ts, `{"package":"core/assets","test_name":"TestCore","fq_test_name":"core/assets:TestCore","commit_sha":"","repository":"","event_type":""}`},
{ts, `{"num_flakes":2,"commit_sha":"","repository":"","event_type":""}`},
{ts, `{"message_type":"flakey_test","commit_sha":"","repository":"","event_type":"","package":"core/assets","test_name":"TestLink","fq_test_name":"core/assets:TestLink"}`},
{ts, `{"message_type":"flakey_test","commit_sha":"","repository":"","event_type":"","package":"core/assets","test_name":"TestCore","fq_test_name":"core/assets:TestCore"}`},
{ts, `{"message_type":"run_report","commit_sha":"","repository":"","event_type":"","num_package_panics":0,"num_flakes":2,"num_combined":2}`},
})
}

func TestMakeRequest_NoTests(t *testing.T) {
now := time.Now()
ts := fmt.Sprintf("%d", now.UnixNano())
ft := map[string]map[string]struct{}{}
r := NewReport()
lr := &LokiReporter{auth: "bla", host: "bla", command: "go_core_tests", now: func() time.Time { return now }}
pr, err := lr.createRequest(ft)
pr, err := lr.createRequest(r)
require.NoError(t, err)
assert.Len(t, pr.Streams, 1)
assert.Equal(t, pr.Streams[0].Stream, map[string]string{"command": "go_core_tests", "app": "flakey-test-reporter"})
assert.ElementsMatch(t, pr.Streams[0].Values, [][]string{
{ts, `{"num_flakes":0,"commit_sha":"","repository":"","event_type":""}`},
{ts, `{"message_type":"run_report","commit_sha":"","repository":"","event_type":"","num_package_panics":0,"num_flakes":0,"num_combined":0}`},
})
}

func TestMakeRequest_WithContext(t *testing.T) {
now := time.Now()
ts := fmt.Sprintf("%d", now.UnixNano())
ft := map[string]map[string]struct{}{}
r := NewReport()
lr := &LokiReporter{auth: "bla", host: "bla", command: "go_core_tests", now: func() time.Time { return now }, ctx: Context{CommitSHA: "42"}}
pr, err := lr.createRequest(ft)
pr, err := lr.createRequest(r)
require.NoError(t, err)
assert.Len(t, pr.Streams, 1)
assert.Equal(t, pr.Streams[0].Stream, map[string]string{"command": "go_core_tests", "app": "flakey-test-reporter"})
assert.ElementsMatch(t, pr.Streams[0].Values, [][]string{
{ts, `{"num_flakes":0,"commit_sha":"42","repository":"","event_type":""}`},
{ts, `{"message_type":"run_report","commit_sha":"42","repository":"","event_type":"","num_package_panics":0,"num_flakes":0,"num_combined":0}`},
})
}

func TestMakeRequest_Panics(t *testing.T) {
now := time.Now()
ts := fmt.Sprintf("%d", now.UnixNano())
r := &Report{
tests: map[string]map[string]int{
"core/assets": map[string]int{
"TestLink": 1,
},
},
packagePanics: map[string]int{
"core/assets": 1,
},
}
lr := &LokiReporter{auth: "bla", host: "bla", command: "go_core_tests", now: func() time.Time { return now }}
pr, err := lr.createRequest(r)
require.NoError(t, err)
assert.Len(t, pr.Streams, 1)
assert.Equal(t, pr.Streams[0].Stream, map[string]string{"command": "go_core_tests", "app": "flakey-test-reporter"})

assert.ElementsMatch(t, pr.Streams[0].Values, [][]string{
{ts, `{"message_type":"flakey_test","commit_sha":"","repository":"","event_type":"","package":"core/assets","test_name":"TestLink","fq_test_name":"core/assets:TestLink"}`},
{ts, `{"message_type":"package_panic","commit_sha":"","repository":"","event_type":"","package":"core/assets"}`},
{ts, `{"message_type":"run_report","commit_sha":"","repository":"","event_type":"","num_package_panics":1,"num_flakes":1,"num_combined":2}`},
})
}

func TestDedupeEntries(t *testing.T) {
r := &Report{
tests: map[string]map[string]int{
"core/assets": map[string]int{
"TestSomethingAboutAssets/test_1": 2,
"TestSomethingAboutAssets": 4,
"TestSomeOtherTest": 1,
"TestSomethingAboutAssets/test_2": 2,
"TestFinalTest/test_1": 1,
},
"core/services/important_service": map[string]int{
"TestAnImportantService/a_subtest": 1,
},
},
}

otherReport, err := dedupeEntries(r)
require.NoError(t, err)

expectedMap := map[string]map[string]int{
"core/assets": map[string]int{
"TestSomethingAboutAssets/test_1": 2,
"TestSomeOtherTest": 1,
"TestSomethingAboutAssets/test_2": 2,
"TestFinalTest/test_1": 1,
},
"core/services/important_service": map[string]int{
"TestAnImportantService/a_subtest": 1,
},
}
assert.Equal(t, expectedMap, otherReport.tests)
}
Loading

0 comments on commit 0c63446

Please sign in to comment.