refactor: re-organize node loading code to make it easier to follow (#1771)

task_test.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	rand "math/rand/v2"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -1047,6 +1048,107 @@ func TestIncludesMultiLevel(t *testing.T) {
 	tt.Run(t)
 }
 
+func TestIncludesRemote(t *testing.T) {
+	enableExperimentForTest(t, &experiments.RemoteTaskfiles, "1")
+
+	dir := "testdata/includes_remote"
+
+	srv := httptest.NewServer(http.FileServer(http.Dir(dir)))
+	defer srv.Close()
+
+	tcs := []struct {
+		firstRemote  string
+		secondRemote string
+	}{
+		{
+			firstRemote:  srv.URL + "/first/Taskfile.yml",
+			secondRemote: srv.URL + "/first/second/Taskfile.yml",
+		},
+		{
+			firstRemote:  srv.URL + "/first/Taskfile.yml",
+			secondRemote: "./second/Taskfile.yml",
+		},
+	}
+
+	tasks := []string{
+		"first:write-file",
+		"first:second:write-file",
+	}
+
+	for i, tc := range tcs {
+		t.Run(fmt.Sprint(i), func(t *testing.T) {
+			t.Setenv("FIRST_REMOTE_URL", tc.firstRemote)
+			t.Setenv("SECOND_REMOTE_URL", tc.secondRemote)
+
+			var buff SyncBuffer
+
+			executors := []struct {
+				name     string
+				executor *task.Executor
+			}{
+				{
+					name: "online, always download",
+					executor: &task.Executor{
+						Dir:      dir,
+						Stdout:   &buff,
+						Stderr:   &buff,
+						Timeout:  time.Minute,
+						Insecure: true,
+						Logger:   &logger.Logger{Stdout: &buff, Stderr: &buff, Verbose: true},
+
+						// Without caching
+						AssumeYes: true,
+						Download:  true,
+					},
+				},
+				{
+					name: "offline, use cache",
+					executor: &task.Executor{
+						Dir:      dir,
+						Stdout:   &buff,
+						Stderr:   &buff,
+						Timeout:  time.Minute,
+						Insecure: true,
+						Logger:   &logger.Logger{Stdout: &buff, Stderr: &buff, Verbose: true},
+
+						// With caching
+						AssumeYes: false,
+						Download:  false,
+						Offline:   true,
+					},
+				},
+			}
+
+			for j, e := range executors {
+				t.Run(fmt.Sprint(j), func(t *testing.T) {
+					require.NoError(t, e.executor.Setup())
+
+					for k, task := range tasks {
+						t.Run(task, func(t *testing.T) {
+							expectedContent := fmt.Sprint(rand.Int64())
+							t.Setenv("CONTENT", expectedContent)
+
+							outputFile := fmt.Sprintf("%d.%d.txt", i, k)
+							t.Setenv("OUTPUT_FILE", outputFile)
+
+							path := filepath.Join(dir, outputFile)
+							require.NoError(t, os.RemoveAll(path))
+
+							require.NoError(t, e.executor.Run(context.Background(), &ast.Call{Task: task}))
+
+							actualContent, err := os.ReadFile(path)
+							require.NoError(t, err)
+							assert.Equal(t, expectedContent, strings.TrimSpace(string(actualContent)))
+						})
+					}
+				})
+			}
+
+			t.Log("\noutput:\n", buff.buf.String())
+		})
+	}
+}
+
 func TestIncludeCycle(t *testing.T) {
 	const dir = "testdata/includes_cycle"
 

taskfile/reader.go

@@ -184,93 +184,10 @@ func (r *Reader) include(node Node) error {
 }
 
 func (r *Reader) readNode(node Node) (*ast.Taskfile, error) {
-	var b []byte
-	var err error
-	var cache *Cache
-
-	if node.Remote() {
-		cache, err = NewCache(r.tempDir)
+	b, err := r.loadNodeContent(node)
 	if err != nil {
 		return nil, err
 	}
-	}
-
-	// If the file is remote and we're in offline mode, check if we have a cached copy
-	if node.Remote() && r.offline {
-		if b, err = cache.read(node); errors.Is(err, os.ErrNotExist) {
-			return nil, &errors.TaskfileCacheNotFoundError{URI: node.Location()}
-		} else if err != nil {
-			return nil, err
-		}
-		r.logger.VerboseOutf(logger.Magenta, "task: [%s] Fetched cached copy\n", node.Location())
-	} else {
-
-		downloaded := false
-		ctx, cf := context.WithTimeout(context.Background(), r.timeout)
-		defer cf()
-
-		// Read the file
-		b, err = node.Read(ctx)
-		var taskfileNetworkTimeoutError *errors.TaskfileNetworkTimeoutError
-		// If we timed out then we likely have a network issue
-		if node.Remote() && errors.As(err, &taskfileNetworkTimeoutError) {
-			// If a download was requested, then we can't use a cached copy
-			if r.download {
-				return nil, &errors.TaskfileNetworkTimeoutError{URI: node.Location(), Timeout: r.timeout}
-			}
-			// Search for any cached copies
-			if b, err = cache.read(node); errors.Is(err, os.ErrNotExist) {
-				return nil, &errors.TaskfileNetworkTimeoutError{URI: node.Location(), Timeout: r.timeout, CheckedCache: true}
-			} else if err != nil {
-				return nil, err
-			}
-			r.logger.VerboseOutf(logger.Magenta, "task: [%s] Network timeout. Fetched cached copy\n", node.Location())
-		} else if err != nil {
-			return nil, err
-		} else {
-			downloaded = true
-		}
-
-		// If the node was remote, we need to check the checksum
-		if node.Remote() && downloaded {
-			r.logger.VerboseOutf(logger.Magenta, "task: [%s] Fetched remote copy\n", node.Location())
-
-			// Get the checksums
-			checksum := checksum(b)
-			cachedChecksum := cache.readChecksum(node)
-
-			var prompt string
-			if cachedChecksum == "" {
-				// If the checksum doesn't exist, prompt the user to continue
-				prompt = fmt.Sprintf(taskfileUntrustedPrompt, node.Location())
-			} else if checksum != cachedChecksum {
-				// If there is a cached hash, but it doesn't match the expected hash, prompt the user to continue
-				prompt = fmt.Sprintf(taskfileChangedPrompt, node.Location())
-			}
-
-			if prompt != "" {
-				if err := func() error {
-					r.promptMutex.Lock()
-					defer r.promptMutex.Unlock()
-					return r.logger.Prompt(logger.Yellow, prompt, "n", "y", "yes")
-				}(); err != nil {
-					return nil, &errors.TaskfileNotTrustedError{URI: node.Location()}
-				}
-			}
-			// If the hash has changed (or is new)
-			if checksum != cachedChecksum {
-				// Store the checksum
-				if err := cache.writeChecksum(node, checksum); err != nil {
-					return nil, err
-				}
-				// Cache the file
-				r.logger.VerboseOutf(logger.Magenta, "task: [%s] Caching downloaded file\n", node.Location())
-				if err = cache.write(node, b); err != nil {
-					return nil, err
-				}
-			}
-		}
-	}
 
 	var tf ast.Taskfile
 	if err := yaml.Unmarshal(b, &tf); err != nil {
@@ -302,3 +219,93 @@ func (r *Reader) readNode(node Node) (*ast.Taskfile, error) {
 
 	return &tf, nil
 }
+
+func (r *Reader) loadNodeContent(node Node) ([]byte, error) {
+	if !node.Remote() {
+		ctx, cf := context.WithTimeout(context.Background(), r.timeout)
+		defer cf()
+		return node.Read(ctx)
+	}
+
+	cache, err := NewCache(r.tempDir)
+	if err != nil {
+		return nil, err
+	}
+
+	if r.offline {
+		// In offline mode try to use cached copy
+		cached, err := cache.read(node)
+		if errors.Is(err, os.ErrNotExist) {
+			return nil, &errors.TaskfileCacheNotFoundError{URI: node.Location()}
+		} else if err != nil {
+			return nil, err
+		}
+		r.logger.VerboseOutf(logger.Magenta, "task: [%s] Fetched cached copy\n", node.Location())
+
+		return cached, nil
+	}
+
+	ctx, cf := context.WithTimeout(context.Background(), r.timeout)
+	defer cf()
+
+	b, err := node.Read(ctx)
+	if errors.Is(err, &errors.TaskfileNetworkTimeoutError{}) {
+		// If we timed out then we likely have a network issue
+
+		// If a download was requested, then we can't use a cached copy
+		if r.download {
+			return nil, &errors.TaskfileNetworkTimeoutError{URI: node.Location(), Timeout: r.timeout}
+		}
+
+		// Search for any cached copies
+		cached, err := cache.read(node)
+		if errors.Is(err, os.ErrNotExist) {
+			return nil, &errors.TaskfileNetworkTimeoutError{URI: node.Location(), Timeout: r.timeout, CheckedCache: true}
+		} else if err != nil {
+			return nil, err
+		}
+		r.logger.VerboseOutf(logger.Magenta, "task: [%s] Network timeout. Fetched cached copy\n", node.Location())
+
+		return cached, nil
+
+	} else if err != nil {
+		return nil, err
+	}
+	r.logger.VerboseOutf(logger.Magenta, "task: [%s] Fetched remote copy\n", node.Location())
+
+	// Get the checksums
+	checksum := checksum(b)
+	cachedChecksum := cache.readChecksum(node)
+
+	var prompt string
+	if cachedChecksum == "" {
+		// If the checksum doesn't exist, prompt the user to continue
+		prompt = fmt.Sprintf(taskfileUntrustedPrompt, node.Location())
+	} else if checksum != cachedChecksum {
+		// If there is a cached hash, but it doesn't match the expected hash, prompt the user to continue
+		prompt = fmt.Sprintf(taskfileChangedPrompt, node.Location())
+	}
+
+	if prompt != "" {
+		if err := func() error {
+			r.promptMutex.Lock()
+			defer r.promptMutex.Unlock()
+			return r.logger.Prompt(logger.Yellow, prompt, "n", "y", "yes")
+		}(); err != nil {
+			return nil, &errors.TaskfileNotTrustedError{URI: node.Location()}
+		}
+
+		// Store the checksum
+		if err := cache.writeChecksum(node, checksum); err != nil {
+			return nil, err
+		}
+
+		// Cache the file
+		r.logger.VerboseOutf(logger.Magenta, "task: [%s] Caching downloaded file\n", node.Location())
+		if err = cache.write(node, b); err != nil {
+			return nil, err
+		}
+	}
+
+	return b, nil
+}

testdata/includes_remote/.gitignore

@@ -0,0 +1 @@
+*.txt

testdata/includes_remote/Taskfile.yml

@@ -0,0 +1,4 @@
+version: '3'
+
+includes:
+  first: "{{.FIRST_REMOTE_URL}}"

testdata/includes_remote/first/Taskfile.yml

@@ -0,0 +1,11 @@
+version: '3'
+
+includes:
+  second: "{{.SECOND_REMOTE_URL}}"
+
+tasks:
+  write-file:
+    requires:
+      vars: [CONTENT, OUTPUT_FILE]
+    cmd: |
+      echo "{{.CONTENT}}" > "{{.OUTPUT_FILE}}"

testdata/includes_remote/first/second/Taskfile.yml

@@ -0,0 +1,8 @@
+version: '3'
+
+tasks:
+  write-file:
+    requires:
+      vars: [CONTENT, OUTPUT_FILE]
+    cmd: |
+      echo "{{.CONTENT}}" > "{{.OUTPUT_FILE}}"