fix: JUnit processing for large files (#5879)

* fix: JUnit processing for large files

* fix: parsing one-line JUnit reports

* chore: add more junit logging

* chore: log xml data

* fix: reports without xml tag

* fix: junit post processor edge cases

* chore: remove unnecessary logs

* chore: add logs for paths

* refactor: read only first 8KB of the xml

* chore: more debug logs for sandbox

* fix: remove duplicated path prefixing

* chore: remove debug logs

* chore: remove log

* test: junit postprocessor with path prefix

cmd/testworkflow-toolkit/artifacts/junit_post_processor.go

@@ -1,8 +1,6 @@
 package artifacts
 
 import (
-	"bufio"
-	"bytes"
 	"context"
 	"fmt"
 	"io"
@@ -59,14 +57,16 @@ func (p *JUnitPostProcessor) Add(path string) error {
 	if ok := isXMLFile(stat); !ok {
 		return nil
 	}
-	xmlData, err := io.ReadAll(file)
-	if err != nil {
+	// Read only the first 8KB of data
+	const BYTE_SIZE_8KB = 8 * 1024
+	xmlData := make([]byte, BYTE_SIZE_8KB)
+	n, err := file.Read(xmlData)
+	if err != nil && err != io.EOF {
 		return errors.Wrapf(err, "failed to read %s", path)
 	}
-	ok, err := isJUnitReport(xmlData)
-	if err != nil {
-		return errors.Wrapf(err, "failed to check if %s is a JUnit report", path)
-	}
+	// Trim the slice to the actual number of bytes read
+	xmlData = xmlData[:n]
+	ok := isJUnitReport(xmlData)
 	if !ok {
 		return nil
 	}
@@ -79,10 +79,6 @@ func (p *JUnitPostProcessor) Add(path string) error {
 
 // sendJUnitReport sends the JUnit report to the Agent gRPC API.
 func (p *JUnitPostProcessor) sendJUnitReport(path string, report []byte) error {
-	// Apply path prefix correctly
-	if p.pathPrefix != "" {
-		path = filepath.Join(p.pathPrefix, path)
-	}
 	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
 	defer cancel()
 	_, err := p.client.Execute(ctx, testworkflow.CmdTestWorkflowExecutionAddReport, &testworkflow.ExecutionsAddReportRequest{
@@ -104,27 +100,22 @@ func isXMLFile(stat fs.FileInfo) bool {
 	return strings.HasSuffix(stat.Name(), ".xml")
 }
 
-// isJUnitReport checks if the file starts with a JUnit XML tag
-func isJUnitReport(xmlData []byte) (bool, error) {
-	scanner := bufio.NewScanner(bytes.NewReader(xmlData))
-	// Read only the first few lines of the file
-	for scanner.Scan() {
-		line := scanner.Text()
-		line = strings.TrimSpace(line) // Remove leading and trailing whitespace
-
-		// Skip comments and declarations
-		if strings.HasPrefix(line, "<!--") || strings.HasPrefix(line, "<?xml") {
-			continue
-		}
-		if strings.Contains(line, "<testsuite") || strings.Contains(line, "<testsuites") {
-			return true, nil
-		}
-		if strings.Contains(line, "<") { // Stop if any non-JUnit tag is found
-			break
+// isJUnitReport checks if the XML data is a JUnit report.
+func isJUnitReport(xmlData []byte) bool {
+	tags := []string{
+		"<testsuite",
+		"<testsuites",
+	}
+
+	content := string(xmlData)
+
+	for _, tag := range tags {
+		if strings.Contains(content, tag) {
+			return true
 		}
 	}
 
-	return false, scanner.Err()
+	return false
 }
 
 func (p *JUnitPostProcessor) End() error {

cmd/testworkflow-toolkit/artifacts/junit_post_processor_test.go

@@ -3,6 +3,7 @@ package artifacts
 import (
 	"io"
 	"io/fs"
+	"path/filepath"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -71,6 +72,33 @@ func TestJUnitPostProcessor_Add(t *testing.T) {
 
 }
 
+func TestJUnitPostProcessor_Add_WithPathPrefix(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	mockFS := filesystem.NewMockFileSystem(mockCtrl)
+	mockClient := executor.NewMockExecutor(mockCtrl)
+
+	pathPrefix := "prefixed/junit/report/"
+	filePath := "junit.xml"
+	junitContent := []byte(testdata.BasicJUnit)
+
+	mockFS.EXPECT().OpenFileRO(gomock.Any()).Return(filesystem.NewMockFile("junit.xml", junitContent), nil)
+
+	pp := NewJUnitPostProcessor(mockFS, mockClient, "/test_root", pathPrefix)
+
+	expectedPayload := testworkflow.ExecutionsAddReportRequest{
+		Filepath: filepath.Join(pathPrefix, filePath),
+		Report:   junitContent,
+	}
+
+	mockClient.EXPECT().Execute(gomock.Any(), testworkflow.CmdTestWorkflowExecutionAddReport, gomock.Eq(&expectedPayload)).Return(nil, nil)
+
+	err := pp.Add(filePath)
+
+	assert.NoError(t, err)
+}
+
 func TestIsXMLFile(t *testing.T) {
 	t.Parallel()
 
@@ -146,6 +174,21 @@ func TestIsJUnitReport(t *testing.T) {
 			file: filesystem.NewMockFile("invalid.xml", []byte(testdata.InvalidJUnit)),
 			want: false,
 		},
+		{
+			name: "one-line junit",
+			file: filesystem.NewMockFile("oneline.xml", []byte(testdata.OneLineJUnit)),
+			want: true,
+		},
+		{
+			name: "testsuites only junit",
+			file: filesystem.NewMockFile("testsuites.xml", []byte(testdata.TestsuitesOnlyJUnit)),
+			want: true,
+		},
+		{
+			name: "testsuite only junit",
+			file: filesystem.NewMockFile("testsuite.xml", []byte(testdata.TestsuiteOnlyJUnit)),
+			want: true,
+		},
 	}
 
 	for _, tc := range tests {
@@ -154,10 +197,7 @@ func TestIsJUnitReport(t *testing.T) {
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
-			ok, err := isJUnitReport(data)
-			if err != nil {
-				t.Fatalf("unexpected error: %v", err)
-			}
+			ok := isJUnitReport(data)
 			assert.Equal(t, tc.want, ok)
 		})
 	}

cmd/testworkflow-toolkit/artifacts/walker.go

@@ -196,7 +196,6 @@ func (w *walker) walk(fsys fs.FS, path string, walker WalkerFn) error {
 	if sanitizedPath == "" {
 		sanitizedPath = "."
 	}
-
 	return fs.WalkDir(fsys, sanitizedPath, func(filePath string, d fs.DirEntry, err error) error {
 		resolvedPath := "/" + filepath.ToSlash(filePath)
 		if !w.matches(resolvedPath) {

cmd/testworkflow-toolkit/common/testdata/junit.go

@@ -176,3 +176,16 @@ var InvalidJUnit = `<?xml version="1.0" encoding="UTF-8"?>
 <foo>
 	<bar>
 </foo>`
+
+var OneLineJUnit = `<?xml version="1.0" encoding="UTF-8"?><testsuites><testsuite name="TestSuite" tests="2" errors="0" failures="1" skipped="0"><testcase name="Test1" classname="TestClass"><failure message="Test failed">Failure details</failure></testcase><testcase name="Test2" classname="TestClass"/></testsuite></testsuites>`
+
+var TestsuitesOnlyJUnit = `<testsuites id="" name="" tests="2" failures="0" skipped="0" errors="0" time="14.511833">
+    <testsuite name="smoke.spec.js" timestamp="2024-10-01T12:48:47.332Z" hostname="chromium" tests="1" failures="0" skipped="0" time="6.259" errors="0">
+        <testcase name="Smoke 1 - has title" classname="smoke.spec.js" time="6.259"></testcase>
+    </testsuite>
+    <testsuite name="smoke2.spec.js" timestamp="2024-10-01T12:48:47.332Z" hostname="chromium" tests="1" failures="0" skipped="0" time="6.657" errors="0">
+        <testcase name="Smoke 2 - has title" classname="smoke2.spec.js" time="6.657"></testcase>
+    </testsuite>
+</testsuites>`
+
+var TestsuiteOnlyJUnit = `<testsuite name="smoke.spec.js" timestamp="2024-10-01T12:48:47.332Z" hostname="chromium" tests="1" failures="0" skipped="0" time="6.259" errors="0"><testcase name="Smoke 1 - has title" classname="smoke.spec.js" time="6.259"></testcase></testsuite>`