Reuse the junit lib to generate junit xml for coverage results (knati…

…ve#726) * Reuse the junit lib to generate junit xml for coverage results * Use AddTestCase instead of appending slice
coryrc · May 1, 2019 · 939e3cc · 939e3cc
1 parent 03b0eab
commit 939e3cc
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 85 deletions.
diff --git a/shared/junit/junit.go b/shared/junit/junit.go
@@ -19,8 +19,8 @@ limitations under the License.
 package junit
 
 import (
-	"fmt"
 	"encoding/xml"
+	"fmt"
 )
 
 // TestStatusEnum is a enum for test result status
@@ -37,52 +37,52 @@ const (
 
 // TestSuites holds a <testSuites/> list of TestSuite results
 type TestSuites struct {
-	XMLName     xml.Name        `xml:"testsuites"`
-	Suites      []TestSuite     `xml:"testsuite"`
+	XMLName xml.Name    `xml:"testsuites"`
+	Suites  []TestSuite `xml:"testsuite"`
 }
 
 // TestSuite holds <testSuite/> results
 type TestSuite struct {
-	XMLName     xml.Name        `xml:"testsuite"`
-	Name        string          `xml:"name,attr"`
-	Time        float64         `xml:"time,attr"` // Seconds
-	Failures    int             `xml:"failures,attr"`
-	Tests       int             `xml:"tests,attr"`
-	TestCases   []TestCase      `xml:"testcase"`
-	Properties  TestProperties  `xml:"properties"`
+	XMLName    xml.Name       `xml:"testsuite"`
+	Name       string         `xml:"name,attr"`
+	Time       float64        `xml:"time,attr"` // Seconds
+	Failures   int            `xml:"failures,attr"`
+	Tests      int            `xml:"tests,attr"`
+	TestCases  []TestCase     `xml:"testcase"`
+	Properties TestProperties `xml:"properties"`
 }
 
 // TestCase holds <testcase/> results
 type TestCase struct {
-	Name        string          `xml:"name,attr"`
-	Time        float64         `xml:"time,attr"`  // Seconds
-	ClassName   string          `xml:"classname,attr"`
-	Failure     *string         `xml:"failure,omitempty"`
-	Output      *string         `xml:"system-out,omitempty"`
-	Error       *string         `xml:"system-err,omitempty"`
-	Skipped     *string         `xml:"skipped,omitempty"`
-	Properties  TestProperties  `xml:"properties"`
+	Name       string         `xml:"name,attr"`
+	Time       float64        `xml:"time,attr"` // Seconds
+	ClassName  string         `xml:"classname,attr"`
+	Failure    *string        `xml:"failure,omitempty"`
+	Output     *string        `xml:"system-out,omitempty"`
+	Error      *string        `xml:"system-err,omitempty"`
+	Skipped    *string        `xml:"skipped,omitempty"`
+	Properties TestProperties `xml:"properties"`
 }
 
 // TestProperties is an array of test properties
 type TestProperties struct {
-	Properties  []TestProperty  `xml:"property"`
+	Properties []TestProperty `xml:"property"`
 }
 
 // TestProperty defines a property of the test
 type TestProperty struct {
-	Name        string          `xml:"name,attr"`
-	Value       string          `xml:"value,attr"`
+	Name  string `xml:"name,attr"`
+	Value string `xml:"value,attr"`
 }
 
 // GetTestStatus returns the test status as a string
 func (testCase *TestCase) GetTestStatus() TestStatusEnum {
 	testStatus := Passed
 	switch {
-		case testCase.Failure != nil:
-			testStatus = Failed
-		case testCase.Skipped != nil:
-			testStatus = Skipped
+	case testCase.Failure != nil:
+		testStatus = Failed
+	case testCase.Skipped != nil:
+		testStatus = Skipped
 	}
 	return testStatus
 }
@@ -93,6 +93,11 @@ func (testCase *TestCase) AddProperty(name, val string) {
 	testCase.Properties.Properties = append(testCase.Properties.Properties, property)
 }
 
+// AddTestCase adds a testcase to the testsuite
+func (ts *TestSuite) AddTestCase(tc TestCase) {
+	ts.TestCases = append(ts.TestCases, tc)
+}
+
 // GetTestSuite gets TestSuite struct by name
 func (testSuites *TestSuites) GetTestSuite(suiteName string) (*TestSuite, error) {
 	for _, testSuite := range testSuites.Suites {
@@ -124,7 +129,7 @@ func (testSuites *TestSuites) ToBytes(prefix, indent string) ([]byte, error) {
 // the input Suite
 func UnMarshal(buf []byte) (*TestSuites, error) {
 	var testSuites TestSuites
-	if err := xml.Unmarshal(buf, &testSuites); nil == err { 
+	if err := xml.Unmarshal(buf, &testSuites); nil == err {
 		return &testSuites, nil
 	}
 

diff --git a/tools/coverage/gcs/gcs.go b/tools/coverage/gcs/gcs.go
@@ -30,20 +30,6 @@ import (
 	"google.golang.org/api/iterator"
 )
 
-const (
-	gcsUrlHost = "storage.cloud.google.com/"
-)
-
-// DoesObjectExist checks whether an object exists in GCS bucket
-func (client StorageClient) DoesObjectExist(ctx context.Context, bucket, object string) bool {
-	_, err := client.Bucket(bucket).Object(object).Attrs(ctx)
-	if err != nil {
-		log.Printf("Error getting attrs from object '%s': %v", object, err)
-		return false
-	}
-	return true
-}
-
 type StorageClientIntf interface {
 	Bucket(bucketName string) *storage.BucketHandle
 	ListGcsObjects(ctx context.Context, bucketName, prefix, delim string) (
@@ -103,6 +89,16 @@ func (client StorageClient) ProfileReader(ctx context.Context, bucket,
 	return artifacts.NewProfileReader(reader)
 }
 
+// DoesObjectExist checks whether an object exists in GCS bucket
+func (client StorageClient) DoesObjectExist(ctx context.Context, bucket, object string) bool {
+	_, err := client.Bucket(bucket).Object(object).Attrs(ctx)
+	if err != nil {
+		log.Printf("Error getting attrs from object '%s': %v", object, err)
+		return false
+	}
+	return true
+}
+
 type GcsBuild struct {
 	StorageClient StorageClientIntf
 	Bucket        string

diff --git a/tools/coverage/gcs/gcsPresubmit.go b/tools/coverage/gcs/gcsPresubmit.go
@@ -29,7 +29,10 @@ import (
 	"github.com/knative/test-infra/tools/coverage/githubUtil/githubPr"
 )
 
-const ArtifactsDirNameOnGcs = "artifacts"
+const (
+	ArtifactsDirNameOnGcs = "artifacts"
+	gcsUrlHost            = "storage.cloud.google.com/"
+)
 
 type PresubmitBuild struct {
 	GcsBuild

diff --git a/tools/coverage/testgrid/testsuiteXmlWriter.go b/tools/coverage/testgrid/testsuiteXmlWriter.go
@@ -16,86 +16,67 @@ limitations under the License.
 package testgrid
 
 import (
-	"encoding/xml"
 	"fmt"
 	"log"
 	"os"
+	"strconv"
 
+	"github.com/knative/test-infra/shared/junit"
 	"github.com/knative/test-infra/tools/coverage/artifacts"
 	"github.com/knative/test-infra/tools/coverage/calc"
 	"github.com/knative/test-infra/tools/coverage/logUtil"
 )
 
-type Property struct {
-	XMLName string `xml:"property"`
-	Name    string `xml:"name,attr"`
-	Value   string `xml:"value,attr"`
-}
-
-type Properties struct {
-	XMLName      string `xml:"properties"`
-	PropertyList []Property
-}
-
-type TestCase struct {
-	XMLName      string     `xml:"testcase"`
-	ClassName    string     `xml:"class_name,attr"`
-	Name         string     `xml:"name,attr"`
-	Time         string     `xml:"time,attr"`
-	Failure      bool       `xml:"failure,omitempty"`
-	PropertyList Properties `xml:"properties"`
-}
-
 // NewTestCase constructs the TestCase struct
-func NewTestCase(targetName, coverage string, failure bool) *TestCase {
-	properties := &Properties{}
-	properties.PropertyList = append(properties.PropertyList, Property{"", "coverage", coverage})
-
-	return &TestCase{"", "go_coverage", targetName, "0", failure, *properties}
-}
-
-type Testsuite struct {
-	XMLName   string     `xml:"testsuite"`
-	Testcases []TestCase `xml:"testsuite"`
-}
+func NewTestCase(targetName, coverage string, failure bool) junit.TestCase {
+	f := strconv.FormatBool(failure)
+	tc := junit.TestCase{
+		ClassName: "go_coverage",
+		Name:      targetName,
+		Failure:   &f,
+	}
+	tc.AddProperty("coverage", coverage)
 
-// addTestCase adds one test case to testsuite
-func (ts *Testsuite) addTestCase(tc TestCase) {
-	ts.Testcases = append(ts.Testcases, tc)
+	return tc
 }
 
 // toTestsuite populates Testsuite struct with data from CoverageList and actual file
 // directories from OS
-func toTestsuite(g *calc.CoverageList, dirs []string) (ts *Testsuite) {
-	ts = &Testsuite{}
+func toTestsuite(g *calc.CoverageList, dirs []string) *junit.TestSuite {
 	g.Summarize()
 	covThresInt := g.CovThresInt()
-	ts.addTestCase(*NewTestCase("OVERALL", g.PercentageForTestgrid(),
-		g.IsCoverageLow(covThresInt)))
+	ts := junit.TestSuite{}
+
+	// Add overall coverage
+	ts.AddTestCase(NewTestCase("OVERALL", g.PercentageForTestgrid(), g.IsCoverageLow(covThresInt)))
 
 	fmt.Println("")
 	log.Println("Constructing Testsuite Struct for Testgrid")
+
+	// Add coverage for individual files
 	for _, cov := range *g.Group() {
 		coverage := cov.PercentageForTestgrid()
 		if coverage != "" {
-			ts.addTestCase(*NewTestCase(cov.Name(), coverage, cov.IsCoverageLow(covThresInt)))
+			ts.AddTestCase(NewTestCase(cov.Name(), coverage, cov.IsCoverageLow(covThresInt)))
 		} else {
 			log.Printf("Skipping file %s as it has no coverage data.\n", cov.Name())
 		}
 	}
 
+	// Add coverage for dirs
 	for _, dir := range dirs {
 		dirCov := g.Subset(dir)
 		coverage := dirCov.PercentageForTestgrid()
 		if coverage != "" {
-			ts.addTestCase(*NewTestCase(dir, coverage, dirCov.IsCoverageLow(covThresInt)))
+			ts.AddTestCase(NewTestCase(dir, coverage, dirCov.IsCoverageLow(covThresInt)))
 		} else {
 			log.Printf("Skipping directory %s as it has no files with coverage data.\n", dir)
 		}
 	}
 	log.Println("Finished Constructing Testsuite Struct for Testgrid")
 	fmt.Println("")
-	return
+
+	return &ts
 }
 
 // ProfileToTestsuiteXML uses coverage profile (and it's corresponding stdout) to produce junit xml
@@ -113,8 +94,9 @@ func ProfileToTestsuiteXML(arts *artifacts.LocalArtifacts, covThres int) {
 	}
 	defer f.Close()
 
-	ts := toTestsuite(groupCov, groupCov.GetDirs())
-	output, err := xml.MarshalIndent(ts, "", "    ")
+	suites := junit.TestSuites{}
+	suites.AddTestSuite(toTestsuite(groupCov, groupCov.GetDirs()))
+	output, err := suites.ToBytes("", "    ")
 	if err != nil {
 		logUtil.LogFatalf("error: %v\n", err)
 	}