Skip to content

Commit a5cc01d

Browse files
committed
Merge branch 'TT-1995-add-go-test-transformer' into TT-1995-add-go-test-transformer-Flakeguard-integration
2 parents 01a6fff + 6af8024 commit a5cc01d

File tree

2 files changed

+95
-176
lines changed

2 files changed

+95
-176
lines changed

tools/flakeguard/go-test-transform/pkg/transformer/transformer.go

Lines changed: 43 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ type TestNode struct {
3030
Children map[string]*TestNode // Child tests
3131
Parent *TestNode // Parent test
3232
IsSubtest bool // Is this a subtest
33-
// Track whether this node is directly matched by patterns
34-
DirectlyIgnored bool
3533
// Store output messages for direct failure detection
3634
OutputMessages []string
35+
// Flag to track if this node has any failing subtests
36+
HasFailingSubtests bool
3737
}
3838

3939
// TransformJSON transforms go test -json output according to the options
@@ -62,11 +62,11 @@ func TransformJSON(input io.Reader, output io.Writer, opts *Options) error {
6262
// Track output messages for direct failure detection
6363
captureOutputMessages(events, testTree)
6464

65-
// Apply ignore rules
66-
applyIgnoreRules(testTree, opts)
65+
// Mark parents with failing subtests
66+
markParentsWithFailingSubtests(testTree)
6767

68-
// Propagate ignore status - but only in specific ways
69-
propagateIgnoreStatus(testTree, opts)
68+
// Identify which tests should be passed
69+
identifyTestsToIgnore(testTree, opts)
7070

7171
// Transform events
7272
transformedEvents, _ := transformEvents(events, testTree)
@@ -224,121 +224,58 @@ func captureOutputMessages(events []TestEvent, tree map[string]*TestNode) {
224224
}
225225
}
226226

