Skip to content

Commit

Permalink
Implements a better config structure for testing and test coverage …
Browse files Browse the repository at this point in the history 
…targets. Implements better (and more configurable) check for whether a project has at least a certain amount of test files, or a certain ratio of test files to other files
  • Loading branch information
@bvobart
bvobart committed Jun 24, 2021
1 parent a549c9b commit 37a2f72
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 44 deletions.
53 changes: 48 additions & 5 deletions config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,43 @@ type TestingConfig struct {
// Expects a JUnit XML file, which when using `pytest` can be generated with `pytest --junitxml=tests-report.xml`
Report string `yaml:"report" toml:"report"`

// Settings about how many tests there should be in a project.
Targets TestingTargets `yaml:"targets" toml:"targets"`

// Settings about the rules for checking project test coverage.
Coverage TestCoverage `yaml:"coverage" toml:"coverage"`
}

type TestingTargets struct {
// Minimum amount of test files to have in a project. Absolute number. Defaults to 1.
Minimum uint64 `yaml:"minimum" toml:"minimum"`

// Ratio of test files to have in a project, i.e. number of test files per other Python file.
// Defaults to 1 part tests to 4 parts non-tests
Ratio TestingTargetsRatio `yaml:"ratio" toml:"ratio"`
}

type TestingTargetsRatio struct {
// Number of parts of test files.
Tests uint64 `yaml:"tests" toml:"tests"`
// Number of parts of other Python files.
Other uint64 `yaml:"other" toml:"other"`
}

type TestCoverage struct {
// Filename of the project's test coverage report, either absolute or relative to the project's root.
// Expects a Cobertura-compatible XML file, which can be generated after `coverage run -m pytest --junitxml=tests-report.xml`
// with `coverage xml -o tests-coverage.xml`
Coverage string `yaml:"coverage" toml:"coverage"`
// with `coverage xml -o tests-coverage.xml`, or using the `pytest-cov` plugin.
Report string `yaml:"report" toml:"report"`

// Specifies the target amount of line / branch / whatever coverage that the user wants want to have in the project
// Only line coverage is implemented so far.
Targets TestCoverageTargets `yaml:"targets" toml:"targets"`
}

// Target percentage of line test coverage to achieve for this project
CoverageTarget float64 `yaml:"coverageTarget" toml:"coverageTarget"`
type TestCoverageTargets struct {
// Target amount of overall line coverage to achieve in tests.
Line float64 `yaml:"line" toml:"line"`
}

//---------------------------------------------------------------------------------------
Expand All @@ -61,7 +91,20 @@ func Default() *Config {
Rules: RuleConfig{Disabled: []string{}},
Git: GitConfig{MaxFileSize: 10_000_000}, // 10 MB
CodeQuality: CodeQualityConfig{Linters: []string{"pylint", "mypy", "black", "isort", "bandit"}},
Testing: TestingConfig{CoverageTarget: 80},
Testing: TestingConfig{
Targets: TestingTargets{
Minimum: 1,
Ratio: TestingTargetsRatio{
Tests: 1,
Other: 4,
},
},
Coverage: TestCoverage{
Targets: TestCoverageTargets{
Line: 80,
},
},
},
}
}

Expand Down
29 changes: 23 additions & 6 deletions config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,16 @@ code-quality:

const yamlTesting = `
testing:
report: tests-report.xml
coverage: coverage-report.xml
report: junit-report.xml
targets:
minimum: 2
ratio:
tests: 2
other: 8
coverage:
report: coverage.xml
targets:
line: 50 # percent line coverage.
`

