Skip to content

Commit

Permalink
dev: add test on ExcludeRule and SeverityRule (#4420)
Browse files Browse the repository at this point in the history
  • Loading branch information
ldez authored Feb 27, 2024
1 parent 9ecfc05 commit 3452e2e
Show file tree
Hide file tree
Showing 3 changed files with 288 additions and 1 deletion.
15 changes: 15 additions & 0 deletions pkg/config/issues.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package config

import (
"errors"
"fmt"
"regexp"
)
Expand Down Expand Up @@ -141,41 +142,55 @@ func (b *BaseRule) Validate(minConditionsCount int) error {
if err := validateOptionalRegex(b.Path); err != nil {
return fmt.Errorf("invalid path regex: %w", err)
}

if err := validateOptionalRegex(b.PathExcept); err != nil {
return fmt.Errorf("invalid path-except regex: %w", err)
}

if err := validateOptionalRegex(b.Text); err != nil {
return fmt.Errorf("invalid text regex: %w", err)
}

if err := validateOptionalRegex(b.Source); err != nil {
return fmt.Errorf("invalid source regex: %w", err)
}

if b.Path != "" && b.PathExcept != "" {
return errors.New("path and path-except should not be set at the same time")
}

nonBlank := 0
if len(b.Linters) > 0 {
nonBlank++
}

// Filtering by path counts as one condition, regardless how it is done (one or both).
// Otherwise, a rule with Path and PathExcept set would pass validation
// whereas before the introduction of path-except that wouldn't have been precise enough.
if b.Path != "" || b.PathExcept != "" {
nonBlank++
}

if b.Text != "" {
nonBlank++
}

if b.Source != "" {
nonBlank++
}

if nonBlank < minConditionsCount {
return fmt.Errorf("at least %d of (text, source, path[-except], linters) should be set", minConditionsCount)
}

return nil
}

func validateOptionalRegex(value string) error {
if value == "" {
return nil
}

_, err := regexp.Compile(value)
return err
}
Expand Down
186 changes: 185 additions & 1 deletion pkg/config/issues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestGetExcludePatterns(t *testing.T) {
assert.Equal(t, GetExcludePatterns(nil), DefaultExcludePatterns)
patterns := GetExcludePatterns(nil)

assert.Equal(t, DefaultExcludePatterns, patterns)
}

func TestGetExcludePatterns_includes(t *testing.T) {
include := []string{DefaultExcludePatterns[0].ID, DefaultExcludePatterns[1].ID}

exclude := GetExcludePatterns(include)
Expand All @@ -19,3 +24,182 @@ func TestGetExcludePatterns(t *testing.T) {
assert.Contains(t, DefaultExcludePatterns, p)
}
}

func TestExcludeRule_Validate(t *testing.T) {
testCases := []struct {
desc string
rule *ExcludeRule
expected string
}{
{
desc: "empty rule",
rule: &ExcludeRule{},
expected: "at least 2 of (text, source, path[-except], linters) should be set",
},
{
desc: "only path rule",
rule: &ExcludeRule{
BaseRule{
Path: "test",
},
},
expected: "at least 2 of (text, source, path[-except], linters) should be set",
},
{
desc: "only path-except rule",
rule: &ExcludeRule{
BaseRule{
PathExcept: "test",
},
},
expected: "at least 2 of (text, source, path[-except], linters) should be set",
},
{
desc: "only text rule",
rule: &ExcludeRule{
BaseRule{
Text: "test",
},
},
expected: "at least 2 of (text, source, path[-except], linters) should be set",
},
{
desc: "only source rule",
rule: &ExcludeRule{
BaseRule{
Source: "test",
},
},
expected: "at least 2 of (text, source, path[-except], linters) should be set",
},
{
desc: "invalid path rule",
rule: &ExcludeRule{
BaseRule{
Path: "**test",
},
},
expected: "invalid path regex: error parsing regexp: missing argument to repetition operator: `*`",
},
{
desc: "invalid path-except rule",
rule: &ExcludeRule{
BaseRule{
PathExcept: "**test",
},
},
expected: "invalid path-except regex: error parsing regexp: missing argument to repetition operator: `*`",
},
{
desc: "invalid text rule",
rule: &ExcludeRule{
BaseRule{
Text: "**test",
},
},
expected: "invalid text regex: error parsing regexp: missing argument to repetition operator: `*`",
},
{
desc: "invalid source rule",
rule: &ExcludeRule{
BaseRule{
Source: "**test",
},
},
expected: "invalid source regex: error parsing regexp: missing argument to repetition operator: `*`",
},
{
desc: "path and path-expect",
rule: &ExcludeRule{
BaseRule{
Path: "test",
PathExcept: "test",
},
},
expected: "path and path-except should not be set at the same time",
},
}

for _, test := range testCases {
test := test
t.Run(test.desc, func(t *testing.T) {
t.Parallel()

err := test.rule.Validate()
require.EqualError(t, err, test.expected)
})
}
}

func TestExcludeRule_Validate_error(t *testing.T) {
testCases := []struct {
desc string
rule *ExcludeRule
}{
{
desc: "path and linter",
rule: &ExcludeRule{
BaseRule{
Path: "test",
Linters: []string{"a"},
},
},
},
{
desc: "path-except and linter",
rule: &ExcludeRule{
BaseRule{
PathExcept: "test",
Linters: []string{"a"},
},
},
},
{
desc: "text and linter",
rule: &ExcludeRule{
BaseRule{
Text: "test",
Linters: []string{"a"},
},
},
},
{
desc: "source and linter",
rule: &ExcludeRule{
BaseRule{
Source: "test",
Linters: []string{"a"},
},
},
},
{
desc: "path and text",
rule: &ExcludeRule{
BaseRule{
Path: "test",
Text: "test",
},
},
},
{
desc: "path and text and linter",
rule: &ExcludeRule{
BaseRule{
Path: "test",
Text: "test",
Linters: []string{"a"},
},
},
},
}

for _, test := range testCases {
test := test
t.Run(test.desc, func(t *testing.T) {
t.Parallel()

err := test.rule.Validate()
require.NoError(t, err)
})
}
}
88 changes: 88 additions & 0 deletions pkg/config/severity_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package config

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestSeverity_Validate(t *testing.T) {
rule := &SeverityRule{
BaseRule: BaseRule{
Path: "test",
},
}

err := rule.Validate()
require.NoError(t, err)
}

func TestSeverity_Validate_error(t *testing.T) {
testCases := []struct {
desc string
rule *SeverityRule
expected string
}{
{
desc: "empty rule",
rule: &SeverityRule{},
expected: "at least 1 of (text, source, path[-except], linters) should be set",
},
{
desc: "invalid path rule",
rule: &SeverityRule{
BaseRule: BaseRule{
Path: "**test",
},
},
expected: "invalid path regex: error parsing regexp: missing argument to repetition operator: `*`",
},
{
desc: "invalid path-except rule",
rule: &SeverityRule{
BaseRule: BaseRule{
PathExcept: "**test",
},
},
expected: "invalid path-except regex: error parsing regexp: missing argument to repetition operator: `*`",
},
{
desc: "invalid text rule",
rule: &SeverityRule{
BaseRule: BaseRule{
Text: "**test",
},
},
expected: "invalid text regex: error parsing regexp: missing argument to repetition operator: `*`",
},
{
desc: "invalid source rule",
rule: &SeverityRule{
BaseRule: BaseRule{
Source: "**test",
},
},
expected: "invalid source regex: error parsing regexp: missing argument to repetition operator: `*`",
},
{
desc: "path and path-expect",
rule: &SeverityRule{
BaseRule: BaseRule{
Path: "test",
PathExcept: "test",
},
},
expected: "path and path-except should not be set at the same time",
},
}

for _, test := range testCases {
test := test
t.Run(test.desc, func(t *testing.T) {
t.Parallel()

err := test.rule.Validate()
require.EqualError(t, err, test.expected)
})
}
}

0 comments on commit 3452e2e

Please sign in to comment.