From 54ab7b69d1a6a5552ac141f9f3c6a7a544a0738a Mon Sep 17 00:00:00 2001 From: Alexander Vasilyev Date: Sun, 22 Mar 2020 03:26:50 +0300 Subject: [PATCH] Add coverpkg specification. Currently, packages in coverme without tests created by student are not checked for coverage, because no there are no test targets for them at all! The issue can be fixed by simply creating empty !change files, but we are engineers, aren't we? :) Bonus point: it prevents new packages from giving false coverage credit. For example, mock file generated by gomock will have lots of covered code almost instantly. However, the task can be hacked by adding new code to existing packages. --- coverme/app/coverage_test.go | 2 +- tools/testtool/commands/coverage.go | 20 +++++++++++++++---- tools/testtool/commands/coverage_test.go | 1 + tools/testtool/commands/test_submission.go | 6 +++++- .../coverage/sum/subpkg/empty_test.go | 18 +++++++++-------- .../coverme/private/coverme/coverage_test.go | 2 +- .../coverme/student/coverme/coverage_test.go | 2 +- .../private/poorcoverage/coverage_test.go | 2 +- .../student/poorcoverage/coverage_test.go | 2 +- 9 files changed, 37 insertions(+), 18 deletions(-) diff --git a/coverme/app/coverage_test.go b/coverme/app/coverage_test.go index 2a9b595..c33daf0 100644 --- a/coverme/app/coverage_test.go +++ b/coverme/app/coverage_test.go @@ -2,4 +2,4 @@ package app -// min coverage: 90% +// min coverage: app,client,models,utils 90% diff --git a/tools/testtool/commands/coverage.go b/tools/testtool/commands/coverage.go index 7d796b9..4a824c8 100644 --- a/tools/testtool/commands/coverage.go +++ b/tools/testtool/commands/coverage.go @@ -18,8 +18,9 @@ import ( const coverageCommentPrefix = "min coverage: " type CoverageRequirements struct { - Enabled bool - Percent float64 + Enabled bool + Percent float64 + Packages []string } // getCoverageRequirements searches for comment in test files @@ -58,14 +59,25 @@ func searchCoverageComment(fname string) (*CoverageRequirements, error) { } t = strings.TrimPrefix(t, coverageCommentPrefix) t = strings.TrimSuffix(t, "%\n") - percent, err := strconv.ParseFloat(t, 64) + + parts := strings.Split(t, " ") + if len(parts) != 2 { + continue + } + + percent, err := strconv.ParseFloat(parts[1], 64) if err != nil { continue } if percent < 0 || percent > 100.0 { continue } - return &CoverageRequirements{Enabled: true, Percent: percent}, nil + + return &CoverageRequirements{ + Enabled: true, + Percent: percent, + Packages: strings.Split(parts[0], ","), + }, nil } return &CoverageRequirements{}, nil diff --git a/tools/testtool/commands/coverage_test.go b/tools/testtool/commands/coverage_test.go index c978296..6771d3a 100644 --- a/tools/testtool/commands/coverage_test.go +++ b/tools/testtool/commands/coverage_test.go @@ -10,4 +10,5 @@ func Test_getCoverageRequirements(t *testing.T) { r := getCoverageRequirements("../testdata/coverage/sum") require.True(t, r.Enabled) require.Equal(t, 90.0, r.Percent) + require.Equal(t, []string{"."}, r.Packages) } diff --git a/tools/testtool/commands/test_submission.go b/tools/testtool/commands/test_submission.go index 1b99b7b..1e744ef 100644 --- a/tools/testtool/commands/test_submission.go +++ b/tools/testtool/commands/test_submission.go @@ -254,7 +254,11 @@ func runTests(testDir, privateRepo, problem string) error { testBinaries[testPkg] = binPath cmd := []string{"test", "-mod", "readonly", "-tags", "private", "-c", "-o", binPath, testPkg} if coverageReq.Enabled { - cmd = append(cmd, "-cover") + pkgs := make([]string, len(coverageReq.Packages)) + for i, pkg := range coverageReq.Packages { + pkgs[i] = path.Join(moduleImportPath, problem, pkg) + } + cmd = append(cmd, "-cover", "-coverpkg", strings.Join(pkgs, ",")) } if err := runGo(cmd...); err != nil { return fmt.Errorf("error building test in %s: %w", testPkg, err) diff --git a/tools/testtool/testdata/coverage/sum/subpkg/empty_test.go b/tools/testtool/testdata/coverage/sum/subpkg/empty_test.go index 78fb9a3..b73bd28 100644 --- a/tools/testtool/testdata/coverage/sum/subpkg/empty_test.go +++ b/tools/testtool/testdata/coverage/sum/subpkg/empty_test.go @@ -3,20 +3,22 @@ package subpkg // Incorrect coverage comments: -// min coverage: -1% +// min coverage: . -1% -// min coverage: 100.001% +// min coverage: . 100.001% -// min coverage: 100 % +// min coverage: . 100 % -// min coverage:10% +// min coverage:. 10% -// min coverage: 19% - -// Correct coverage comment: +// min coverage: . 19% // min coverage: 90% +// Correct coverage comment: + +// min coverage: . 90% + // Testtool uses first matching comment. -// min coverage: 91% +// min coverage: . 91% diff --git a/tools/testtool/testdata/submissions/correct/coverme/private/coverme/coverage_test.go b/tools/testtool/testdata/submissions/correct/coverme/private/coverme/coverage_test.go index 6aebc0d..a9c1cc8 100644 --- a/tools/testtool/testdata/submissions/correct/coverme/private/coverme/coverage_test.go +++ b/tools/testtool/testdata/submissions/correct/coverme/private/coverme/coverage_test.go @@ -10,7 +10,7 @@ import ( "gitlab.com/slon/shad-go/coverme" ) -// min coverage: 70% +// min coverage: .,subpkg 70% func TestSum(t *testing.T) { require.Equal(t, int64(2), coverme.Sum(1, 1)) diff --git a/tools/testtool/testdata/submissions/correct/coverme/student/coverme/coverage_test.go b/tools/testtool/testdata/submissions/correct/coverme/student/coverme/coverage_test.go index 6aebc0d..a9c1cc8 100644 --- a/tools/testtool/testdata/submissions/correct/coverme/student/coverme/coverage_test.go +++ b/tools/testtool/testdata/submissions/correct/coverme/student/coverme/coverage_test.go @@ -10,7 +10,7 @@ import ( "gitlab.com/slon/shad-go/coverme" ) -// min coverage: 70% +// min coverage: .,subpkg 70% func TestSum(t *testing.T) { require.Equal(t, int64(2), coverme.Sum(1, 1)) diff --git a/tools/testtool/testdata/submissions/incorrect/poorcoverage/private/poorcoverage/coverage_test.go b/tools/testtool/testdata/submissions/incorrect/poorcoverage/private/poorcoverage/coverage_test.go index 23d2167..41647e0 100644 --- a/tools/testtool/testdata/submissions/incorrect/poorcoverage/private/poorcoverage/coverage_test.go +++ b/tools/testtool/testdata/submissions/incorrect/poorcoverage/private/poorcoverage/coverage_test.go @@ -9,7 +9,7 @@ import ( "gitlab.com/slon/shad-go/poorcoverage" ) -// min coverage: 100% +// min coverage: . 100% func TestSum(t *testing.T) { require.Equal(t, int64(2), poorcoverage.Sum(1, 1)) diff --git a/tools/testtool/testdata/submissions/incorrect/poorcoverage/student/poorcoverage/coverage_test.go b/tools/testtool/testdata/submissions/incorrect/poorcoverage/student/poorcoverage/coverage_test.go index 23d2167..41647e0 100644 --- a/tools/testtool/testdata/submissions/incorrect/poorcoverage/student/poorcoverage/coverage_test.go +++ b/tools/testtool/testdata/submissions/incorrect/poorcoverage/student/poorcoverage/coverage_test.go @@ -9,7 +9,7 @@ import ( "gitlab.com/slon/shad-go/poorcoverage" ) -// min coverage: 100% +// min coverage: . 100% func TestSum(t *testing.T) { require.Equal(t, int64(2), poorcoverage.Sum(1, 1))