Skip to content

Commit

Permalink
Starts implementing a linter for the Testing category: user should pr…
Browse files Browse the repository at this point in the history
…ovide JUnit XML test report and Cobertura-compatible XML coverage report that will be analysed for test completion and coverage. Implements checking test completion stats from JUnit XML
  • Loading branch information
bvobart committed Jun 2, 2021
1 parent 498dedd commit 120fc8e
Show file tree
Hide file tree
Showing 8 changed files with 218 additions and 0 deletions.
21 changes: 21 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type Config struct {
Rules RuleConfig `yaml:"rules" toml:"rules"`
Git GitConfig `yaml:"git" toml:"git"`
CodeQuality CodeQualityConfig `yaml:"code-quality" toml:"code-quality"`
Testing TestingConfig `yaml:"testing" toml:"testing"`
}

// RuleConfig contains info about which rules are enabled / disabled.
Expand All @@ -32,19 +33,37 @@ type GitConfig struct {
MaxFileSize uint64 `yaml:"maxFileSize" toml:"maxFileSize"`
}

// CodeQualityConfig contains the configuration for the CQ linters used in the Code Quality category
type CodeQualityConfig struct {
// Defines all code linters to use in the Code Quality category
Linters []string `yaml:"linters" toml:"linters"`
}

// TestingConfig contains the configuration for the rules in the Testing category.
type TestingConfig struct {
// Filename of the project's test execution report, either absolute or relative to the project's root.
// Expects a JUnit XML file, which when using `pytest` can be generated with `pytest --junitxml=tests-report.xml`
Report string `yaml:"report" toml:"report"`

// 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"`
}

//---------------------------------------------------------------------------------------

func Default() *Config {
return &Config{
Rules: RuleConfig{Disabled: []string{}},
Git: GitConfig{MaxFileSize: 10_000_000}, // 10 MB
CodeQuality: CodeQualityConfig{Linters: []string{"pylint", "mypy", "black", "isort", "bandit"}},
Testing: TestingConfig{},
}
}

//---------------------------------------------------------------------------------------

type FileType string

