refactor: NewWorkflowPlanner (#648)

feat: add flag `--no-recurse` to disable recursion when reading workflows from directories
feat: added more tests to `TestPlanner`, renamed `TestJobFileInfo` to more appropriate name `WorkflowPlanTest`
style: changed error message to lowercase, added single quotes for better visibility

Co-authored-by: Casey Lee <cplee@nektos.com>

pkg/model/planner.go

@@ -50,34 +50,78 @@ func (r *Run) Job() *Job {
 	return r.Workflow.GetJob(r.JobID)
 }
 
-// NewWorkflowPlanner will load a specific workflow or all workflows from a directory
-func NewWorkflowPlanner(path string) (WorkflowPlanner, error) {
+type WorkflowFiles struct {
+	workflowFileInfo os.FileInfo
+	dirPath          string
+}
+
+// NewWorkflowPlanner will load a specific workflow, all workflows from a directory or all workflows from a directory and its subdirectories
+func NewWorkflowPlanner(path string, noWorkflowRecurse bool) (WorkflowPlanner, error) {
+	path, err := filepath.Abs(path)
+	if err != nil {
+		return nil, err
+	}
+
 	fi, err := os.Stat(path)
 	if err != nil {
 		return nil, err
 	}
 
-	var files []os.FileInfo
-	var dirname string
+	var workflows []WorkflowFiles
 
 	if fi.IsDir() {
 		log.Debugf("Loading workflows from '%s'", path)
-		dirname = path
-		files, err = ioutil.ReadDir(path)
+		if noWorkflowRecurse {
+			files, err := ioutil.ReadDir(path)
+			if err != nil {
+				return nil, err
+			}
+
+			for _, v := range files {
+				workflows = append(workflows, WorkflowFiles{
+					dirPath:          path,
+					workflowFileInfo: v,
+				})
+			}
+		} else {
+			log.Debug("Loading workflows recursively")
+			if err := filepath.Walk(path,
+				func(p string, f os.FileInfo, err error) error {
+					if err != nil {
+						return err
+					}
+
+					if !f.IsDir() {
+						log.Debugf("Found workflow '%s' in '%s'", f.Name(), p)
+						workflows = append(workflows, WorkflowFiles{
+							dirPath:          filepath.Dir(p),
+							workflowFileInfo: f,
+						})
+					}
+
+					return nil
+				}); err != nil {
+				return nil, err
+			}
+		}
 	} else {
 		log.Debugf("Loading workflow '%s'", path)
-		dirname, err = filepath.Abs(filepath.Dir(path))
-		files = []os.FileInfo{fi}
+		dirname := filepath.Dir(path)
+
+		workflows = append(workflows, WorkflowFiles{
+			dirPath:          dirname,
+			workflowFileInfo: fi,
+		})
 	}
 	if err != nil {
 		return nil, err
 	}
 
 	wp := new(workflowPlanner)
-	for _, file := range files {
-		ext := filepath.Ext(file.Name())
+	for _, wf := range workflows {
+		ext := filepath.Ext(wf.workflowFileInfo.Name())
 		if ext == ".yml" || ext == ".yaml" {
-			f, err := os.Open(filepath.Join(dirname, file.Name()))
+			f, err := os.Open(filepath.Join(wf.dirPath, wf.workflowFileInfo.Name()))
 			if err != nil {
 				return nil, err
 			}
@@ -87,19 +131,22 @@ func NewWorkflowPlanner(path string) (WorkflowPlanner, error) {
 			if err != nil {
 				f.Close()
 				if err == io.EOF {
-					return nil, errors.WithMessagef(err, "unable to read workflow, %s file is empty", file.Name())
+					return nil, errors.WithMessagef(err, "unable to read workflow, %s file is empty", wf.workflowFileInfo.Name())
 				}
 				return nil, err
 			}
+
 			if workflow.Name == "" {
-				workflow.Name = file.Name()
+				workflow.Name = wf.workflowFileInfo.Name()
 			}
+
 			jobNameRegex := regexp.MustCompile(`^([[:alpha:]_][[:alnum:]_\-]*)$`)
 			for k := range workflow.Jobs {
 				if ok := jobNameRegex.MatchString(k); !ok {
-					return nil, fmt.Errorf("The workflow is not valid. %s: Job name %s is invalid. Names must start with a letter or '_' and contain only alphanumeric characters, '-', or '_'", workflow.Name, k)
+					return nil, fmt.Errorf("workflow is not valid. '%s': Job name '%s' is invalid. Names must start with a letter or '_' and contain only alphanumeric characters, '-', or '_'", workflow.Name, k)
 				}
 			}
+
 			wp.workflows = append(wp.workflows, workflow)
 			f.Close()
 		}

pkg/model/planner_test.go

@@ -8,25 +8,30 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-type TestJobFileInfo struct {
-	workflowPath string
-	errorMessage string
+type WorkflowPlanTest struct {
+	workflowPath      string
+	errorMessage      string
+	noWorkflowRecurse bool
 }
 
 func TestPlanner(t *testing.T) {
-	tables := []TestJobFileInfo{
-		{"invalid-job-name", "The workflow is not valid. invalid-job-name: Job name invalid-JOB-Name-v1.2.3-docker_hub is invalid. Names must start with a letter or '_' and contain only alphanumeric characters, '-', or '_'"},
-		{"empty-workflow", "unable to read workflow, push.yml file is empty: EOF"},
-
-		{"", ""}, // match whole directory
-	}
 	log.SetLevel(log.DebugLevel)
 
+	tables := []WorkflowPlanTest{
+		{"invalid-job-name/invalid-1.yml", "workflow is not valid. 'invalid-job-name-1': Job name 'invalid-JOB-Name-v1.2.3-docker_hub' is invalid. Names must start with a letter or '_' and contain only alphanumeric characters, '-', or '_'", false},
+		{"invalid-job-name/invalid-2.yml", "workflow is not valid. 'invalid-job-name-2': Job name '1234invalid-JOB-Name-v123-docker_hub' is invalid. Names must start with a letter or '_' and contain only alphanumeric characters, '-', or '_'", false},
+		{"invalid-job-name/valid-1.yml", "", false},
+		{"invalid-job-name/valid-2.yml", "", false},
+		{"empty-workflow", "unable to read workflow, push.yml file is empty: EOF", false},
+		{"nested", "unable to read workflow, fail.yml file is empty: EOF", false},
+		{"nested", "", true},
+	}
+
 	workdir, err := filepath.Abs("testdata")
 	assert.NoError(t, err, workdir)
 	for _, table := range tables {
 		fullWorkflowPath := filepath.Join(workdir, table.workflowPath)
-		_, err = NewWorkflowPlanner(fullWorkflowPath)
+		_, err = NewWorkflowPlanner(fullWorkflowPath, table.noWorkflowRecurse)
 		if table.errorMessage == "" {
 			assert.NoError(t, err, "WorkflowPlanner should exit without any error")
 		} else {

pkg/model/testdata/invalid-job-name/invalid-1.yml

@@ -1,4 +1,4 @@
-name: invalid-job-name
+name: invalid-job-name-1
 on: push
 
 jobs:

pkg/model/testdata/invalid-job-name/invalid-2.yml

@@ -0,0 +1,8 @@
+name: invalid-job-name-2
+on: push
+
+jobs:
+  1234invalid-JOB-Name-v123-docker_hub:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo hi

pkg/model/testdata/invalid-job-name/valid-1.yml

@@ -0,0 +1,8 @@
+name: valid-job-name-1
+on: push
+
+jobs:
+  valid-JOB-Name-v123-docker_hub:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo hi

pkg/model/testdata/invalid-job-name/valid-2.yml

@@ -0,0 +1,8 @@
+name: valid-job-name-2
+on: push
+
+jobs:
+  ___valid-JOB-Name-v123-docker_hub:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo hi

pkg/model/testdata/nested/success.yml

@@ -0,0 +1,9 @@
+name: Hello World Workflow
+on: push
+
+jobs:
+  hello-world:
+    name: Hello World Job
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo "Hello World!"