From 699b7c2630e8e5354acf321a3767ab6ad7fdaf43 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Thu, 11 Nov 2021 20:03:11 +0100 Subject: [PATCH 1/5] Report ingest pipeline processor coverage This improves the coverage reporting (--test-coverage flag) to include detailed coverage for ingest pipelines in the pipeline test. --- cmd/testrunner.go | 5 +- internal/testrunner/coverageoutput.go | 309 ++++++++++++++++-- internal/testrunner/coverageoutput_test.go | 279 ++++++++++++++++ .../testrunner/runners/pipeline/runner.go | 6 + internal/testrunner/testrunner.go | 4 + 5 files changed, 568 insertions(+), 35 deletions(-) create mode 100644 internal/testrunner/coverageoutput_test.go diff --git a/cmd/testrunner.go b/cmd/testrunner.go index e51fd5df8e..9247b56b8f 100644 --- a/cmd/testrunner.go +++ b/cmd/testrunner.go @@ -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) - } } if runner.TestFolderRequired() { @@ -203,6 +199,7 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command API: esClient.API, DeferCleanup: deferCleanup, ServiceVariant: variantFlag, + WithCoverage: testCoverage, }) results = append(results, r...) diff --git a/internal/testrunner/coverageoutput.go b/internal/testrunner/coverageoutput.go index 6bfce1f921..6256347742 100644 --- a/internal/testrunner/coverageoutput.go +++ b/internal/testrunner/coverageoutput.go @@ -10,11 +10,15 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" "github.com/pkg/errors" "github.com/elastic/elastic-package/internal/builder" + "github.com/elastic/elastic-package/internal/elasticsearch/ingest" + "github.com/elastic/elastic-package/internal/multierror" + "github.com/elastic/elastic-package/internal/packages" ) const coverageDtd = `` @@ -23,6 +27,8 @@ type testCoverageDetails struct { packageName string testType TestType dataStreams map[string][]string // : + cobertura *CoberturaCoverage // For tests to provide custom Cobertura results. + errors multierror.Error } func newTestCoverageDetails(packageName string, testType TestType) *testCoverageDetails { @@ -42,11 +48,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 { XMLName xml.Name `xml:"coverage"` LineRate float32 `xml:"line-rate,attr"` BranchRate float32 `xml:"branch-rate,attr"` @@ -57,48 +71,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") @@ -113,6 +132,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 { @@ -121,7 +243,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 { @@ -139,6 +265,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 } @@ -197,28 +326,28 @@ 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, @@ -226,9 +355,9 @@ func transformToCoberturaReport(details *testCoverageDetails) *coberturaCoverage classes = append(classes, aClass) } - return &coberturaCoverage{ + return &CoberturaCoverage{ Timestamp: time.Now().UnixNano(), - Packages: []*coberturaPackage{ + Packages: []*CoberturaPackage{ { Name: details.packageName, Classes: classes, @@ -237,7 +366,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") @@ -272,3 +401,121 @@ func testCoverageReportsDir() (string, error) { } return filepath.Join(buildDir, "test-coverage"), nil } + +// GetPipelineCoverage returns a coverage report for the provided set of ingest pipelines. +func GetPipelineCoverage(options TestOptions, pipelines []ingest.Pipeline) (*CoberturaCoverage, error) { + packagePath, err := packages.MustFindPackageRoot() + if err != nil { + return nil, errors.Wrap(err, "error finding package root") + } + + dataStreamPath, found, err := packages.FindDataStreamRootForPath(options.TestFolder.Path) + if err != nil { + return nil, errors.Wrap(err, "locating data_stream root failed") + } + if !found { + return nil, errors.New("data stream root not found") + } + + // Use the Node Stats API to get stats for all installed pipelines. + // These stats contain hit counts for all main processors in a pipeline. + stats, err := ingest.GetPipelineStats(options.API, pipelines) + if err != nil { + return nil, errors.Wrap(err, "error fetching pipeline stats for code coverage calculations") + } + + // Construct the Cobertura report. + pkg := &CoberturaPackage{ + Name: options.TestFolder.Package + "." + options.TestFolder.DataStream, + } + + coverage := &CoberturaCoverage{ + Sources: []*CoberturaSource{ + { + Path: packagePath, + }, + }, + Packages: []*CoberturaPackage{pkg}, + Timestamp: time.Now().UnixNano(), + } + + // Calculate coverage for each pipeline + for _, pipeline := range pipelines { + covered, class, err := coverageForSinglePipeline(pipeline, stats, packagePath, dataStreamPath) + if err != nil { + return nil, errors.Wrapf(err, "error calculating coverage for pipeline '%s'", pipeline.Filename()) + } + pkg.Classes = append(pkg.Classes, class) + coverage.LinesValid += int64(len(class.Methods)) + coverage.LinesCovered += covered + } + return coverage, nil +} + +func coverageForSinglePipeline(pipeline ingest.Pipeline, stats ingest.PipelineStatsMap, packagePath, dataStreamPath string) (linesCovered int64, class *CoberturaClass, err error) { + // Load the list of main processors from the pipeline source code, annotated with line numbers. + src, err := pipeline.Processors() + if err != nil { + return 0, nil, err + } + + pstats, found := stats[pipeline.Name] + if !found { + return 0, nil, errors.Errorf("pipeline '%s' not installed in Elasticsearch", pipeline.Name) + } + + // Ensure there is no inconsistency in the list of processors in stats vs obtained from source. + if len(src) != len(pstats.Processors) { + return 0, nil, errors.Errorf("processor count mismatch for %s (src:%d stats:%d)", pipeline.Filename(), len(src), len(pstats.Processors)) + } + for idx, st := range pstats.Processors { + // Check that we have the expected type of processor, except for `compound` processors. + // Elasticsearch will return a `compound` processor in the case of `foreach` and + // any processor that defines `on_failure` processors. + if st.Type != "compound" && st.Type != src[idx].Type { + return 0, nil, errors.Errorf("processor type mismatch for %s processor %d (src:%s stats:%s)", pipeline.Filename(), idx, src[idx].Type, st.Type) + } + } + + // Tests install pipelines as `filename-` (without original extension). + // Use the filename part for the report. + pipelineName := pipeline.Name + if nameEnd := strings.LastIndexByte(pipelineName, '-'); nameEnd != -1 { + pipelineName = pipelineName[:nameEnd] + } + + // File path has to be relative to the packagePath added to the cobertura Sources list + // so that the source is reachable by the report tool. + pipelinePath := filepath.Join(dataStreamPath, "elasticsearch", "ingest_pipeline", pipeline.Filename()) + pipelineRelPath, err := filepath.Rel(packagePath, pipelinePath) + if err != nil { + return 0, nil, errors.Wrapf(err, "cannot create relative path to pipeline file. Package root: '%s', pipeline path: '%s'", packagePath, pipelinePath) + } + + // Report every pipeline as a "class". + class = &CoberturaClass{ + Name: pipelineName, + Filename: pipelineRelPath, + } + + // Calculate covered and total processors (reported as both lines and methods). + for idx, srcProc := range src { + if pstats.Processors[idx].Stats.Count > 0 { + linesCovered++ + } + method := CoberturaMethod{ + Name: srcProc.Type, + Hits: pstats.Processors[idx].Stats.Count, + } + for num := srcProc.FirstLine; num <= srcProc.LastLine; num++ { + line := &CoberturaLine{ + Number: num, + Hits: pstats.Processors[idx].Stats.Count, + } + class.Lines = append(class.Lines, line) + method.Lines = append(method.Lines, line) + } + class.Methods = append(class.Methods, &method) + } + return linesCovered, class, nil +} diff --git a/internal/testrunner/coverageoutput_test.go b/internal/testrunner/coverageoutput_test.go new file mode 100644 index 0000000000..799fa6c481 --- /dev/null +++ b/internal/testrunner/coverageoutput_test.go @@ -0,0 +1,279 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package testrunner + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCoberturaCoverage_Merge(t *testing.T) { + tests := []struct { + name string + rhs, lhs, expected CoberturaCoverage + wantErr bool + }{ + { + name: "merge sources", + rhs: CoberturaCoverage{ + Sources: []*CoberturaSource{ + {Path: "/a"}, + {Path: "/c"}, + }, + }, + lhs: CoberturaCoverage{ + Sources: []*CoberturaSource{ + {Path: "/b"}, + {Path: "/c"}, + }, + }, + expected: CoberturaCoverage{ + Sources: []*CoberturaSource{ + {Path: "/a"}, + {Path: "/c"}, + {Path: "/b"}, + }, + }, + }, + { + name: "merge packages and classes", + rhs: CoberturaCoverage{ + Packages: []*CoberturaPackage{ + { + Name: "a", + Classes: []*CoberturaClass{ + {Name: "a.a"}, + {Name: "a.b"}, + }, + }, + { + Name: "b", + Classes: []*CoberturaClass{ + {Name: "b.a"}, + }, + }, + }, + }, + lhs: CoberturaCoverage{ + Packages: []*CoberturaPackage{ + { + Name: "c", + Classes: []*CoberturaClass{ + {Name: "a.a"}, + }, + }, + { + Name: "b", + Classes: []*CoberturaClass{ + {Name: "b.a"}, + {Name: "b.b"}, + }, + }, + }, + }, + expected: CoberturaCoverage{ + Packages: []*CoberturaPackage{ + { + Name: "a", + Classes: []*CoberturaClass{ + {Name: "a.a"}, + {Name: "a.b"}, + }, + }, + { + Name: "b", + Classes: []*CoberturaClass{ + {Name: "b.a"}, + {Name: "b.b"}, + }, + }, + { + Name: "c", + Classes: []*CoberturaClass{ + {Name: "a.a"}, + }, + }, + }, + }, + }, + { + name: "merge methods and lines", + rhs: CoberturaCoverage{ + Packages: []*CoberturaPackage{ + { + Name: "a", + Classes: []*CoberturaClass{ + { + Name: "a.a", + Methods: []*CoberturaMethod{ + { + Name: "foo", + Hits: 2, + Lines: []*CoberturaLine{ + { + Number: 13, + Hits: 2, + }, + { + Number: 14, + Hits: 2, + }, + }, + }, + { + Name: "bar", + Hits: 1, + Lines: []*CoberturaLine{ + { + Number: 24, + Hits: 1, + }, + }, + }, + }, + Lines: []*CoberturaLine{ + { + Number: 13, + Hits: 2, + }, + { + Number: 14, + Hits: 2, + }, + { + Number: 24, + Hits: 1, + }, + }, + }, + }, + }, + }, + }, + lhs: CoberturaCoverage{ + Packages: []*CoberturaPackage{ + { + Name: "a", + Classes: []*CoberturaClass{ + { + Name: "a.a", + Methods: []*CoberturaMethod{ + { + Name: "foo", + Hits: 1, + Lines: []*CoberturaLine{ + { + Number: 13, + Hits: 1, + }, + { + Number: 14, + Hits: 1, + }, + }, + }, + { + Name: "bar", + Hits: 1, + Lines: []*CoberturaLine{ + { + Number: 24, + Hits: 1, + }, + }, + }, + }, + Lines: []*CoberturaLine{ + { + Number: 13, + Hits: 1, + }, + { + Number: 14, + Hits: 1, + }, + { + Number: 24, + Hits: 1, + }, + }, + }, + }, + }, + }, + }, + expected: CoberturaCoverage{ + LinesCovered: 3, + LinesValid: 3, + Packages: []*CoberturaPackage{ + { + Name: "a", + Classes: []*CoberturaClass{ + { + Name: "a.a", + Methods: []*CoberturaMethod{ + { + Name: "foo", + Hits: 3, + Lines: []*CoberturaLine{ + { + Number: 13, + Hits: 3, + }, + { + Number: 14, + Hits: 3, + }, + }, + }, + { + Name: "bar", + Hits: 2, + Lines: []*CoberturaLine{ + { + Number: 24, + Hits: 2, + }, + }, + }, + }, + Lines: []*CoberturaLine{ + { + Number: 13, + Hits: 3, + }, + { + Number: 14, + Hits: 3, + }, + { + Number: 24, + Hits: 2, + }, + }, + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.rhs.Merge(&tt.lhs) + if !tt.wantErr { + if !assert.NoError(t, err) { + t.Fatal(err) + } + } else { + if !assert.Error(t, err) { + t.Fatal("error expected") + } + } + assert.Equal(t, tt.expected, tt.rhs) + }) + } +} diff --git a/internal/testrunner/runners/pipeline/runner.go b/internal/testrunner/runners/pipeline/runner.go index df1888d792..98c4229868 100644 --- a/internal/testrunner/runners/pipeline/runner.go +++ b/internal/testrunner/runners/pipeline/runner.go @@ -156,6 +156,12 @@ func (r *runner) run() ([]testrunner.TestResult, error) { continue } + if r.options.WithCoverage { + tr.Coverage, err = testrunner.GetPipelineCoverage(r.options, r.pipelines) + if err != nil { + return nil, errors.Wrap(err, "error calculating pipeline coverage") + } + } results = append(results, tr) } return results, nil diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 66358b306d..3c6fb4f1d8 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -29,6 +29,7 @@ type TestOptions struct { DeferCleanup time.Duration ServiceVariant string + WithCoverage bool } // TestRunner is the interface all test runners must implement. @@ -86,6 +87,9 @@ type TestResult struct { // If the test was skipped, the reason it was skipped and a link for more // details. Skipped *SkipConfig + + // Coverage details in Cobertura format (optional). + Coverage *CoberturaCoverage } // ResultComposer wraps a TestResult and provides convenience methods for From 5f78a2b63c912dd59646aff3802d6b377905f0dd Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Tue, 22 Feb 2022 16:09:46 +0100 Subject: [PATCH 2/5] Make merge methods package-private --- internal/testrunner/coverageoutput.go | 18 +++++++++--------- internal/testrunner/coverageoutput_test.go | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/testrunner/coverageoutput.go b/internal/testrunner/coverageoutput.go index 6256347742..117de5360d 100644 --- a/internal/testrunner/coverageoutput.go +++ b/internal/testrunner/coverageoutput.go @@ -49,7 +49,7 @@ func (tcd *testCoverageDetails) withTestResults(results []TestResult) *testCover } 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 { + 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 { @@ -132,8 +132,8 @@ 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 { +// 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 && @@ -164,8 +164,8 @@ func (c *CoberturaClass) Merge(b *CoberturaClass) error { return nil } -// Merge merges two coverage reports for a given package. -func (p *CoberturaPackage) Merge(b *CoberturaPackage) error { +// 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 @@ -176,7 +176,7 @@ func (p *CoberturaPackage) Merge(b *CoberturaPackage) error { } } if target != nil { - if err := target.Merge(class); err != nil { + if err := target.merge(class); err != nil { return err } } else { @@ -186,8 +186,8 @@ func (p *CoberturaPackage) Merge(b *CoberturaPackage) error { return nil } -// Merge merges two coverage reports. -func (c *CoberturaCoverage) Merge(b *CoberturaCoverage) error { +// merge merges two coverage reports. +func (c *CoberturaCoverage) merge(b *CoberturaCoverage) error { // Merge source paths for _, path := range b.Sources { found := false @@ -211,7 +211,7 @@ func (c *CoberturaCoverage) Merge(b *CoberturaCoverage) error { } } if target != nil { - if err := target.Merge(pkg); err != nil { + if err := target.merge(pkg); err != nil { return err } } else { diff --git a/internal/testrunner/coverageoutput_test.go b/internal/testrunner/coverageoutput_test.go index 799fa6c481..5cdd234c30 100644 --- a/internal/testrunner/coverageoutput_test.go +++ b/internal/testrunner/coverageoutput_test.go @@ -263,7 +263,7 @@ func TestCoberturaCoverage_Merge(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := tt.rhs.Merge(&tt.lhs) + err := tt.rhs.merge(&tt.lhs) if !tt.wantErr { if !assert.NoError(t, err) { t.Fatal(err) From 1184d39998f12841c9b064c57f49ca83a16e4306 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Wed, 23 Feb 2022 16:15:31 +0100 Subject: [PATCH 3/5] Move GetPipelineCoverage to pipeline test package --- internal/testrunner/coverageoutput.go | 121 ---------------- .../testrunner/runners/pipeline/coverage.go | 135 ++++++++++++++++++ .../testrunner/runners/pipeline/runner.go | 2 +- 3 files changed, 136 insertions(+), 122 deletions(-) create mode 100644 internal/testrunner/runners/pipeline/coverage.go diff --git a/internal/testrunner/coverageoutput.go b/internal/testrunner/coverageoutput.go index 117de5360d..ef3e2e7ae2 100644 --- a/internal/testrunner/coverageoutput.go +++ b/internal/testrunner/coverageoutput.go @@ -10,15 +10,12 @@ import ( "fmt" "os" "path/filepath" - "strings" "time" "github.com/pkg/errors" "github.com/elastic/elastic-package/internal/builder" - "github.com/elastic/elastic-package/internal/elasticsearch/ingest" "github.com/elastic/elastic-package/internal/multierror" - "github.com/elastic/elastic-package/internal/packages" ) const coverageDtd = `` @@ -401,121 +398,3 @@ func testCoverageReportsDir() (string, error) { } return filepath.Join(buildDir, "test-coverage"), nil } - -// GetPipelineCoverage returns a coverage report for the provided set of ingest pipelines. -func GetPipelineCoverage(options TestOptions, pipelines []ingest.Pipeline) (*CoberturaCoverage, error) { - packagePath, err := packages.MustFindPackageRoot() - if err != nil { - return nil, errors.Wrap(err, "error finding package root") - } - - dataStreamPath, found, err := packages.FindDataStreamRootForPath(options.TestFolder.Path) - if err != nil { - return nil, errors.Wrap(err, "locating data_stream root failed") - } - if !found { - return nil, errors.New("data stream root not found") - } - - // Use the Node Stats API to get stats for all installed pipelines. - // These stats contain hit counts for all main processors in a pipeline. - stats, err := ingest.GetPipelineStats(options.API, pipelines) - if err != nil { - return nil, errors.Wrap(err, "error fetching pipeline stats for code coverage calculations") - } - - // Construct the Cobertura report. - pkg := &CoberturaPackage{ - Name: options.TestFolder.Package + "." + options.TestFolder.DataStream, - } - - coverage := &CoberturaCoverage{ - Sources: []*CoberturaSource{ - { - Path: packagePath, - }, - }, - Packages: []*CoberturaPackage{pkg}, - Timestamp: time.Now().UnixNano(), - } - - // Calculate coverage for each pipeline - for _, pipeline := range pipelines { - covered, class, err := coverageForSinglePipeline(pipeline, stats, packagePath, dataStreamPath) - if err != nil { - return nil, errors.Wrapf(err, "error calculating coverage for pipeline '%s'", pipeline.Filename()) - } - pkg.Classes = append(pkg.Classes, class) - coverage.LinesValid += int64(len(class.Methods)) - coverage.LinesCovered += covered - } - return coverage, nil -} - -func coverageForSinglePipeline(pipeline ingest.Pipeline, stats ingest.PipelineStatsMap, packagePath, dataStreamPath string) (linesCovered int64, class *CoberturaClass, err error) { - // Load the list of main processors from the pipeline source code, annotated with line numbers. - src, err := pipeline.Processors() - if err != nil { - return 0, nil, err - } - - pstats, found := stats[pipeline.Name] - if !found { - return 0, nil, errors.Errorf("pipeline '%s' not installed in Elasticsearch", pipeline.Name) - } - - // Ensure there is no inconsistency in the list of processors in stats vs obtained from source. - if len(src) != len(pstats.Processors) { - return 0, nil, errors.Errorf("processor count mismatch for %s (src:%d stats:%d)", pipeline.Filename(), len(src), len(pstats.Processors)) - } - for idx, st := range pstats.Processors { - // Check that we have the expected type of processor, except for `compound` processors. - // Elasticsearch will return a `compound` processor in the case of `foreach` and - // any processor that defines `on_failure` processors. - if st.Type != "compound" && st.Type != src[idx].Type { - return 0, nil, errors.Errorf("processor type mismatch for %s processor %d (src:%s stats:%s)", pipeline.Filename(), idx, src[idx].Type, st.Type) - } - } - - // Tests install pipelines as `filename-` (without original extension). - // Use the filename part for the report. - pipelineName := pipeline.Name - if nameEnd := strings.LastIndexByte(pipelineName, '-'); nameEnd != -1 { - pipelineName = pipelineName[:nameEnd] - } - - // File path has to be relative to the packagePath added to the cobertura Sources list - // so that the source is reachable by the report tool. - pipelinePath := filepath.Join(dataStreamPath, "elasticsearch", "ingest_pipeline", pipeline.Filename()) - pipelineRelPath, err := filepath.Rel(packagePath, pipelinePath) - if err != nil { - return 0, nil, errors.Wrapf(err, "cannot create relative path to pipeline file. Package root: '%s', pipeline path: '%s'", packagePath, pipelinePath) - } - - // Report every pipeline as a "class". - class = &CoberturaClass{ - Name: pipelineName, - Filename: pipelineRelPath, - } - - // Calculate covered and total processors (reported as both lines and methods). - for idx, srcProc := range src { - if pstats.Processors[idx].Stats.Count > 0 { - linesCovered++ - } - method := CoberturaMethod{ - Name: srcProc.Type, - Hits: pstats.Processors[idx].Stats.Count, - } - for num := srcProc.FirstLine; num <= srcProc.LastLine; num++ { - line := &CoberturaLine{ - Number: num, - Hits: pstats.Processors[idx].Stats.Count, - } - class.Lines = append(class.Lines, line) - method.Lines = append(method.Lines, line) - } - class.Methods = append(class.Methods, &method) - } - return linesCovered, class, nil -} diff --git a/internal/testrunner/runners/pipeline/coverage.go b/internal/testrunner/runners/pipeline/coverage.go new file mode 100644 index 0000000000..67764997dd --- /dev/null +++ b/internal/testrunner/runners/pipeline/coverage.go @@ -0,0 +1,135 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package pipeline + +import ( + "path/filepath" + "strings" + "time" + + "github.com/pkg/errors" + + "github.com/elastic/elastic-package/internal/elasticsearch/ingest" + "github.com/elastic/elastic-package/internal/packages" + "github.com/elastic/elastic-package/internal/testrunner" +) + +// GetPipelineCoverage returns a coverage report for the provided set of ingest pipelines. +func GetPipelineCoverage(options testrunner.TestOptions, pipelines []ingest.Pipeline) (*testrunner.CoberturaCoverage, error) { + packagePath, err := packages.MustFindPackageRoot() + if err != nil { + return nil, errors.Wrap(err, "error finding package root") + } + + dataStreamPath, found, err := packages.FindDataStreamRootForPath(options.TestFolder.Path) + if err != nil { + return nil, errors.Wrap(err, "locating data_stream root failed") + } + if !found { + return nil, errors.New("data stream root not found") + } + + // Use the Node Stats API to get stats for all installed pipelines. + // These stats contain hit counts for all main processors in a pipeline. + stats, err := ingest.GetPipelineStats(options.API, pipelines) + if err != nil { + return nil, errors.Wrap(err, "error fetching pipeline stats for code coverage calculations") + } + + // Construct the Cobertura report. + pkg := &testrunner.CoberturaPackage{ + Name: options.TestFolder.Package + "." + options.TestFolder.DataStream, + } + + coverage := &testrunner.CoberturaCoverage{ + Sources: []*testrunner.CoberturaSource{ + { + Path: packagePath, + }, + }, + Packages: []*testrunner.CoberturaPackage{pkg}, + Timestamp: time.Now().UnixNano(), + } + + // Calculate coverage for each pipeline + for _, pipeline := range pipelines { + covered, class, err := coverageForSinglePipeline(pipeline, stats, packagePath, dataStreamPath) + if err != nil { + return nil, errors.Wrapf(err, "error calculating coverage for pipeline '%s'", pipeline.Filename()) + } + pkg.Classes = append(pkg.Classes, class) + coverage.LinesValid += int64(len(class.Methods)) + coverage.LinesCovered += covered + } + return coverage, nil +} + +func coverageForSinglePipeline(pipeline ingest.Pipeline, stats ingest.PipelineStatsMap, packagePath, dataStreamPath string) (linesCovered int64, class *testrunner.CoberturaClass, err error) { + // Load the list of main processors from the pipeline source code, annotated with line numbers. + src, err := pipeline.Processors() + if err != nil { + return 0, nil, err + } + + pstats, found := stats[pipeline.Name] + if !found { + return 0, nil, errors.Errorf("pipeline '%s' not installed in Elasticsearch", pipeline.Name) + } + + // Ensure there is no inconsistency in the list of processors in stats vs obtained from source. + if len(src) != len(pstats.Processors) { + return 0, nil, errors.Errorf("processor count mismatch for %s (src:%d stats:%d)", pipeline.Filename(), len(src), len(pstats.Processors)) + } + for idx, st := range pstats.Processors { + // Check that we have the expected type of processor, except for `compound` processors. + // Elasticsearch will return a `compound` processor in the case of `foreach` and + // any processor that defines `on_failure` processors. + if st.Type != "compound" && st.Type != src[idx].Type { + return 0, nil, errors.Errorf("processor type mismatch for %s processor %d (src:%s stats:%s)", pipeline.Filename(), idx, src[idx].Type, st.Type) + } + } + + // Tests install pipelines as `filename-` (without original extension). + // Use the filename part for the report. + pipelineName := pipeline.Name + if nameEnd := strings.LastIndexByte(pipelineName, '-'); nameEnd != -1 { + pipelineName = pipelineName[:nameEnd] + } + + // File path has to be relative to the packagePath added to the cobertura Sources list + // so that the source is reachable by the report tool. + pipelinePath := filepath.Join(dataStreamPath, "elasticsearch", "ingest_pipeline", pipeline.Filename()) + pipelineRelPath, err := filepath.Rel(packagePath, pipelinePath) + if err != nil { + return 0, nil, errors.Wrapf(err, "cannot create relative path to pipeline file. Package root: '%s', pipeline path: '%s'", packagePath, pipelinePath) + } + + // Report every pipeline as a "class". + class = &testrunner.CoberturaClass{ + Name: pipelineName, + Filename: pipelineRelPath, + } + + // Calculate covered and total processors (reported as both lines and methods). + for idx, srcProc := range src { + if pstats.Processors[idx].Stats.Count > 0 { + linesCovered++ + } + method := testrunner.CoberturaMethod{ + Name: srcProc.Type, + Hits: pstats.Processors[idx].Stats.Count, + } + for num := srcProc.FirstLine; num <= srcProc.LastLine; num++ { + line := &testrunner.CoberturaLine{ + Number: num, + Hits: pstats.Processors[idx].Stats.Count, + } + class.Lines = append(class.Lines, line) + method.Lines = append(method.Lines, line) + } + class.Methods = append(class.Methods, &method) + } + return linesCovered, class, nil +} diff --git a/internal/testrunner/runners/pipeline/runner.go b/internal/testrunner/runners/pipeline/runner.go index 98c4229868..6ed9c49518 100644 --- a/internal/testrunner/runners/pipeline/runner.go +++ b/internal/testrunner/runners/pipeline/runner.go @@ -157,7 +157,7 @@ func (r *runner) run() ([]testrunner.TestResult, error) { } if r.options.WithCoverage { - tr.Coverage, err = testrunner.GetPipelineCoverage(r.options, r.pipelines) + tr.Coverage, err = GetPipelineCoverage(r.options, r.pipelines) if err != nil { return nil, errors.Wrap(err, "error calculating pipeline coverage") } From 9d774e9a039d20f840092b6ecc723321628af617 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Tue, 8 Mar 2022 14:49:10 +0100 Subject: [PATCH 4/5] Use TestOptions.PackageRootPath --- internal/testrunner/runners/pipeline/coverage.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/internal/testrunner/runners/pipeline/coverage.go b/internal/testrunner/runners/pipeline/coverage.go index 67764997dd..f5dea1f731 100644 --- a/internal/testrunner/runners/pipeline/coverage.go +++ b/internal/testrunner/runners/pipeline/coverage.go @@ -18,11 +18,6 @@ import ( // GetPipelineCoverage returns a coverage report for the provided set of ingest pipelines. func GetPipelineCoverage(options testrunner.TestOptions, pipelines []ingest.Pipeline) (*testrunner.CoberturaCoverage, error) { - packagePath, err := packages.MustFindPackageRoot() - if err != nil { - return nil, errors.Wrap(err, "error finding package root") - } - dataStreamPath, found, err := packages.FindDataStreamRootForPath(options.TestFolder.Path) if err != nil { return nil, errors.Wrap(err, "locating data_stream root failed") @@ -46,7 +41,7 @@ func GetPipelineCoverage(options testrunner.TestOptions, pipelines []ingest.Pipe coverage := &testrunner.CoberturaCoverage{ Sources: []*testrunner.CoberturaSource{ { - Path: packagePath, + Path: options.PackageRootPath, }, }, Packages: []*testrunner.CoberturaPackage{pkg}, @@ -55,7 +50,7 @@ func GetPipelineCoverage(options testrunner.TestOptions, pipelines []ingest.Pipe // Calculate coverage for each pipeline for _, pipeline := range pipelines { - covered, class, err := coverageForSinglePipeline(pipeline, stats, packagePath, dataStreamPath) + covered, class, err := coverageForSinglePipeline(pipeline, stats, options.PackageRootPath, dataStreamPath) if err != nil { return nil, errors.Wrapf(err, "error calculating coverage for pipeline '%s'", pipeline.Filename()) } From 71312b133bb5de20e1cfd7a9b0647cbac0621ee6 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Tue, 8 Mar 2022 15:18:37 +0100 Subject: [PATCH 5/5] Make paths relatives to package parent directory --- internal/testrunner/runners/pipeline/coverage.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/internal/testrunner/runners/pipeline/coverage.go b/internal/testrunner/runners/pipeline/coverage.go index f5dea1f731..b79c07f4ac 100644 --- a/internal/testrunner/runners/pipeline/coverage.go +++ b/internal/testrunner/runners/pipeline/coverage.go @@ -38,10 +38,16 @@ func GetPipelineCoverage(options testrunner.TestOptions, pipelines []ingest.Pipe Name: options.TestFolder.Package + "." + options.TestFolder.DataStream, } + // Use the package's parent directory as base path, so that the relative paths + // for each class (pipeline) include the package name. This prevents paths for + // different packages colliding (i.e. a lot of packages have a "log" datastream + // and a default.yml pipeline). + basePath := filepath.Dir(options.PackageRootPath) + coverage := &testrunner.CoberturaCoverage{ Sources: []*testrunner.CoberturaSource{ { - Path: options.PackageRootPath, + Path: basePath, }, }, Packages: []*testrunner.CoberturaPackage{pkg}, @@ -50,7 +56,7 @@ func GetPipelineCoverage(options testrunner.TestOptions, pipelines []ingest.Pipe // Calculate coverage for each pipeline for _, pipeline := range pipelines { - covered, class, err := coverageForSinglePipeline(pipeline, stats, options.PackageRootPath, dataStreamPath) + covered, class, err := coverageForSinglePipeline(pipeline, stats, basePath, dataStreamPath) if err != nil { return nil, errors.Wrapf(err, "error calculating coverage for pipeline '%s'", pipeline.Filename()) } @@ -61,7 +67,7 @@ func GetPipelineCoverage(options testrunner.TestOptions, pipelines []ingest.Pipe return coverage, nil } -func coverageForSinglePipeline(pipeline ingest.Pipeline, stats ingest.PipelineStatsMap, packagePath, dataStreamPath string) (linesCovered int64, class *testrunner.CoberturaClass, err error) { +func coverageForSinglePipeline(pipeline ingest.Pipeline, stats ingest.PipelineStatsMap, basePath, dataStreamPath string) (linesCovered int64, class *testrunner.CoberturaClass, err error) { // Load the list of main processors from the pipeline source code, annotated with line numbers. src, err := pipeline.Processors() if err != nil { @@ -96,9 +102,9 @@ func coverageForSinglePipeline(pipeline ingest.Pipeline, stats ingest.PipelineSt // File path has to be relative to the packagePath added to the cobertura Sources list // so that the source is reachable by the report tool. pipelinePath := filepath.Join(dataStreamPath, "elasticsearch", "ingest_pipeline", pipeline.Filename()) - pipelineRelPath, err := filepath.Rel(packagePath, pipelinePath) + pipelineRelPath, err := filepath.Rel(basePath, pipelinePath) if err != nil { - return 0, nil, errors.Wrapf(err, "cannot create relative path to pipeline file. Package root: '%s', pipeline path: '%s'", packagePath, pipelinePath) + return 0, nil, errors.Wrapf(err, "cannot create relative path to pipeline file. Package root: '%s', pipeline path: '%s'", basePath, pipelinePath) } // Report every pipeline as a "class".