Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 1 addition & 21 deletions engine/linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ import (
"github.com/drone/runner-go/manifest"
)

// ErrDuplicateStepName is returned when two Pipeline steps
// have the same name.
var ErrDuplicateStepName = errors.New("linter: duplicate step names")

// Opts provides linting options.
type Opts struct {
Trusted bool
Expand Down Expand Up @@ -53,22 +49,6 @@ func checkPipeline(pipeline *resource.Pipeline, trusted bool) error {
return nil
}

// func checkNames(pipeline *resource.Pipeline) error {
// names := map[string]struct{}{}
// if !pipeline.Clone.Disable {
// names["clone"] = struct{}{}
// }
// steps := append(pipeline.Services, pipeline.Steps...)
// for _, step := range steps {
// _, ok := names[step.Name]
// if ok {
// return ErrDuplicateStepName
// }
// names[step.Name] = struct{}{}
// }
// return nil
// }

func checkSteps(pipeline *resource.Pipeline, trusted bool) error {
steps := append(pipeline.Services, pipeline.Steps...)
names := map[string]struct{}{}
Expand All @@ -83,7 +63,7 @@ func checkSteps(pipeline *resource.Pipeline, trusted bool) error {
// unique list of names
_, ok := names[step.Name]
if ok {
return ErrDuplicateStepName
return fmt.Errorf("linter: duplicate step names (%s)", step.Name)
}
names[step.Name] = struct{}{}

Expand Down
11 changes: 6 additions & 5 deletions engine/linter/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,17 @@ func TestLint(t *testing.T) {
// message: "linter: duplicate step names",
// },
// {
// path: "testdata/duplicate_step_service.yml",
// invalid: true,
// message: "linter: duplicate step names",
// },
// {
// path: "testdata/missing_name.yml",
// invalid: true,
// message: "linter: invalid or missing name",
// },

{
path: "testdata/duplicate_step_service.yml",
invalid: true,
message: "linter: duplicate step names (test)",
},

{
path: "testdata/missing_dep.yml",
invalid: true,
Expand Down
5 changes: 3 additions & 2 deletions engine/resource/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package resource

import (
"errors"
"fmt"

"github.com/drone/runner-go/manifest"

Expand Down Expand Up @@ -47,10 +48,10 @@ func lint(pipeline *Pipeline) error {
return errors.New("Linter: invalid or missing step name")
}
if len(step.Name) > 100 {
return errors.New("Linter: step name cannot exceed 100 characters")
return fmt.Errorf("Linter: step name (%s) cannot exceed 100 characters", step.Name)
}
if _, ok := names[step.Name]; ok {
return errors.New("Linter: duplicate step name")
return fmt.Errorf("Linter: duplicate step name (%s)", step.Name)
}
names[step.Name] = struct{}{}
}
Expand Down
23 changes: 22 additions & 1 deletion engine/resource/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package resource

import (
"fmt"
"strings"
"testing"

"github.com/drone/runner-go/manifest"
Expand Down Expand Up @@ -48,7 +50,7 @@ func TestParse(t *testing.T) {
Clone: manifest.Clone{
Depth: 50,
},
Deps: []string{"dependency"},
Deps: []string{"dependency"},
PullSecrets: []string{"dockerconfigjson"},
Trigger: manifest.Conditions{
Branch: manifest.Condition{
Expand Down Expand Up @@ -170,4 +172,23 @@ func TestLint(t *testing.T) {
if err := lint(p); err == nil {
t.Errorf("Expect error when empty name")
}

aVeryLongName := "a-" + strings.Repeat("very-", 100) + "long-name"
p.Steps = []*Step{{Name: aVeryLongName}}
err := lint(p)
if err == nil {
t.Errorf("Expect error when a name exceeds 100 characters")
}
if err.Error() != fmt.Sprintf("Linter: step name (%s) cannot exceed 100 characters", aVeryLongName) {
t.Errorf("Expected error about a very long name, got: %s", err.Error())
}

p.Steps = []*Step{{Name: "build"}, {Name: "build"}}
err = lint(p)
if err == nil {
t.Errorf("Expect error when duplicate name")
}
if err.Error() != "Linter: duplicate step name (build)" {
t.Errorf("Expected duplicate step name error, got: %s", err.Error())
}
}