Skip to content

Commit a08fcaf

Browse files
committed
flakeguard: Improve performance of report aggregation
1 parent 11410d9 commit a08fcaf

File tree

4 files changed

+158
-36
lines changed

4 files changed

+158
-36
lines changed

tools/flakeguard/cmd/aggregate_results.go

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,25 +36,17 @@ var AggregateResultsCmd = &cobra.Command{
3636

3737
// Start spinner for loading test reports
3838
s := spinner.New(spinner.CharSets[11], 100*time.Millisecond)
39-
s.Suffix = " Loading test reports..."
39+
s.Suffix = " Aggregating test reports..."
4040
s.Start()
4141

42-
// Load test reports from JSON files
43-
testReports, err := reports.LoadReports(resultsPath)
42+
// Load test reports from JSON files and aggregate them
43+
aggregatedReport, err := reports.LoadAndAggregate(resultsPath)
4444
if err != nil {
4545
s.Stop()
4646
return fmt.Errorf("error loading test reports: %w", err)
4747
}
4848
s.Stop()
49-
fmt.Println("Test reports loaded successfully.")
50-
51-
// Start spinner for aggregating reports
52-
s = spinner.New(spinner.CharSets[11], 100*time.Millisecond)
53-
s.Suffix = " Aggregating test reports..."
54-
s.Start()
55-
56-
// Aggregate the reports
57-
aggregatedReport, err := reports.Aggregate(testReports...)
49+
fmt.Println("Test reports aggregated successfully.")
5850

5951
// Add metadata to the aggregated report
6052
aggregatedReport.HeadSHA = headSHA

tools/flakeguard/reports/data.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,13 @@ func FilterTests(results []TestResult, predicate func(TestResult) bool) []TestRe
128128
return filtered
129129
}
130130

131-
func Aggregate(reports ...*TestReport) (*TestReport, error) {
131+
func aggregate(reportChan <-chan *TestReport) (*TestReport, error) {
132132
testMap := make(map[string]TestResult)
133133
fullReport := &TestReport{}
134134
excludedTests := map[string]struct{}{}
135135
selectedTests := map[string]struct{}{}
136136

137-
for _, report := range reports {
137+
for report := range reportChan {
138138
if fullReport.GoProject == "" {
139139
fullReport.GoProject = report.GoProject
140140
} else if fullReport.GoProject != report.GoProject {
@@ -159,13 +159,15 @@ func Aggregate(reports ...*TestReport) (*TestReport, error) {
159159
}
160160
}
161161

162+
// Finalize excluded and selected tests
162163
for test := range excludedTests {
163164
fullReport.ExcludedTests = append(fullReport.ExcludedTests, test)
164165
}
165166
for test := range selectedTests {
166167
fullReport.SelectedTests = append(fullReport.SelectedTests, test)
167168
}
168169

170+
// Prepare final results
169171
var aggregatedResults []TestResult
170172
for _, result := range testMap {
171173
aggregatedResults = append(aggregatedResults, result)

tools/flakeguard/reports/data_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ func TestAggregate(t *testing.T) {
287287
},
288288
}
289289

290-
aggregatedReport, err := Aggregate(report1, report2)
290+
aggregatedReport, err := aggregate(report1, report2)
291291
if err != nil {
292292
t.Fatalf("Error aggregating reports: %v", err)
293293
}
@@ -371,7 +371,7 @@ func TestAggregateOutputs(t *testing.T) {
371371
},
372372
}
373373

374-
aggregatedReport, err := Aggregate(report1, report2)
374+
aggregatedReport, err := aggregate(report1, report2)
375375
if err != nil {
376376
t.Fatalf("Error aggregating reports: %v", err)
377377
}
@@ -432,7 +432,7 @@ func TestAggregateIdenticalOutputs(t *testing.T) {
432432
},
433433
}
434434

435-
aggregatedReport, err := Aggregate(report1, report2)
435+
aggregatedReport, err := aggregate(report1, report2)
436436
if err != nil {
437437
t.Fatalf("Error aggregating reports: %v", err)
438438
}
@@ -569,7 +569,7 @@ func TestAggregate_AllSkippedTests(t *testing.T) {
569569
},
570570
}
571571

572-
aggregatedReport, err := Aggregate(report1, report2)
572+
aggregatedReport, err := aggregate(report1, report2)
573573
if err != nil {
574574
t.Fatalf("Error aggregating reports: %v", err)
575575
}

tools/flakeguard/reports/io.go

Lines changed: 146 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"fmt"
77
"io"
8+
"log"
89
"os"
910
"path/filepath"
1011
)
@@ -31,30 +32,157 @@ func (OSFileSystem) WriteFile(filename string, data []byte, perm os.FileMode) er
3132
return os.WriteFile(filename, data, perm)
3233
}
3334

