Repository: cli/cli
Pull request #8944 (2024-04-08 13:47:46)

Ensure run log cache creates cache dir if it doesn't exist
## Description

In https://github.com/cli/cli/pull/8931 @andyfeller smartly called out that we [still needed to ensure the cache dir existed](https://github.com/cli/cli/pull/8931#discussion_r1553647620). I copied and pasted the previous code blindly without realising that it was stripping the filename from the end using `filepath.Dir()`. This made sense before when we were passing a full filepath into the cache functions, but now we have the dir itself as a field on the struct.



Patch:
diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go
index 4c7fbf2d9..2ffe7f72c 100644
--- a/pkg/cmd/run/view/view.go
+++ b/pkg/cmd/run/view/view.go
@@ -46,7 +46,7 @@ func (c RunLogCache) Exists(key string) (bool, error) {
 }
 
 func (c RunLogCache) Create(key string, content io.Reader) error {
-	if err := os.MkdirAll(filepath.Dir(c.cacheDir), 0755); err != nil {
+	if err := os.MkdirAll(c.cacheDir, 0755); err != nil {
 		return fmt.Errorf("creating cache directory: %v", err)
 	}
 


Test patch:
diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go
index 88c8aa21d..f053ef321 100644
--- a/pkg/cmd/run/view/view_test.go
+++ b/pkg/cmd/run/view/view_test.go
@@ -7,6 +7,8 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -1535,3 +1537,60 @@ sad job	quux the barf	log line 3
 var coolJobRunLogOutput = fmt.Sprintf("%s%s", fobTheBarzLogOutput, barfTheFobLogOutput)
 var sadJobRunLogOutput = fmt.Sprintf("%s%s", barfTheQuuxLogOutput, quuxTheBarfLogOutput)
 var expectedRunLogOutput = fmt.Sprintf("%s%s", coolJobRunLogOutput, sadJobRunLogOutput)
+
+func TestRunLog(t *testing.T) {
+	t.Run("when the cache dir doesn't exist, exists return false", func(t *testing.T) {
+		cacheDir := t.TempDir() + "/non-existent-dir"
+		rlc := RunLogCache{cacheDir: cacheDir}
+
+		exists, err := rlc.Exists("unimportant-key")
+		require.NoError(t, err)
+		require.False(t, exists)
+	})
+
+	t.Run("when no cache entry has been created, exists returns false", func(t *testing.T) {
+		cacheDir := t.TempDir()
+		rlc := RunLogCache{cacheDir: cacheDir}
+
+		exists, err := rlc.Exists("unimportant-key")
+		require.NoError(t, err)
+		require.False(t, exists)
+	})
+
+	t.Run("when a cache entry has been created, exists returns true", func(t *testing.T) {
+		cacheDir := t.TempDir()
+		rlc := RunLogCache{cacheDir: cacheDir}
+
+		contents := strings.NewReader("unimportant-content")
+		require.NoError(t, rlc.Create("key", contents))
+
+		exists, err := rlc.Exists("key")
+		require.NoError(t, err)
+		require.True(t, exists)
+	})
+
+	t.Run("when the cache dir doesn't exist, creating a cache entry creates it", func(t *testing.T) {
+		cacheDir := t.TempDir() + "/non-existent-dir"
+		rlc := RunLogCache{cacheDir: cacheDir}
+
+		contents := strings.NewReader("unimportant-content")
+		require.NoError(t, rlc.Create("key", contents))
+
+		require.DirExists(t, cacheDir)
+	})
+
+	t.Run("when a cache entry has been created, reading it returns its contents", func(t *testing.T) {
+		cacheDir := t.TempDir()
+		rlc := RunLogCache{cacheDir: cacheDir}
+
+		f, err := os.Open("./fixtures/run_log.zip")
+		require.NoError(t, err)
+
+		require.NoError(t, rlc.Create("key", f))
+
+		zipReader, err := rlc.Open("key")
+		require.NoError(t, err)
+		defer zipReader.Close()
+		require.NotEmpty(t, zipReader.File)
+	})
+}