const yamlInvalid = `
Expand All @@ -58,7 +66,8 @@ linters = ["pylint", "mypy"]
const tomlTesting = `
[tool.mllint.testing]
report = "tests-report.xml"
coverage = "coverage-report.xml"
targets = { minimum = 2, ratio = { tests = 2, other = 8 }}
coverage = { report = "coverage.xml", targets = { line = 100.0 }}
`

const tomlInvalid = `
Expand Down Expand Up @@ -114,8 +123,12 @@ func TestParseYAML(t *testing.T) {
File: strings.NewReader(yamlTesting),
Expected: func() *config.Config {
c := config.Default()
c.Testing.Report = "tests-report.xml"
c.Testing.Coverage = "coverage-report.xml"
c.Testing.Report = "junit-report.xml"
c.Testing.Targets.Minimum = 2
c.Testing.Targets.Ratio.Tests = 2
c.Testing.Targets.Ratio.Other = 8
c.Testing.Coverage.Report = "coverage.xml"
c.Testing.Coverage.Targets.Line = 50
return c
}(),
Err: nil,
Expand Down Expand Up @@ -195,7 +208,11 @@ func TestParseTOML(t *testing.T) {
Expected: func() *config.Config {
c := config.Default()
c.Testing.Report = "tests-report.xml"
c.Testing.Coverage = "coverage-report.xml"
c.Testing.Targets.Minimum = 2
c.Testing.Targets.Ratio.Tests = 2
c.Testing.Targets.Ratio.Other = 8
c.Testing.Coverage.Report = "coverage.xml"
c.Testing.Coverage.Targets.Line = 100
return c
}(),
Err: nil,
Expand Down
48 changes: 29 additions & 19 deletions linters/testing/linter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"io"
"math"
"path"
"strings"

Expand Down Expand Up @@ -36,10 +37,10 @@ func (l *TestingLinter) Name() string {

func (l *TestingLinter) Configure(conf *config.Config) error {
l.Config = conf.Testing
if l.Config.CoverageTarget > 100 {
return fmt.Errorf("%w: %.1f", ErrCoverageTargetTooHigh, l.Config.CoverageTarget)
} else if l.Config.CoverageTarget < 0 {
return fmt.Errorf("%w: %.1f", ErrCoverageTargetTooLow, l.Config.CoverageTarget)
if l.Config.Coverage.Targets.Line > 100 {
return fmt.Errorf("%w: %.1f", ErrCoverageTargetTooHigh, l.Config.Coverage.Targets.Line)
} else if l.Config.Coverage.Targets.Line < 0 {
return fmt.Errorf("%w: %.1f", ErrCoverageTargetTooLow, l.Config.Coverage.Targets.Line)
}
return nil
}
Expand All @@ -60,9 +61,6 @@ func (l *TestingLinter) LintProject(project api.Project) (api.Report, error) {
l.ScoreRuleTestsPass(&report, project)
l.ScoreRuleTestCoverage(&report, project)

// TODO: determine possible config options:
// - target amount of tests per file

return report, nil
}

Expand All @@ -74,10 +72,22 @@ func (l *TestingLinter) ScoreRuleHasTests(report *api.Report, project api.Projec
return
}

// Possible TODO: have a config option for the target amount of tests per file?
if len(l.TestFiles) < int(l.Config.Targets.Minimum) {
report.Scores[RuleHasTests] = 0
report.Details[RuleHasTests] = fmt.Sprintf("There are **%d** test files in your project, but `mllint` was expecting at least **%d**.", len(l.TestFiles), l.Config.Targets.Minimum)
return
}

// there should be at 1 test file per 4 non-test Python files.
report.Scores[RuleHasTests] = 100 * (float64(len(l.TestFiles)) * 4 / float64(len(project.PythonFiles)-len(l.TestFiles)))
// determine expected ratio of test files vs. other Python files
expectedRatio := float64(l.Config.Targets.Ratio.Tests) / float64(l.Config.Targets.Ratio.Other)
actualRatio := float64(len(l.TestFiles)) / float64(len(project.PythonFiles))
report.Scores[RuleHasTests] = math.Min(100*actualRatio/expectedRatio, 100)
if actualRatio < expectedRatio {
report.Details[RuleHasTests] = fmt.Sprintf("There are **%d** test files in your project, which meets the minimum of **%d** test files required.", len(l.TestFiles), l.Config.Targets.Minimum)
report.Details[RuleHasTests] += fmt.Sprintf("\n\nHowever, this only equates to **%.1f%%** of Python files in your project being tests, while `mllint` expects that **%.1f%%** of your project's Python files are tests.", 100*actualRatio, 100*expectedRatio)
} else {
report.Details[RuleHasTests] = fmt.Sprintf("Great! Your project contains **%d** test files, which meets the minimum of **%d** test files required.\nThis equates to **%.1f%%** of Python files in your project being tests, which meets the target ratio of **%.1f%%**", len(l.TestFiles), l.Config.Targets.Minimum, 100*actualRatio, 100*expectedRatio)
}
}

//---------------------------------------------------------------------------------------
Expand Down Expand Up @@ -182,16 +192,16 @@ Please make sure your test report file is a valid JUnit XML file. %s`, l.Config.
//---------------------------------------------------------------------------------------

func (l *TestingLinter) ScoreRuleTestCoverage(report *api.Report, project api.Project) {
if l.Config.Coverage == "" {
if l.Config.Coverage.Report == "" {
report.Scores[RuleTestCoverage] = 0
report.Details[RuleTestCoverage] = "No test coverage report was provided. Please update the `testing.coverage` setting in your project's `mllint` configuration to specify the path to your project's test coverage report.\n\n" + howToMakeCoverageXML
return
}

covReportFile, err := utils.OpenFile(project.Dir, l.Config.Coverage)
covReportFile, err := utils.OpenFile(project.Dir, l.Config.Coverage.Report)
if err != nil {
report.Scores[RuleTestCoverage] = 0
report.Details[RuleTestCoverage] = fmt.Sprintf("A test coverage report was provided, namely `%s`, but this file could not be found or opened (%s). Please update the `testing.coverage` setting in your project's `mllint` configuration to fix the path to your project's test report. Remember that this path must be relative to the root of your project directory.", l.Config.Coverage, err.Error())
report.Details[RuleTestCoverage] = fmt.Sprintf("A test coverage report was provided, namely `%s`, but this file could not be found or opened (%s). Please update the `testing.coverage` setting in your project's `mllint` configuration to fix the path to your project's test report. Remember that this path must be relative to the root of your project directory.", l.Config.Coverage.Report, err.Error())
return
}

Expand All @@ -213,21 +223,21 @@ Please make sure your test report file is a valid Cobertura-compatible XML file.
totalLines := covReport.NumLines()
hitLines := covReport.NumLinesWithHits()
hitRate := 100 * float64(hitLines) / float64(totalLines) // percentage of lines covered.
score := 100 * hitRate / l.Config.CoverageTarget // percentage of coverage target achieved.
score := 100 * hitRate / l.Config.Coverage.Targets.Line // percentage of coverage target achieved.
if totalLines == 0 {
score = 0
}
if l.Config.CoverageTarget == 0 {
if l.Config.Coverage.Targets.Line == 0 {
score = 100
}
report.Scores[RuleTestCoverage] = score

if totalLines != 0 && hitLines == totalLines {
report.Details[RuleTestCoverage] = "Wow! Congratulations! You've achieved full **100%** line test coverage! Great job!"
} else if hitRate < l.Config.CoverageTarget {
report.Details[RuleTestCoverage] = fmt.Sprintf("Your project's tests achieved **%.1f%%** line test coverage, but **%.1f%%** is the target amount of test coverage to beat. You'll need to further improve your tests.", hitRate, l.Config.CoverageTarget)
} else if hitRate >= l.Config.CoverageTarget {
report.Details[RuleTestCoverage] = fmt.Sprintf("Congratulations, your project's tests have achieved **%.1f%%** line test coverage, which meets the target of **%.1f%%** test coverage!", hitRate, l.Config.CoverageTarget)
} else if hitRate < l.Config.Coverage.Targets.Line {
report.Details[RuleTestCoverage] = fmt.Sprintf("Your project's tests achieved **%.1f%%** line test coverage, but **%.1f%%** is the target amount of test coverage to beat. You'll need to further improve your tests.", hitRate, l.Config.Coverage.Targets.Line)
} else if hitRate >= l.Config.Coverage.Targets.Line {
report.Details[RuleTestCoverage] = fmt.Sprintf("Congratulations, your project's tests have achieved **%.1f%%** line test coverage, which meets the target of **%.1f%%** test coverage!", hitRate, l.Config.Coverage.Targets.Line)
} else if totalLines == 0 {
report.Details[RuleTestCoverage] = "It seems your test coverage report is empty, no lines were covered."
}
Expand Down
39 changes: 25 additions & 14 deletions linters/testing/linter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func TestTestingLinter(t *stdtesting.T) {
WithConfig(func() *config.Config {
c := config.Default()
c.Testing.Report = "junit-passed-all.xml"
c.Testing.Coverage = "coverage-0.xml"
c.Testing.Coverage.Report = "coverage-0.xml"
return c
}()),
Expect: func(t *stdtesting.T, report api.Report, err error) {
Expand All @@ -126,8 +126,8 @@ func TestTestingLinter(t *stdtesting.T) {
WithConfig(func() *config.Config {
c := config.Default()
c.Testing.Report = "junit-passed-all.xml"
c.Testing.Coverage = "coverage-50.xml"
c.Testing.CoverageTarget = 100
c.Testing.Coverage.Report = "coverage-50.xml"
c.Testing.Coverage.Targets.Line = 100
return c
}()),
Expect: func(t *stdtesting.T, report api.Report, err error) {
Expand All @@ -148,8 +148,8 @@ func TestTestingLinter(t *stdtesting.T) {
WithConfig(func() *config.Config {
c := config.Default()
c.Testing.Report = "junit-passed-all.xml"
c.Testing.Coverage = "coverage-50.xml"
c.Testing.CoverageTarget = 50
c.Testing.Coverage.Report = "coverage-50.xml"
c.Testing.Coverage.Targets.Line = 50
return c
}()),
Expect: func(t *stdtesting.T, report api.Report, err error) {
Expand All @@ -170,8 +170,8 @@ func TestTestingLinter(t *stdtesting.T) {
WithConfig(func() *config.Config {
c := config.Default()
c.Testing.Report = "junit-passed-all.xml"
c.Testing.Coverage = "coverage-100.xml"
c.Testing.CoverageTarget = 100
c.Testing.Coverage.Report = "coverage-100.xml"
c.Testing.Coverage.Targets.Line = 100
return c
}()),
Expect: func(t *stdtesting.T, report api.Report, err error) {
Expand All @@ -192,7 +192,7 @@ func TestTestingLinter(t *stdtesting.T) {
WithConfig(func() *config.Config {
c := config.Default()
c.Testing.Report = "junit-failed-all.xml"
c.Testing.Coverage = ""
c.Testing.Coverage.Report = ""
return c
}()),
Expect: func(t *stdtesting.T, report api.Report, err error) {
Expand All @@ -211,7 +211,7 @@ func TestTestingLinter(t *stdtesting.T) {
WithConfig(func() *config.Config {
c := config.Default()
c.Testing.Report = "junit-passed-half.xml"
c.Testing.Coverage = "" // TODO
c.Testing.Coverage.Report = "" // TODO
return c
}()),
Expect: func(t *stdtesting.T, report api.Report, err error) {
Expand Down Expand Up @@ -247,7 +247,7 @@ func TestTestingLinter(t *stdtesting.T) {
Options: testutils.NewOptions().UsePythonFiles(createPythonFilenames(16).Concat(createPythonTestFilenames(4))).
WithConfig(func() *config.Config {
c := config.Default()
c.Testing.Coverage = "non-existant-file.xml"
c.Testing.Coverage.Report = "non-existant-file.xml"
return c
}()),
Expect: func(t *stdtesting.T, report api.Report, err error) {
Expand Down Expand Up @@ -281,7 +281,7 @@ func TestTestingLinter(t *stdtesting.T) {
Options: testutils.NewOptions().UsePythonFiles(createPythonFilenames(16).Concat(createPythonTestFilenames(4))).
WithConfig(func() *config.Config {
c := config.Default()
c.Testing.Coverage = "coverage-malformed.xml"
c.Testing.Coverage.Report = "coverage-malformed.xml"
return c
}()),
Expect: func(t *stdtesting.T, report api.Report, err error) {
Expand All @@ -298,7 +298,7 @@ func TestTestingLinter(t *stdtesting.T) {
WithConfig(func() *config.Config {
c := config.Default()
c.Testing.Report = "junit-empty.xml"
c.Testing.Coverage = "coverage-empty.xml"
c.Testing.Coverage.Report = "coverage-empty.xml"
return c
}()),
Expect: func(t *stdtesting.T, report api.Report, err error) {
Expand All @@ -318,8 +318,8 @@ func TestTestingLinter(t *stdtesting.T) {
Options: testutils.NewOptions().UsePythonFiles(createPythonFilenames(16).Concat(createPythonTestFilenames(4))).
WithConfig(func() *config.Config {
c := config.Default()
c.Testing.Coverage = "coverage-0.xml"
c.Testing.CoverageTarget = 0
c.Testing.Coverage.Report = "coverage-0.xml"
c.Testing.Coverage.Targets.Line = 0
return c
}()),
Expect: func(t *stdtesting.T, report api.Report, err error) {
Expand All @@ -335,6 +335,17 @@ func TestTestingLinter(t *stdtesting.T) {
suite.RunAll(t)
}

func TestTestingLinterConfigure(t *stdtesting.T) {
linter := testing.NewLinter()
conf := config.Default()
require.NoError(t, linter.Configure(conf))

conf.Testing.Coverage.Targets.Line = -1
require.ErrorIs(t, linter.Configure(conf), testing.ErrCoverageTargetTooLow)
conf.Testing.Coverage.Targets.Line = 200
require.ErrorIs(t, linter.Configure(conf), testing.ErrCoverageTargetTooHigh)
}

func createPythonFilenames(n int) utils.Filenames {
files := make(utils.Filenames, n)
for i := 0; i < n; i++ {
Expand Down

0 comments on commit 37a2f72

Please sign in to comment.