const (
Expand All @@ -60,6 +79,8 @@ func (t FileType) String() string {
return string(t)
}

//---------------------------------------------------------------------------------------

// ParseFromDir parses the mllint config from the given project directory.
// If an `.mllint.yml` file is present, then this will be used,
// otherwise, if a `pyproject.toml` file is present, then this will be used,
Expand Down
34 changes: 34 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ code-quality:
- black
`

const yamlTesting = `
testing:
report: tests-report.xml
coverage: coverage-report.xml
`

const yamlInvalid = `
rules:
disabled: nothing
Expand All @@ -49,6 +55,12 @@ const tomlLinters = `
linters = ["pylint", "mypy"]
`

const tomlTesting = `
[tool.mllint.testing]
report = "tests-report.xml"
coverage = "coverage-report.xml"
`

const tomlInvalid = `
[tool.mllint.rules]
disabled = "nothing"
Expand Down Expand Up @@ -97,6 +109,17 @@ func TestParseYAML(t *testing.T) {
}(),
Err: nil,
},
{
Name: "YamlTesting",
File: strings.NewReader(yamlTesting),
Expected: func() *config.Config {
c := config.Default()
c.Testing.Report = "tests-report.xml"
c.Testing.Coverage = "coverage-report.xml"
return c
}(),
Err: nil,
},
{
Name: "YamlError",
File: strings.NewReader(yamlInvalid),
Expand Down Expand Up @@ -166,6 +189,17 @@ func TestParseTOML(t *testing.T) {
}(),
Err: nil,
},
{
Name: "TomlTesting",
File: strings.NewReader(tomlTesting),
Expected: func() *config.Config {
c := config.Default()
c.Testing.Report = "tests-report.xml"
c.Testing.Coverage = "coverage-report.xml"
return c
}(),
Err: nil,
},
{
Name: "TomlError",
File: strings.NewReader(tomlInvalid),
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ require (
github.com/gosuri/uilive v0.0.4
github.com/hashicorp/go-multierror v1.1.1
github.com/hhatto/gocloc v0.4.1
github.com/joshdk/go-junit v0.0.0-20210226021600-6145f504ca0d // indirect
github.com/juliangruber/go-intersect v1.0.0
github.com/mattn/go-isatty v0.0.12
github.com/nathan-fiscaletti/consolesize-go v0.0.0-20210105204122-a87d9f614b9d
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NH
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo=
github.com/joshdk/go-junit v0.0.0-20210226021600-6145f504ca0d h1:lcSbmPJf3b19MTZtGDLI6Y2Jnk3VBDT8UG/8IVCEMxA=
github.com/joshdk/go-junit v0.0.0-20210226021600-6145f504ca0d/go.mod h1:TiiV0PqkaNfFXjEiyjWM3XXrhVyCa1K4Zfga6W52ung=
github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU=
github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU=
github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU=
Expand Down
110 changes: 110 additions & 0 deletions linters/testing/linter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
package template

import (
"fmt"
"path"
"strings"

"github.com/joshdk/go-junit"

"github.com/bvobart/mllint/api"
"github.com/bvobart/mllint/categories"
"github.com/bvobart/mllint/config"
"github.com/bvobart/mllint/utils"
)

func NewLinter() api.ConfigurableLinter {
return &TestingLinter{}
}

type TestingLinter struct {
Config config.TestingConfig
}

func (l *TestingLinter) Name() string {
return categories.Testing.Name
}

func (l *TestingLinter) Configure(conf *config.Config) error {
l.Config = conf.Testing
return nil
}

func (l *TestingLinter) Rules() []*api.Rule {
return []*api.Rule{&RuleHasTests, &RuleTestsPass, &RuleTestCoverage, &RuleTestsFolder}
}

func (l *TestingLinter) LintProject(project api.Project) (api.Report, error) {
report := api.NewReport()

// TODO: find and count the amount of test files to score RuleHasTests
l.ScoreRuleHasTests(&report, project)
l.ScoreRuleTestsPass(&report, project)

// TODO: implement the linting for RuleTestCoverage, which checks whether there is a Cobertura XML coverage report and analyses it for test coverage.
// TODO: check whether all test files are in tests folder.

return report, nil
}

func (l *TestingLinter) ScoreRuleHasTests(report *api.Report, project api.Project) {
testFiles := project.PythonFiles.Filter(func(filename string) bool {
return strings.HasSuffix(filename, "_test.py") || strings.HasPrefix(path.Base(filename), "test_")
})

// there should be at 1 test file per 4 non-test Python files.
report.Scores[RuleHasTests] = 100 * (float64(len(testFiles)) * 4 / float64(len(project.PythonFiles)-len(testFiles)))
}

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

if !utils.FileExists(l.Config.Report) {
report.Scores[RuleTestsPass] = 0
report.Details[RuleTestsPass] = fmt.Sprintf("A test report was provided, namely `%s`, but this file could not be found. Please update the `testing.report` setting in your project's `mllint` configuration to fix the path to your project's test report.", l.Config.Report)
return
}

suites, err := junit.IngestFile(l.Config.Report)
if err != nil {
report.Scores[RuleTestsPass] = 0
report.Details[RuleTestsPass] = fmt.Sprintf(`A test report file `+"`%s`"+` was provided and found, but there was an error parsing the JUnit XML contents:
%s
Please make sure your test report file is a valid JUnit XML file. %s`, l.Config.Report, "```\n"+err.Error()+"\n```", howToMakeJUnitXML)
return
}

passedTests := 0
totalTests := 0
for _, suite := range suites {
totalTests += suite.Totals.Tests
passedTests += suite.Totals.Passed
}

if totalTests == 0 {
report.Scores[RuleTestsPass] = 0
report.Details[RuleTestsPass] = fmt.Sprintf(`No tests were run, according to the provided test report file `+"`%s`"+`. Don't be shy, implement some tests!`, l.Config.Report)
}

