fix: include filename in JUnit XML parse errors and consolidate directory walk (#895)

* green: ingestJunitDir error includes filename when XML parsing fails

Replace junit.IngestDir() with our own filepath.Walk that calls
junit.IngestFile() per file and wraps errors with the filename.
This makes XML parsing errors actionable — users can now see which
file caused the failure.

Add test fixture with ISO-8859-15 encoding declaration and three
unit tests for ingestJunitDir: filename in error, no-tests-found,
and valid JUnit parsing.

Refs #894

* green: getJunitFilenames error includes filename when XML parsing fails

Same wrapping treatment as ingestJunitDir — wrap junit.IngestFile()
errors with the filename so users can identify which file caused
the failure.

Refs #894

* green: consolidate JUnit dir walk into single pass

ingestJunitDir now returns both parsed results and the list of
JUnit filenames (for upload), eliminating the separate
getJunitFilenames() function and its redundant directory walk.

Refs #894

* refactor: address review feedback

- Return nil instead of empty slice on Walk error for consistency
- Merge TestIngestJunitDir_ReturnsFilenames into TestIngestJunitDir

Refs #894

* green: user-friendly error message for unsupported XML encoding

Replace the cryptic Go stdlib error (Decoder.CharsetReader is nil)
with an actionable message that tells the user which file failed,
that only UTF-8 is supported, and to use --results-dir if the file
is not a JUnit results file.

Refs #894

* refactor: address second round of review feedback

- Use filepath.WalkDir instead of filepath.Walk (avoids os.Lstat per entry)
- Return nil, nil on all error paths for consistency
- Assert exact filename count in test

Refs #894

* fix: alphabetical import ordering

Refs #894

Author: dangrondahl

cmd/kosli/attestJunit.go

@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"io"
+	"io/fs"
 	"net/http"
 	"net/url"
 	"os"
@@ -161,17 +162,13 @@ func (o *attestJunitOptions) run(args []string) error {
 		return err
 	}
 
-	o.payload.JUnitResults, err = ingestJunitDir(o.testResultsDir)
+	var junitFilenames []string
+	o.payload.JUnitResults, junitFilenames, err = ingestJunitDir(o.testResultsDir)
 	if err != nil {
 		return err
 	}
 
 	if o.uploadResultsDir {
-		// prepare the files to upload as attachments. We are only interested in the actual Junit XMl files
-		junitFilenames, err := getJunitFilenames(o.testResultsDir)
-		if err != nil {
-			return err
-		}
 		o.attachments = append(o.attachments, junitFilenames...)
 	}
 
@@ -212,22 +209,44 @@ type JUnitResults struct {
 	Timestamp float64 `json:"timestamp,omitempty"`
 }
 
-func ingestJunitDir(testResultsDir string) ([]*JUnitResults, error) {
+func ingestJunitDir(testResultsDir string) ([]*JUnitResults, []string, error) {
 	results := []*JUnitResults{}
-	suites, err := junit.IngestDir(testResultsDir)
+	var junitFilenames []string
+
+	var allSuites []junit.Suite
+	err := filepath.WalkDir(testResultsDir, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if d.Type().IsRegular() && strings.HasSuffix(d.Name(), ".xml") {
+			suites, err := junit.IngestFile(path)
+			if err != nil {
+				if strings.Contains(err.Error(), "Decoder.CharsetReader is nil") {
+					return fmt.Errorf("failed to parse XML file %s: only UTF-8 encoding is supported. "+
+						"If this is not a JUnit results file, use --results-dir to specify the directory containing only JUnit XML files", path)
+				}
+				return fmt.Errorf("failed to parse XML file %s: %w", path, err)
+			}
+			if len(suites) > 0 {
+				allSuites = append(allSuites, suites...)
+				junitFilenames = append(junitFilenames, path)
+			}
+		}
+		return nil
+	})
 	if err != nil {
-		return results, err
+		return nil, nil, err
 	}
 
-	if len(suites) == 0 {
-		return results, fmt.Errorf("no tests found in %s directory", testResultsDir)
+	if len(allSuites) == 0 {
+		return nil, nil, fmt.Errorf("no tests found in %s directory", testResultsDir)
 	}
 
-	for _, suite := range suites {
+	for _, suite := range allSuites {
 		var timestamp float64
 		timestamp, err := parseTimestamp(suite.Properties["timestamp"])
 		if err != nil {
-			return results, err
+			return nil, nil, err
 		}
 
 		// The values in suite.Totals are based on the results of the tests in the suite and not in the header of the suite.
@@ -244,36 +263,7 @@ func ingestJunitDir(testResultsDir string) ([]*JUnitResults, error) {
 		results = append(results, suiteResult)
 	}
 
-	return results, nil
-}
-
-func getJunitFilenames(directory string) ([]string, error) {
-	var filenames []string
-
-	err := filepath.Walk(directory, func(path string, info os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-
-		// Add all regular files that end with ".xml"
-		if info.Mode().IsRegular() && strings.HasSuffix(info.Name(), ".xml") {
-			suites, err := junit.IngestFile(path)
-			if err != nil {
-				return err
-			}
-			if len(suites) > 0 {
-				filenames = append(filenames, path)
-			}
-		}
-
-		return nil
-	})
-
-	if err != nil {
-		return filenames, err
-	}
-
-	return filenames, nil
+	return results, junitFilenames, nil
 }
 
 func parseTimestamp(timestampStr string) (float64, error) {

cmd/kosli/attestJunit_test.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -138,3 +140,34 @@ func (suite *AttestJunitCommandTestSuite) TestAttestJunitCmd() {
 func TestAttestJunitCommandTestSuite(t *testing.T) {
 	suite.Run(t, new(AttestJunitCommandTestSuite))
 }
+
+func TestIngestJunitDir(t *testing.T) {
+	t.Run("error includes filename and user-friendly message for unsupported encoding", func(t *testing.T) {
+		_, _, err := ingestJunitDir("testdata_junit_iso8859")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "results.xml")
+		assert.Contains(t, err.Error(), "only UTF-8 encoding is supported")
+		assert.Contains(t, err.Error(), "--results-dir")
+	})
+
+	t.Run("returns no tests found for empty directory", func(t *testing.T) {
+		dir := t.TempDir()
+		_, _, err := ingestJunitDir(dir)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "no tests found")
+	})
+
+	t.Run("parses valid JUnit XML correctly", func(t *testing.T) {
+		results, _, err := ingestJunitDir("testdata/junit")
+		require.NoError(t, err)
+		assert.NotEmpty(t, results)
+	})
+
+	t.Run("returns filenames of parsed JUnit XML files", func(t *testing.T) {
+		results, filenames, err := ingestJunitDir("testdata/junit")
+		require.NoError(t, err)
+		assert.NotEmpty(t, results)
+		assert.Len(t, filenames, 1)
+		assert.Contains(t, filenames[0], ".xml")
+	})
+}

cmd/kosli/testdata_junit_iso8859/results.xml

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="ISO-8859-15"?>
+<testsuites>
+  <testsuite name="com.example.MyTest" tests="2" failures="0" errors="0" time="0.123">
+    <testcase name="testPass" classname="com.example.MyTest" time="0.05"/>
+    <testcase name="testAlsoPass" classname="com.example.MyTest" time="0.073"/>
+  </testsuite>
+</testsuites>