Review comments

dannykopping · dannykopping · commit fdaacda2bd39 · 2024-03-13T15:31:48.000+02:00
Signed-off-by: Danny Kopping &lt;danny@coder.com&gt;
diff --git a/cli/templatepull.go b/cli/templatepull.go
@@ -133,12 +133,10 @@ func (r *RootCmd) templatePull() *clibase.Cmd {
 
 			clean, err := filepath.Abs(filepath.Clean(dest))
 			if err != nil {
-				cliui.Error(inv.Stderr, fmt.Sprintf("cleaning destination path %s failed: %q", dest, err))
-				return err
+				return xerrors.Errorf("cleaning destination path %s failed: %w", dest, err)
 			}
 
 			if dest != clean {
-				cliui.Warn(inv.Stderr, fmt.Sprintf("cleaning destination path from %s to %s", dest, clean))
 				dest = clean
 			}
 
diff --git a/cli/templatepull_test.go b/cli/templatepull_test.go
@@ -230,8 +230,9 @@ func TestTemplatePull_LatestStdout(t *testing.T) {
 
 // ToDir tests that 'templates pull' pulls down the active template
 // and writes it to the correct directory.
-// nolint: paralleltest
 func TestTemplatePull_ToDir(t *testing.T) {
+	t.Parallel()
+
 	// Prevents the tests from running in parallel.
 	tmp := t.TempDir()
 	expectedDest := filepath.Join(tmp, "expected")
@@ -245,9 +246,13 @@ func TestTemplatePull_ToDir(t *testing.T) {
 			givenPath: filepath.Join(tmp, "actual"),
 		},
 		{
-			name:      "relative path is cleaned up",
+			name:      "relative path to specific dir is sanitized",
 			givenPath: "./pulltmp",
 		},
+		{
+			name:      "relative path to current dir is sanitized",
+			givenPath: ".",
+		},
 		{
 			name:      "directory traversal is acceptable",
 			givenPath: "../../../mytmpl",
@@ -261,7 +266,12 @@ func TestTemplatePull_ToDir(t *testing.T) {
 	for _, tc := range tests {
 		tc := tc
 
+		// nolint: paralleltest // These tests all share expectedDest
 		t.Run(tc.name, func(t *testing.T) {
+			// Use a different working directory to not interfere with actual directory when using relative paths.
+			newWD := t.TempDir()
+			require.NoError(t, os.Chdir(newWD))
+
 			t.Cleanup(func() {
 				_ = os.RemoveAll(tc.givenPath)
 				_ = os.RemoveAll(expectedDest)

Original file line number	Diff line number	Diff line change
`@@ -133,12 +133,10 @@ func (r RootCmd) templatePull() clibase.Cmd {`
`133`	`133`
`134`	`134`	`clean, err := filepath.Abs(filepath.Clean(dest))`
`135`	`135`	`if err != nil {`
`136`		`- cliui.Error(inv.Stderr, fmt.Sprintf("cleaning destination path %s failed: %q", dest, err))`
`137`		`- return err`
	`136`	`+ return xerrors.Errorf("cleaning destination path %s failed: %w", dest, err)`
`138`	`137`	`}`
`139`	`138`
`140`	`139`	`if dest != clean {`
`141`		`- cliui.Warn(inv.Stderr, fmt.Sprintf("cleaning destination path from %s to %s", dest, clean))`
`142`	`140`	`dest = clean`
`143`	`141`	`}`
`144`	`142`