Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions cmd/testrunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,6 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command
if err != nil {
return cobraext.FlagParsingError(err, cobraext.DataStreamsFlagName)
}

if testCoverage && len(dataStreams) > 0 {
return cobraext.FlagParsingError(errors.New("test coverage can be calculated only if all data streams are selected"), cobraext.DataStreamsFlagName)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This restriction has been removed for convenience, as coverage reports are useful during development and we may want to test as quick as possible and only the data stream being developed.

}

if runner.TestFolderRequired() {
Expand Down Expand Up @@ -203,6 +199,7 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command
API: esClient.API,
DeferCleanup: deferCleanup,
ServiceVariant: variantFlag,
WithCoverage: testCoverage,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this property apply to all test runners (system tests, static tests, etc.)? Maybe we should add a WARN informing if we can't calculate the coverage properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I see it we have the old default coverage, which only tells whether we have a particular kind of test for the package, and the opt-in detailed coverage added by this PR. Each individual test runner can err if they wanted to calculate detailed coverage but failed, as the pipeline tests will do after this PR.

})

results = append(results, r...)
Expand Down
188 changes: 157 additions & 31 deletions internal/testrunner/coverageoutput.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/pkg/errors"

"github.com/elastic/elastic-package/internal/builder"
"github.com/elastic/elastic-package/internal/multierror"
)

const coverageDtd = `<!DOCTYPE coverage SYSTEM "http://cobertura.sourceforge.net/xml/coverage-04.dtd">`
Expand All @@ -23,6 +24,8 @@ type testCoverageDetails struct {
packageName string
testType TestType
dataStreams map[string][]string // <data_stream> : <test case 1, test case 2, ...>
cobertura *CoberturaCoverage // For tests to provide custom Cobertura results.
errors multierror.Error
}

func newTestCoverageDetails(packageName string, testType TestType) *testCoverageDetails {
Expand All @@ -42,11 +45,19 @@ func (tcd *testCoverageDetails) withTestResults(results []TestResult) *testCover
tcd.dataStreams[result.DataStream] = []string{}
}
tcd.dataStreams[result.DataStream] = append(tcd.dataStreams[result.DataStream], result.Name)
if tcd.cobertura != nil && result.Coverage != nil {
if err := tcd.cobertura.merge(result.Coverage); err != nil {
tcd.errors = append(tcd.errors, errors.Wrapf(err, "can't merge Cobertura coverage for test `%s`", result.Name))
}
} else if tcd.cobertura == nil {
tcd.cobertura = result.Coverage
}
}
return tcd
}

type coberturaCoverage struct {
// CoberturaCoverage is the root element for a Cobertura XML report.
type CoberturaCoverage struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason why these properties are exposed? Is it for the consistency with CoberturaCoverage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I got mixed here a little bit. The original idea was for the test runners to have the option to generate their own coverage in Cobertura format (via the new TestResult.Coverage), that's why these types are exposed.

To align with this idea, I think it makes more sense to put GetPipelineCoverage into the pipeline runner instead of the global testrunner package as is now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To align with this idea, I think it makes more sense to put GetPipelineCoverage into the pipeline runner instead of the global testrunner package as is now.

Yes, I have a similar feeling about this :)

XMLName xml.Name `xml:"coverage"`
LineRate float32 `xml:"line-rate,attr"`
BranchRate float32 `xml:"branch-rate,attr"`
Expand All @@ -57,48 +68,53 @@ type coberturaCoverage struct {
BranchesCovered int64 `xml:"branches-covered,attr"`
BranchesValid int64 `xml:"branches-valid,attr"`
Complexity float32 `xml:"complexity,attr"`
Sources []*coberturaSource `xml:"sources>source"`
Packages []*coberturaPackage `xml:"packages>package"`
Sources []*CoberturaSource `xml:"sources>source"`
Packages []*CoberturaPackage `xml:"packages>package"`
}

