diff --git a/config/config.go b/config/config.go index e3b59eb..9a432d0 100644 --- a/config/config.go +++ b/config/config.go @@ -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"` } //--------------------------------------------------------------------------------------- @@ -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, + }, + }, + }, } } diff --git a/config/config_test.go b/config/config_test.go index ae6c7e8..efcf2ca 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -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 = ` @@ -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 = ` @@ -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, @@ -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, diff --git a/linters/testing/linter.go b/linters/testing/linter.go index 730078b..74eb5fc 100644 --- a/linters/testing/linter.go +++ b/linters/testing/linter.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "math" "path" "strings" @@ -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 } @@ -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 } @@ -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) + } } //--------------------------------------------------------------------------------------- @@ -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 } @@ -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." } diff --git a/linters/testing/linter_test.go b/linters/testing/linter_test.go index 7e37e5c..e3454a1 100644 --- a/linters/testing/linter_test.go +++ b/linters/testing/linter_test.go @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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++ {