227-
// applyIgnoreRules applies ignore rules to the test tree
228-
func applyIgnoreRules(tree map[string]*TestNode, opts *Options) {
227+
// markParentsWithFailingSubtests marks parent nodes that have any failing subtests
228+
func markParentsWithFailingSubtests(tree map[string]*TestNode) {
229229
// Process each package
230230
for _, pkgNode := range tree {
231-
// Process all tests in this package recursively
232-
var processNode func(*TestNode)
233-
processNode = func(node *TestNode) {
234-
// Only need to check if it's a failing test
235-
if node.Failed {
236-
// Apply ignore rules
237-
if node.IsSubtest && opts.IgnoreAllSubtestFailures {
238-
node.Ignored = true
239-
node.DirectlyIgnored = true
240-
}
241-
}
231+
// Bottom-up traversal to mark parents with failing subtests
232+
var markParents func(*TestNode) bool
233+
markParents = func(node *TestNode) bool {
234+
// Check if any child is failing or has failing subtests
235+
hasFailingSubtests := false
242236

243-
// Recursively process children
244237
for _, child := range node.Children {
245-
processNode(child)
238+
// Process child first (depth-first)
239+
childHasFailingSubtests := markParents(child)
240+
241+
// Check if child is failing or has failing subtests
242+
if child.Failed || childHasFailingSubtests {
243+
hasFailingSubtests = true
244+
}
246245
}
246+
247+
// Update the node's status
248+
node.HasFailingSubtests = hasFailingSubtests
249+
250+
// Return whether this node has failing subtests
251+
return hasFailingSubtests
247252
}
248253

249-
processNode(pkgNode)
254+
markParents(pkgNode)
250255
}
251256
}
252257

253-
// propagateIgnoreStatus propagates ignore status up the tree
254-
func propagateIgnoreStatus(tree map[string]*TestNode, opts *Options) {
258+
// identifyTestsToIgnore identifies which tests should be converted to pass
259+
func identifyTestsToIgnore(tree map[string]*TestNode, opts *Options) {
255260
// Process each package
256261
for _, pkgNode := range tree {
257-
// We need to be careful how we propagate status
258-
// Only propagate from children to parent when ALL failing children are ignored
259-
260-
// Bottom-up traversal
261-
var markIgnored func(*TestNode) bool
262-
markIgnored = func(node *TestNode) bool {
263-
// First process all children
264-
allFailingChildrenIgnored := true
265-
anyFailingChildren := false
266-
262+
// Process all nodes in the tree and set ignore status
263+
var processNode func(*TestNode)
264+
processNode = func(node *TestNode) {
265+
// Process children first
267266
for _, child := range node.Children {
268-
if child.Failed {
269-
anyFailingChildren = true
270-
if !markIgnored(child) {
271-
allFailingChildrenIgnored = false
272-
}
273-
} else {
274-
// Make sure to process non-failing children too
275-
markIgnored(child)
276-
}
267+
processNode(child)
277268
}
278269

279-
// Now decide for this node
280-
if node.Failed {
281-
// If explicitly ignored, return that status
282-
if node.DirectlyIgnored || node.Ignored {
283-
return true
284-
}
285-
286-
// If this is a parent and ALL failing children are ignored,
287-
// Decide if we should ignore the parent
288-
if anyFailingChildren && allFailingChildrenIgnored {
289-
// For TestNestedSubtests, we should always propagate
290-
if containsNestedFail(node) || containsParallel(node) {
291-
node.Ignored = true
292-
return true
293-
}
294-
295-
// For IgnoreAllSubtestFailures, we should always propagate
296-
if opts.IgnoreAllSubtestFailures {
297-
node.Ignored = true
298-
return true
299-
}
300-
}
301-
302-
// Default case: not ignored
303-
return false
270+
// If this node has failing subtests and it's failing itself,
271+
// mark it as ignored (so it will be converted to PASS)
272+
if node.HasFailingSubtests && node.Failed {
273+
node.Ignored = true
304274
}
305-
306-
// Node isn't failed, so it's "ignored" for propagation purposes
307-
return true
308-
}
309-
310-
markIgnored(pkgNode)
311-
}
312-
}
313-
314-
// containsNestedFail checks if this node or any of its children contain NestedFail
315-
func containsNestedFail(node *TestNode) bool {
316-
if strings.Contains(node.Name, "NestedFail") {
317-
return true
318-
}
319-
320-
for _, child := range node.Children {
321-
if containsNestedFail(child) {
322-
return true
323275
}
324-
}
325-
326-
return false
327-
}
328276

329-
// containsParallel checks if this node or any of its children contain Parallel
330-
func containsParallel(node *TestNode) bool {
331-
if strings.Contains(node.Name, "Parallel") {
332-
return true
333-
}
334-
335-
for _, child := range node.Children {
336-
if containsParallel(child) {
337-
return true
338-
}
277+
processNode(pkgNode)
339278
}
340-
341-
return false
342279
}
343280

344281
// transformEvents applies transformations to events based on the test tree
@@ -382,18 +319,6 @@ func transformEvents(events []TestEvent, tree map[string]*TestNode) ([]TestEvent
382319
return nil
383320
}
384321

385-
// Helper function to check if a node has direct failure messages
386-
hasDirectFailure := func(node *TestNode) bool {
387-
for _, msg := range node.OutputMessages {
388-
// Check if the message indicates a direct failure (not just reporting a child failure)
389-
// This is a simple heuristic - direct failures usually don't mention child tests
390-
if !strings.Contains(msg, "===") && !strings.Contains(msg, "---") {
391-
return true
392-
}
393-
}
394-
return false
395-
}
396-
397322
for i, event := range events {
398323
// Make a copy of the event
399324
transformedEvents[i] = event
@@ -402,16 +327,8 @@ func transformEvents(events []TestEvent, tree map[string]*TestNode) ([]TestEvent
402327
if event.Action == "fail" {
403328
node := findNode(event.Package, event.Test)
404329
if node != nil && node.Ignored {
405-
// For leaf subtests, keep them as failed
406-
// For parent tests with direct failures, keep them as failed
407-
// For parent tests that only fail because of child failures, change to pass
408-
if (node.IsSubtest && len(node.Children) == 0) || hasDirectFailure(node) {
409-
// This is a leaf subtest or has a direct failure - keep it as a failure
410-
anyRemainingFailures = true
411-
} else {
412-
// This is a parent test without direct failures - change to pass
413-
transformedEvents[i].Action = "pass"
414-
}
330+
// Convert this fail to a pass
331+
transformedEvents[i].Action = "pass"
415332
} else {
416333
// We're keeping this as a failure
417334
anyRemainingFailures = true
@@ -422,10 +339,8 @@ func transformEvents(events []TestEvent, tree map[string]*TestNode) ([]TestEvent
422339
} else if event.Action == "output" {
423340
node := findNode(event.Package, event.Test)
424341
if node != nil && node.Failed && node.Ignored {
425-
// Only transform output text for non-subtests or parent tests without direct failures
426-
if !node.IsSubtest && !hasDirectFailure(node) {
427-
transformedEvents[i].Output = transformOutputText(event.Output)
428-
}
342+
// Transform output text for passed tests
343+
transformedEvents[i].Output = transformOutputText(event.Output)
429344
}
430345
}
431346
}

0 commit comments

Comments
 (0)