type coberturaSource struct {
// CoberturaSource represents a base path to the covered source code.
type CoberturaSource struct {
Path string `xml:",chardata"`
}

type coberturaPackage struct {
// CoberturaPackage represents a package in a Cobertura XML report.
type CoberturaPackage struct {
Name string `xml:"name,attr"`
LineRate float32 `xml:"line-rate,attr"`
BranchRate float32 `xml:"branch-rate,attr"`
Complexity float32 `xml:"complexity,attr"`
Classes []*coberturaClass `xml:"classes>class"`
Classes []*CoberturaClass `xml:"classes>class"`
}

type coberturaClass struct {
// CoberturaClass represents a class in a Cobertura XML report.
type CoberturaClass struct {
Name string `xml:"name,attr"`
Filename string `xml:"filename,attr"`
LineRate float32 `xml:"line-rate,attr"`
BranchRate float32 `xml:"branch-rate,attr"`
Complexity float32 `xml:"complexity,attr"`
Methods []*coberturaMethod `xml:"methods>method"`
Methods []*CoberturaMethod `xml:"methods>method"`
Lines []*CoberturaLine `xml:"lines>line"`
}

type coberturaMethod struct {
Name string `xml:"name,attr"`
Signature string `xml:"signature,attr"`
LineRate float32 `xml:"line-rate,attr"`
BranchRate float32 `xml:"branch-rate,attr"`
Complexity float32 `xml:"complexity,attr"`
Lines coberturaLines `xml:"lines>line"`
// CoberturaMethod represents a method in a Cobertura XML report.
type CoberturaMethod struct {
Name string `xml:"name,attr"`
Signature string `xml:"signature,attr"`
LineRate float32 `xml:"line-rate,attr"`
BranchRate float32 `xml:"branch-rate,attr"`
Complexity float32 `xml:"complexity,attr"`
Hits int64 `xml:"hits,attr"`
Lines []*CoberturaLine `xml:"lines>line"`
}

type coberturaLine struct {
// CoberturaLine represents a source line in a Cobertura XML report.
type CoberturaLine struct {
Number int `xml:"number,attr"`
Hits int64 `xml:"hits,attr"`
}

type coberturaLines []*coberturaLine

func (c *coberturaCoverage) bytes() ([]byte, error) {
func (c *CoberturaCoverage) bytes() ([]byte, error) {
out, err := xml.MarshalIndent(&c, "", " ")
if err != nil {
return nil, errors.Wrap(err, "unable to format test results as xUnit")
Expand All @@ -113,6 +129,109 @@ func (c *coberturaCoverage) bytes() ([]byte, error) {
return buffer.Bytes(), nil
}

// merge merges two coverage reports for a given class.
func (c *CoberturaClass) merge(b *CoberturaClass) error {
// Check preconditions: classes should be the same.
equal := c.Name == b.Name &&
c.Filename == b.Filename &&
len(c.Lines) == len(b.Lines) &&
len(c.Methods) == len(b.Methods)
for idx := range c.Lines {
equal = equal && c.Lines[idx].Number == b.Lines[idx].Number
}
for idx := range c.Methods {
equal = equal && c.Methods[idx].Name == b.Methods[idx].Name &&
len(c.Methods[idx].Lines) == len(b.Methods[idx].Lines)
}
if !equal {
return errors.Errorf("merging incompatible classes: %+v != %+v", *c, *b)
}
// Update methods
for idx := range b.Methods {
c.Methods[idx].Hits += b.Methods[idx].Hits
for l := range b.Methods[idx].Lines {
c.Methods[idx].Lines[l].Hits += b.Methods[idx].Lines[l].Hits
}
}
// Rebuild lines
c.Lines = nil
for _, m := range c.Methods {
c.Lines = append(c.Lines, m.Lines...)
}
return nil
}

// merge merges two coverage reports for a given package.
func (p *CoberturaPackage) merge(b *CoberturaPackage) error {
// Merge classes
for _, class := range b.Classes {
var target *CoberturaClass
for _, existing := range p.Classes {
if existing.Name == class.Name {
target = existing
break
}
}
if target != nil {
if err := target.merge(class); err != nil {
return err
}
} else {
p.Classes = append(p.Classes, class)
}
}
return nil
}

// merge merges two coverage reports.
func (c *CoberturaCoverage) merge(b *CoberturaCoverage) error {
// Merge source paths
for _, path := range b.Sources {
found := false
for _, existing := range c.Sources {
if found = existing.Path == path.Path; found {
break
}
}
if !found {
c.Sources = append(c.Sources, path)
}
}

// Merge packages
for _, pkg := range b.Packages {
var target *CoberturaPackage
for _, existing := range c.Packages {
if existing.Name == pkg.Name {
target = existing
break
}
}
if target != nil {
if err := target.merge(pkg); err != nil {
return err
}
} else {
c.Packages = append(c.Packages, pkg)
}
}

// Recalculate global line coverage count
c.LinesValid = 0
c.LinesCovered = 0
for _, pkg := range c.Packages {
for _, cls := range pkg.Classes {
for _, line := range cls.Lines {
c.LinesValid++
if line.Hits > 0 {
c.LinesCovered++
}
}
}
}
return nil
}

// WriteCoverage function calculates test coverage for the given package.
// It requires to execute tests for all data streams (same test type), so the coverage can be calculated properly.
func WriteCoverage(packageRootPath, packageName string, testType TestType, results []TestResult) error {
Expand All @@ -121,7 +240,11 @@ func WriteCoverage(packageRootPath, packageName string, testType TestType, resul
return errors.Wrap(err, "can't collect test coverage details")
}

report := transformToCoberturaReport(details)
// Use provided cobertura report, or generate a custom report if not available.
report := details.cobertura
if report == nil {
report = transformToCoberturaReport(details)
}

err = writeCoverageReportFile(report, packageName)
if err != nil {
Expand All @@ -139,6 +262,9 @@ func collectTestCoverageDetails(packageRootPath, packageName string, testType Te
details := newTestCoverageDetails(packageName, testType).
withUncoveredDataStreams(withoutTests).
withTestResults(results)
if len(details.errors) > 0 {
return nil, details.errors
}
return details, nil
}

Expand Down Expand Up @@ -197,38 +323,38 @@ func verifyTestExpected(packageRootPath string, dataStreamName string, testType
return true, nil
}

func transformToCoberturaReport(details *testCoverageDetails) *coberturaCoverage {
var classes []*coberturaClass
func transformToCoberturaReport(details *testCoverageDetails) *CoberturaCoverage {
var classes []*CoberturaClass
for dataStream, testCases := range details.dataStreams {
if dataStream == "" {
continue // ignore tests running in the package context (not data stream), mostly referring to installed assets
}

var methods []*coberturaMethod
var methods []*CoberturaMethod

if len(testCases) == 0 {
methods = append(methods, &coberturaMethod{
methods = append(methods, &CoberturaMethod{
Name: "Missing",
Lines: []*coberturaLine{{Number: 1, Hits: 0}},
Lines: []*CoberturaLine{{Number: 1, Hits: 0}},
})
} else {
methods = append(methods, &coberturaMethod{
methods = append(methods, &CoberturaMethod{
Name: "OK",
Lines: []*coberturaLine{{Number: 1, Hits: 1}},
Lines: []*CoberturaLine{{Number: 1, Hits: 1}},
})
}

aClass := &coberturaClass{
aClass := &CoberturaClass{
Name: string(details.testType),
Filename: details.packageName + "/" + dataStream,
Methods: methods,
}
classes = append(classes, aClass)
}

return &coberturaCoverage{
return &CoberturaCoverage{
Timestamp: time.Now().UnixNano(),
Packages: []*coberturaPackage{
Packages: []*CoberturaPackage{
{
Name: details.packageName,
Classes: classes,
Expand All @@ -237,7 +363,7 @@ func transformToCoberturaReport(details *testCoverageDetails) *coberturaCoverage
}
}

func writeCoverageReportFile(report *coberturaCoverage, packageName string) error {
func writeCoverageReportFile(report *CoberturaCoverage, packageName string) error {
dest, err := testCoverageReportsDir()
if err != nil {
return errors.Wrap(err, "could not determine test coverage reports folder")
Expand Down
Loading