Repository: supabase/cli
Pull request #332 (2022-08-05 04:39:36)

fix: allow branch deletion without current branch
## What kind of change does this PR introduce?

unit test

## What is the current behavior?

delete requires current branch

## What is the new behavior?

allows deletion without setting current branch

## Additional context

Add any other context or screenshots.


coveralls: ## Pull Request Test Coverage Report for [Build 2801315635](https://coveralls.io/builds/51439051)

* **13** of **46**   **(28.26%)**  changed or added relevant lines in **2** files are covered.
* **1** unchanged line in **1** file lost coverage.
* Overall coverage increased (+**0.3%**) to **22.527%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [cmd/db.go](https://coveralls.io/builds/51439051/source?filename=cmd%2Fdb.go#L47) | 0 | 1 | 0.0%
| [internal/db/branch/delete/delete.go](https://coveralls.io/builds/51439051/source?filename=internal%2Fdb%2Fbranch%2Fdelete%2Fdelete.go#L25) | 13 | 45 | 28.89%
<!-- | **Total:** | **13** | **46** | **28.26%** | -->

|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [internal/db/branch/delete/delete.go](https://coveralls.io/builds/51439051/source?filename=internal%2Fdb%2Fbranch%2Fdelete%2Fdelete.go#L87) | 1 | 37.93% |
<!-- | **Total:** | **1** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/51439051/badge)](https://coveralls.io/builds/51439051) |
| :-- | --: |
| Change from base [Build 2801218766](https://coveralls.io/builds/51438755): |  0.3% |
| Covered Lines: | 1159 |
| Relevant Lines: | 5145 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/cmd/db.go b/cmd/db.go
index a2e843b5..df0aa2a1 100644
--- a/cmd/db.go
+++ b/cmd/db.go
@@ -44,7 +44,7 @@ var (
 		Short: "Delete a branch.",
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return delete.Run(args[0])
+			return delete.Run(args[0], afero.NewOsFs())
 		},
 	}
 
diff --git a/internal/db/branch/delete/delete.go b/internal/db/branch/delete/delete.go
index d1a04ce6..78456ddc 100644
--- a/internal/db/branch/delete/delete.go
+++ b/internal/db/branch/delete/delete.go
@@ -6,20 +6,38 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"os"
+	"path/filepath"
 
+	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/pkg/stdcopy"
+	"github.com/spf13/afero"
 	"github.com/supabase/cli/internal/utils"
 )
 
-func Run(branch string) error {
-	if err := utils.AssertSupabaseStartIsRunning(); err != nil {
+func Run(branch string, fsys afero.Fs) error {
+	if err := utils.LoadConfigFS(fsys); err != nil {
+		return err
+	}
+	if err := utils.AssertSupabaseDbIsRunning(); err != nil {
+		return err
+	}
+
+	// TODO: update branch history atomically with database
+	if err := deleteBranchDir(branch, fsys); err != nil {
 		return err
 	}
 
-	if currBranch, err := utils.GetCurrentBranch(); err != nil {
+	ctx := context.Background()
+	if err := deleteBranchPG(ctx, branch); err != nil {
 		return err
-	} else if branch == currBranch {
+	}
+
+	fmt.Println("Deleted branch " + utils.Aqua(branch) + ".")
+	return nil
+}
+
+func deleteBranchDir(branch string, fsys afero.Fs) error {
+	if currBranch, _ := utils.GetCurrentBranchFS(fsys); branch == currBranch {
 		return errors.New("Cannot delete current branch.")
 	}
 
@@ -27,30 +45,45 @@ func Run(branch string) error {
 		return errors.New("Cannot delete branch " + utils.Aqua(branch) + ": branch name is reserved.")
 	}
 
-	if _, err := os.ReadDir("supabase/.branches/" + branch); err != nil {
+	branchPath := filepath.Join(filepath.Dir(utils.CurrBranchPath), branch)
+	if _, err := afero.ReadDir(fsys, branchPath); err != nil {
 		return errors.New("Branch " + utils.Aqua(branch) + " does not exist.")
 	}
 
-	if err := os.RemoveAll("supabase/.branches/" + branch); err != nil {
+	if err := fsys.RemoveAll(branchPath); err != nil {
 		return fmt.Errorf("Failed deleting branch %s: %w", utils.Aqua(branch), err)
 	}
 
-	{
-		out, err := utils.DockerExec(context.Background(), utils.DbId, []string{
-			"dropdb", "--username", "postgres", "--host", "127.0.0.1", branch,
-		})
-		if err != nil {
-			return err
-		}
-		var errBuf bytes.Buffer
-		if _, err := stdcopy.StdCopy(io.Discard, &errBuf, out); err != nil {
-			return err
-		}
-		if errBuf.Len() > 0 {
-			return errors.New("Error dropping database: " + errBuf.String())
-		}
-	}
+	return nil
+}
 
-	fmt.Println("Deleted branch " + utils.Aqua(branch) + ".")
+func deleteBranchPG(ctx context.Context, branch string) error {
+	exec, err := utils.Docker.ContainerExecCreate(ctx, utils.DbId, types.ExecConfig{
+		Cmd:          []string{"dropdb", "--username", "postgres", "--host", "127.0.0.1", branch},
+		AttachStderr: true,
+		AttachStdout: true,
+	})
+	if err != nil {
+		return err
+	}
+	// Read exec output
+	resp, err := utils.Docker.ContainerExecAttach(ctx, exec.ID, types.ExecStartCheck{})
+	if err != nil {
+		return err
+	}
+	defer resp.Close()
+	// Capture error details
+	var errBuf bytes.Buffer
+	if _, err := stdcopy.StdCopy(io.Discard, &errBuf, resp.Reader); err != nil {
+		return err
+	}
+	// Get the exit code
+	iresp, err := utils.Docker.ContainerExecInspect(ctx, exec.ID)
+	if err != nil {
+		return err
+	}
+	if iresp.ExitCode > 0 {
+		return errors.New("Error deleting branch: " + errBuf.String())
+	}
 	return nil
 }


Test patch:
diff --git a/internal/db/branch/delete/delete_test.go b/internal/db/branch/delete/delete_test.go
new file mode 100644
index 00000000..cef27a93
--- /dev/null
+++ b/internal/db/branch/delete/delete_test.go
@@ -0,0 +1,80 @@
+package delete
+
+import (
+	"net/http"
+	"path/filepath"
+	"testing"
+
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/supabase/cli/internal/utils"
+	"gopkg.in/h2non/gock.v1"
+)
+
+func TestBranchDir(t *testing.T) {
+	t.Run("removes a branch directory", func(t *testing.T) {
+		// Setup in-memory fs
+		fsys := afero.NewMemMapFs()
+		path := filepath.Join(filepath.Dir(utils.CurrBranchPath), "test-branch")
+		require.NoError(t, fsys.Mkdir(path, 0755))
+		// Run test
+		assert.NoError(t, deleteBranchDir("test-branch", fsys))
+		// Validate removal
+		exists, err := afero.Exists(fsys, path)
+		assert.NoError(t, err)
+		assert.False(t, exists)
+	})
+
+	t.Run("branch is current", func(t *testing.T) {
+		// Setup in-memory fs
+		fsys := afero.NewMemMapFs()
+		require.NoError(t, afero.WriteFile(fsys, utils.CurrBranchPath, []byte("main"), 0644))
+		// Run test
+		assert.Error(t, deleteBranchDir("main", fsys))
+	})
+
+	t.Run("branch is reserved", func(t *testing.T) {
+		assert.Error(t, deleteBranchDir("main", afero.NewMemMapFs()))
+	})
+
+	t.Run("branch does not exist", func(t *testing.T) {
+		assert.Error(t, deleteBranchDir("test-branch", afero.NewMemMapFs()))
+	})
+
+	t.Run("branch permission denied", func(t *testing.T) {
+		// Setup read-only fs
+		fsys := afero.NewMemMapFs()
+		path := filepath.Join(filepath.Dir(utils.CurrBranchPath), "test-branch")
+		require.NoError(t, fsys.Mkdir(path, 0755))
+		// Run test
+		assert.Error(t, deleteBranchDir("test-branch", afero.NewReadOnlyFs(fsys)))
+	})
+}
+
+func TestDeleteCommand(t *testing.T) {
+	const (
+		version = "v1.41"
+		branch  = "test-branch"
+	)
+
+	t.Run("throws error on missing config", func(t *testing.T) {
+		assert.Error(t, Run(branch, afero.NewMemMapFs()))
+	})
+
+	t.Run("throws error on stopped db", func(t *testing.T) {
+		// Setup in-memory fs
+		fsys := &afero.MemMapFs{}
+		require.NoError(t, utils.WriteConfig(fsys, false))
+		// Setup mock docker
+		defer gock.Off()
+		gock.InterceptClient(utils.Docker.HTTPClient())
+		gock.New("http:///var/run/docker.sock").
+			Head("/_ping").
+			Reply(http.StatusOK).
+			SetHeader("API-Version", version).
+			SetHeader("OSType", "linux")
+		// Run test
+		assert.Error(t, Run(branch, fsys))
+	})
+}