From 0a027df50d56ba90338733acd785597a2e442f2b Mon Sep 17 00:00:00 2001 From: Pete Davison Date: Tue, 5 Sep 2023 23:55:56 +0000 Subject: [PATCH] feat: better error handling for duplicate edges and fixed tests --- errors/errors.go | 1 + errors/errors_taskfile.go | 17 +++++++++++++++++ task_test.go | 4 ++-- taskfile/reader.go | 14 +++++++++++++- testdata/includes/Taskfile.yml | 12 ++++++------ testdata/includes/module1/Taskfile.yml | 4 ---- testdata/includes/module2/Taskfile.yml | 6 +----- testdata/includes/module3/Taskfile.yml | 6 ++++++ testdata/includes/module4/Taskfile.yml | 6 ++++++ 9 files changed, 52 insertions(+), 18 deletions(-) create mode 100644 testdata/includes/module3/Taskfile.yml create mode 100644 testdata/includes/module4/Taskfile.yml diff --git a/errors/errors.go b/errors/errors.go index df684a47..46ad328b 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -19,6 +19,7 @@ const ( CodeTaskfileCacheNotFound CodeTaskfileVersionCheckError CodeTaskfileNetworkTimeout + CodeTaskfileDuplicateInclude ) // Task related exit codes diff --git a/errors/errors_taskfile.go b/errors/errors_taskfile.go index f080c4ef..2951bf90 100644 --- a/errors/errors_taskfile.go +++ b/errors/errors_taskfile.go @@ -3,6 +3,7 @@ package errors import ( "fmt" "net/http" + "strings" "time" "github.com/Masterminds/semver/v3" @@ -174,3 +175,19 @@ func (err *TaskfileNetworkTimeoutError) Error() string { func (err *TaskfileNetworkTimeoutError) Code() int { return CodeTaskfileNetworkTimeout } + +type TaskfileDuplicateIncludeError struct { + URI string + IncludedURI string + Namespaces []string +} + +func (err *TaskfileDuplicateIncludeError) Error() string { + return fmt.Sprintf( + `task: Taskfile %q attempted to include %q multiple times with namespaces: %s`, err.URI, err.IncludedURI, strings.Join(err.Namespaces, ", "), + ) +} + +func (err *TaskfileDuplicateIncludeError) Code() int { + return CodeTaskfileDuplicateInclude +} diff --git a/task_test.go b/task_test.go index 28fded0d..99b16502 100644 --- a/task_test.go +++ b/task_test.go @@ -981,8 +981,8 @@ func TestIncludes(t *testing.T) { "included_directory.txt": "included_directory", "included_directory_without_dir.txt": "included_directory_without_dir", "included_taskfile_without_dir.txt": "included_taskfile_without_dir", - "./module2/included_directory_with_dir.txt": "included_directory_with_dir", - "./module2/included_taskfile_with_dir.txt": "included_taskfile_with_dir", + "./module3/included_taskfile_with_dir.txt": "included_taskfile_with_dir", + "./module4/included_directory_with_dir.txt": "included_directory_with_dir", "os_include.txt": "os", }, } diff --git a/taskfile/reader.go b/taskfile/reader.go index b194ef08..790c991a 100644 --- a/taskfile/reader.go +++ b/taskfile/reader.go @@ -141,7 +141,19 @@ func (r *Reader) include(node Node) error { } // Create an edge between the Taskfiles - return r.graph.AddEdge(node.Location(), includeNode.Location()) + err = r.graph.AddEdge(node.Location(), includeNode.Location()) + if errors.Is(err, graph.ErrEdgeAlreadyExists) { + edge, err := r.graph.Edge(node.Location(), includeNode.Location()) + if err != nil { + return err + } + return &errors.TaskfileDuplicateIncludeError{ + URI: node.Location(), + IncludedURI: includeNode.Location(), + Namespaces: []string{namespace, edge.Properties.Data.(*ast.Include).Namespace}, + } + } + return err }) return nil }) diff --git a/testdata/includes/Taskfile.yml b/testdata/includes/Taskfile.yml index 8ed9e416..0d1efec9 100644 --- a/testdata/includes/Taskfile.yml +++ b/testdata/includes/Taskfile.yml @@ -6,13 +6,13 @@ includes: included_without_dir: taskfile: ./module1 included_taskfile_without_dir: - taskfile: ./module1/Taskfile.yml - included_with_dir: - taskfile: ./module2 - dir: ./module2 - included_taskfile_with_dir: taskfile: ./module2/Taskfile.yml - dir: ./module2 + included_with_dir: + taskfile: ./module3 + dir: ./module3 + included_taskfile_with_dir: + taskfile: ./module4/Taskfile.yml + dir: ./module4 included_os: ./Taskfile_{{OS}}.yml tasks: diff --git a/testdata/includes/module1/Taskfile.yml b/testdata/includes/module1/Taskfile.yml index 3659073e..34596e52 100644 --- a/testdata/includes/module1/Taskfile.yml +++ b/testdata/includes/module1/Taskfile.yml @@ -1,10 +1,6 @@ version: '3' tasks: - gen_dir: - cmds: - - echo included_directory_without_dir > included_directory_without_dir.txt - gen_file: cmds: - echo included_taskfile_without_dir > included_taskfile_without_dir.txt diff --git a/testdata/includes/module2/Taskfile.yml b/testdata/includes/module2/Taskfile.yml index 09bbdb60..26e749bf 100644 --- a/testdata/includes/module2/Taskfile.yml +++ b/testdata/includes/module2/Taskfile.yml @@ -3,8 +3,4 @@ version: '3' tasks: gen_dir: cmds: - - echo included_directory_with_dir > included_directory_with_dir.txt - - gen_file: - cmds: - - echo included_taskfile_with_dir > included_taskfile_with_dir.txt + - echo included_directory_without_dir > included_directory_without_dir.txt diff --git a/testdata/includes/module3/Taskfile.yml b/testdata/includes/module3/Taskfile.yml new file mode 100644 index 00000000..619f1986 --- /dev/null +++ b/testdata/includes/module3/Taskfile.yml @@ -0,0 +1,6 @@ +version: '3' + +tasks: + gen_file: + cmds: + - echo included_taskfile_with_dir > included_taskfile_with_dir.txt diff --git a/testdata/includes/module4/Taskfile.yml b/testdata/includes/module4/Taskfile.yml new file mode 100644 index 00000000..25cb62cd --- /dev/null +++ b/testdata/includes/module4/Taskfile.yml @@ -0,0 +1,6 @@ +version: '3' + +tasks: + gen_dir: + cmds: + - echo included_directory_with_dir > included_directory_with_dir.txt