score := 100 * float64(passedTests) / float64(totalTests)
report.Scores[RuleTestsPass] = score
if passedTests == totalTests {
report.Details[RuleTestsPass] = fmt.Sprintf("Congratulations, all of your project's %d tests passed!", totalTests)
} else if passedTests == 0 {
report.Details[RuleTestsPass] = fmt.Sprintf("Oh no! What a shame... **None** of your project's %d tests passed! There must be something terribly wrong.", totalTests)
} else if score < 0.25 {
report.Details[RuleTestsPass] = fmt.Sprintf("Oh no! Only **%d** out of **%d** tests passed... That's less than a quarter of all tests in your project...", passedTests, totalTests)
} else if score > 0.75 {
report.Details[RuleTestsPass] = fmt.Sprintf("Hmm, only **%d** out of **%d** of your project's tests passed... That's over three quarter of all tests in your project, but it's not enough: all tests must pass. Good luck fixing the broken tests!", passedTests, totalTests)
} else {
report.Details[RuleTestsPass] = fmt.Sprintf("Oh my, only **%d** out of **%d** of your project's tests passed... You can do better, right? Good luck fixing those tests!", passedTests, totalTests)
}
}

const howToMakeJUnitXML = "When using `pytest` to run your project's tests, use the `--junitxml=<filename>` option to generate such a test report, e.g.: `pytest --junitxml=tests-report.xml`"
31 changes: 31 additions & 0 deletions linters/testing/rules.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package template

import "github.com/bvobart/mllint/api"

var RuleHasTests = api.Rule{
Name: "Project has automated tests",
Slug: "testing/has-tests",
Details: "TODO",
Weight: 1,
}

var RuleTestsPass = api.Rule{
Name: "Project passes all of its automated tests",
Slug: "testing/pass",
Details: "TODO",
Weight: 1,
}

var RuleTestCoverage = api.Rule{
Name: "Project provides a test coverage report",
Slug: "testing/coverage",
Details: "TODO: fill this in with details about what the rule checks, what the reasoning behind it is, where people can get more information on how to implement what this rule checks.",
Weight: 1,
}

var RuleTestsFolder = api.Rule{
Name: "Tests should be placed in the tests folder",
Slug: "testing/tests-folder",
Details: "TODO",
Weight: 1,
}
10 changes: 10 additions & 0 deletions utils/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,16 @@ func FindFilesByExtInDir(dir string, extension string) (Filenames, error) {
// Filenames is simply an alias for []string, but allows me to add some methods.
type Filenames []string

func (names Filenames) Filter(shouldInclude func(filename string) bool) Filenames {
result := Filenames{}
for _, filename := range names {
if shouldInclude(filename) {
result = append(result, filename)
}
}
return result
}

// Prefix prefixes each of the filenames with a directory name.
// i.e. Filenames{"name.py"}.Prefix("something") becomes Filenames{"something/name.py"}
func (names Filenames) Prefix(dir string) Filenames {
Expand Down
9 changes: 9 additions & 0 deletions utils/files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package utils_test
import (
"os"
"path"
"strings"
"testing"

"github.com/bvobart/mllint/utils"
Expand Down Expand Up @@ -72,6 +73,14 @@ func TestFindPythonFiles(t *testing.T) {
require.Equal(t, utils.Filenames{"some_other_script.py", "some_script.py", "subfolder/yet_another_script.py"}, files)
}

func TestFilenamesFilter(t *testing.T) {
files := utils.Filenames{"some_script.py", "some_other_script.py", "subfolder/yet_another_script.py"}
files = files.Filter(func(filename string) bool {
return strings.Contains(filename, "other")
})
require.Equal(t, utils.Filenames{"some_other_script.py", "subfolder/yet_another_script.py"}, files)
}

func TestCountLoC(t *testing.T) {
files := utils.Filenames{"some_script.py", "some_other_script.py", "subfolder/yet_another_script.py"}
files = files.Prefix(path.Join("test-resources", "python-files"))
Expand Down

0 comments on commit 120fc8e

Please sign in to comment.