34-
// LoadReports reads JSON files from a directory and returns a slice of TestReport pointers
35-
func LoadReports(resultsPath string) ([]*TestReport, error) {
36-
var testReports []*TestReport
37-
err := filepath.Walk(resultsPath, func(path string, info os.FileInfo, err error) error {
35+
func LoadAndAggregate(resultsPath string) (*TestReport, error) {
36+
reportChan := make(chan *TestReport)
37+
errChan := make(chan error, 1)
38+
39+
// Start file processing in a goroutine
40+
go func() {
41+
defer close(reportChan)
42+
defer close(errChan)
43+
44+
err := filepath.Walk(resultsPath, func(path string, info os.FileInfo, err error) error {
45+
if err != nil {
46+
return fmt.Errorf("error accessing path %s: %w", path, err)
47+
}
48+
if !info.IsDir() && filepath.Ext(path) == ".json" {
49+
log.Printf("Processing file: %s", path)
50+
processLargeFile(path, reportChan, errChan)
51+
}
52+
return nil
53+
})
54+
if err != nil {
55+
errChan <- err
56+
}
57+
}()
58+
59+
// Aggregate results as they are being loaded
60+
aggregatedReport, err := aggregate(reportChan)
61+
if err != nil {
62+
return nil, fmt.Errorf("error aggregating reports: %w", err)
63+
}
64+
return aggregatedReport, nil
65+
}
66+
67+
func processLargeFile(filePath string, reportChan chan<- *TestReport, errChan chan<- error) {
68+
file, err := os.Open(filePath)
69+
if err != nil {
70+
errChan <- fmt.Errorf("error opening file %s: %w", filePath, err)
71+
log.Printf("Error opening file: %s, Error: %v", filePath, err)
72+
return
73+
}
74+
defer file.Close()
75+
76+
decoder := json.NewDecoder(file)
77+
78+
var report TestReport
79+
token, err := decoder.Token() // Read opening brace '{'
80+
if err != nil || token != json.Delim('{') {
81+
errChan <- fmt.Errorf("error reading JSON object start from file %s: %w", filePath, err)
82+
log.Printf("Error reading JSON object start from file: %s, Error: %v", filePath, err)
83+
return
84+
}
85+
86+
// Parse fields until we reach the end of the object
87+
for decoder.More() {
88+
token, err := decoder.Token()
3889
if err != nil {
39-
return fmt.Errorf("error accessing path %s: %w", path, err)
90+
errChan <- fmt.Errorf("error reading JSON token from file %s: %w", filePath, err)
91+
log.Printf("Error reading JSON token from file: %s, Error: %v", filePath, err)
92+
return
4093
}
41-
if !info.IsDir() && filepath.Ext(path) == ".json" {
42-
data, readErr := os.ReadFile(path)
43-
if readErr != nil {
44-
return fmt.Errorf("error reading file %s: %w", path, readErr)
94+
95+
fieldName, ok := token.(string)
96+
if !ok {
97+
errChan <- fmt.Errorf("unexpected JSON token in file %s", filePath)
98+
log.Printf("Unexpected JSON token in file: %s, Token: %v", filePath, token)
99+
return
100+
}
101+
102+
switch fieldName {
103+
case "GoProject":
104+
if err := decoder.Decode(&report.GoProject); err != nil {
105+
log.Printf("Error decoding GoProject in file: %s, Error: %v", filePath, err)
106+
return
107+
}
108+
case "HeadSHA":
109+
if err := decoder.Decode(&report.HeadSHA); err != nil {
110+
log.Printf("Error decoding HeadSHA in file: %s, Error: %v", filePath, err)
111+
return
112+
}
113+
case "BaseSHA":
114+
if err := decoder.Decode(&report.BaseSHA); err != nil {
115+
log.Printf("Error decoding BaseSHA in file: %s, Error: %v", filePath, err)
116+
return
117+
}
118+
case "RepoURL":
119+
if err := decoder.Decode(&report.RepoURL); err != nil {
120+
log.Printf("Error decoding RepoURL in file: %s, Error: %v", filePath, err)
121+
return
122+
}
123+
case "GitHubWorkflowName":
124+
if err := decoder.Decode(&report.GitHubWorkflowName); err != nil {
125+
log.Printf("Error decoding GitHubWorkflowName in file: %s, Error: %v", filePath, err)
126+
return
127+
}
128+
case "TestRunCount":
129+
if err := decoder.Decode(&report.TestRunCount); err != nil {
130+
log.Printf("Error decoding TestRunCount in file: %s, Error: %v", filePath, err)
131+
return
132+
}
133+
case "RaceDetection":
134+
if err := decoder.Decode(&report.RaceDetection); err != nil {
135+
log.Printf("Error decoding RaceDetection in file: %s, Error: %v", filePath, err)
136+
return
137+
}
138+
case "ExcludedTests":
139+
if err := decoder.Decode(&report.ExcludedTests); err != nil {
140+
log.Printf("Error decoding ExcludedTests in file: %s, Error: %v", filePath, err)
141+
return
45142
}
46-
var report TestReport
47-
if jsonErr := json.Unmarshal(data, &report); jsonErr != nil {
48-
return fmt.Errorf("error unmarshaling JSON from file %s: %w", path, jsonErr)
143+
case "SelectedTests":
144+
if err := decoder.Decode(&report.SelectedTests); err != nil {
145+
log.Printf("Error decoding SelectedTests in file: %s, Error: %v", filePath, err)
146+
return
49147
}
50-
testReports = append(testReports, &report)
148+
case "Results":
149+
token, err := decoder.Token() // Read opening bracket '['
150+
if err != nil || token != json.Delim('[') {
151+
log.Printf("Error reading Results array start in file: %s, Error: %v", filePath, err)
152+
return
153+
}
154+
155+
for decoder.More() {
156+
var result TestResult
157+
if err := decoder.Decode(&result); err != nil {
158+
log.Printf("Error decoding TestResult in file: %s, Error: %v", filePath, err)
159+
return
160+
}
161+
report.Results = append(report.Results, result)
162+
}
163+
164+
if _, err := decoder.Token(); err != nil {
165+
log.Printf("Error reading Results array end in file: %s, Error: %v", filePath, err)
166+
return
167+
}
168+
default:
169+
// Skip unknown fields
170+
var skip interface{}
171+
if err := decoder.Decode(&skip); err != nil {
172+
log.Printf("Error skipping unknown field: %s in file: %s, Error: %v", fieldName, filePath, err)
173+
return
174+
}
175+
log.Printf("Skipped unknown field: %s in file: %s", fieldName, filePath)
51176
}
52-
return nil
53-
})
54-
if err != nil {
55-
return nil, err
56177
}
57-
return testReports, nil
178+
179+
// Read closing brace '}'
180+
if _, err := decoder.Token(); err != nil {
181+
log.Printf("Error reading JSON object end in file: %s, Error: %v", filePath, err)
182+
return
183+
}
184+
185+
reportChan <- &report
58186
}
59187

60188
// LoadReport reads a JSON file and returns a TestReport pointer

0 commit comments

Comments
 (0)