refactor: reduce the dependency on the default config in tests

cmd/cleanup.go

@@ -10,6 +10,7 @@ import (
 	"github.com/simontheleg/konf-go/config"
 	"github.com/simontheleg/konf-go/konf"
 	log "github.com/simontheleg/konf-go/log"
+	"github.com/simontheleg/konf-go/store"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 )
@@ -23,12 +24,14 @@ An active config is considered unused when no process points to it anymore`,
 	RunE: func(cmd *cobra.Command, args []string) error {
 
 		fs := afero.NewOsFs()
-		err := cleanLeftOvers(fs)
+		sm := &store.Storemanager{Activedir: config.ActiveDir(), Storedir: config.StoreDir(), Fs: fs}
+
+		err := cleanLeftOvers(sm)
 		if err != nil {
 			return err
 		}
 
-		err = selfClean(fs)
+		err = selfClean(sm)
 		if err != nil {
 			return err
 		}
@@ -41,11 +44,12 @@ An active config is considered unused when no process points to it anymore`,
 // it is required as the idempotent clean would delete all files that
 // do not belong to any process anymore, but of course the current process
 // is still running at this time
-func selfClean(f afero.Fs) error {
+func selfClean(sm *store.Storemanager) error {
 	pid := os.Getppid()
 
-	fpath := konf.IDFromProcessID(pid).ActivePath()
-	err := f.Remove(fpath)
+	konfID := konf.IDFromProcessID(pid)
+	fpath := sm.ActivePathFromID(konfID)
+	err := sm.Fs.Remove(fpath)
 
 	if errors.Is(err, fs.ErrNotExist) {
 		log.Info("current konf '%s' was already deleted, nothing to self-cleanup\n", fpath)
@@ -64,8 +68,8 @@ func selfClean(f afero.Fs) error {
 // any leftovers that can occur if a previous session was not cleaned up nicely. This is
 // necessary as we cannot tell a user that a selfClean has failed if they close the shell
 // session before
-func cleanLeftOvers(f afero.Fs) error {
-	konfs, err := afero.ReadDir(f, config.ActiveDir())
+func cleanLeftOvers(sm *store.Storemanager) error {
+	konfs, err := afero.ReadDir(sm.Fs, sm.Activedir)
 
 	if err != nil {
 		return err
@@ -86,7 +90,7 @@ func cleanLeftOvers(f afero.Fs) error {
 		}
 
 		if p == nil {
-			err := f.Remove(konfID.ActivePath())
+			err := sm.Fs.Remove(sm.ActivePathFromID(konfID))
 			if err != nil {
 				return err
 			}

cmd/cleanup_test.go

@@ -9,15 +9,16 @@ import (
 	"syscall"
 	"testing"
 
-	"github.com/simontheleg/konf-go/config"
 	"github.com/simontheleg/konf-go/konf"
+	"github.com/simontheleg/konf-go/store"
 	"github.com/simontheleg/konf-go/testhelper"
 	"github.com/simontheleg/konf-go/utils"
 	"github.com/spf13/afero"
 )
 
 func TestSelfClean(t *testing.T) {
-	activeDir := config.ActiveDir()
+	activeDir := "./konf/active"
+	storeDir := "./konf/store"
 
 	ppid := os.Getppid()
 
@@ -28,26 +29,28 @@ func TestSelfClean(t *testing.T) {
 		NotExpFiles []string
 	}{
 		"PID FS": {
-			ppidFS(),
+			ppidFS(activeDir),
 			nil,
-			[]string{activeDir + "/abc", konf.KonfID("1234").ActivePath()},
+			[]string{activeDir + "/abc", activeDir + "/1234"},
 			[]string{activeDir + "/" + fmt.Sprint(ppid) + ".yaml"},
 		},
 		"PID file deleted by external source": {
-			ppidFileMissing(),
+			ppidFileMissing(activeDir),
 			nil,
-			[]string{activeDir + "/abc", konf.KonfID("1234").ActivePath()},
+			[]string{activeDir + "/abc", activeDir + "/1234"},
 			[]string{},
 		},
-		// Unfortunately it was not possible with afero to test what happens if
-		// someone changes the active dir permissions an we cannot delete it anymore
-		// apparently in the memFS afero can just delete these files
+		// Unfortunately it was not possible with afero memFS to test what happens if
+		// someone changes the active dir permissions and we cannot delete it anymore.
+		// Apparently in the memFS afero can just delete these files, regardless of
+		// permissions :D
 	}
 
 	for name, tc := range tt {
 		t.Run(name, func(t *testing.T) {
 
-			err := selfClean(tc.Fs)
+			sm := &store.Storemanager{Fs: tc.Fs, Activedir: activeDir, Storedir: storeDir}
+			err := selfClean(sm)
 
 			if !testhelper.EqualError(err, tc.ExpError) {
 				t.Errorf("Want error '%s', got '%s'", tc.ExpError, err)
@@ -74,26 +77,26 @@ func TestSelfClean(t *testing.T) {
 	}
 }
 
-func ppidFS() afero.Fs {
+func ppidFS(activeDir string) afero.Fs {
 	ppid := os.Getppid()
-	fs := ppidFileMissing()
+	fs := ppidFileMissing(activeDir)
 	sm := testhelper.SampleKonfManager{}
-	afero.WriteFile(fs, konf.IDFromProcessID(ppid).ActivePath(), []byte(sm.SingleClusterSingleContextEU()), utils.KonfPerm)
+	afero.WriteFile(fs, activeDir+"/"+fmt.Sprint(ppid), []byte(sm.SingleClusterSingleContextEU()), utils.KonfPerm)
 	return fs
 }
 
-func ppidFileMissing() afero.Fs {
+func ppidFileMissing(activeDir string) afero.Fs {
 	fs := afero.NewMemMapFs()
 	sm := testhelper.SampleKonfManager{}
-	afero.WriteFile(fs, config.ActiveDir()+"/abc", []byte("I am not even a kubeconfig, what am I doing here?"), utils.KonfPerm)
-	afero.WriteFile(fs, konf.KonfID("1234").ActivePath(), []byte(sm.SingleClusterSingleContextEU()), utils.KonfPerm)
+	afero.WriteFile(fs, activeDir+"/abc", []byte("I am not even a kubeconfig, what am I doing here?"), utils.KonfPerm)
+	afero.WriteFile(fs, activeDir+"/1234", []byte(sm.SingleClusterSingleContextEU()), utils.KonfPerm)
 	return fs
 }
 
 func TestCleanLeftOvers(t *testing.T) {
 
 	tt := map[string]struct {
-		Setup  func(t *testing.T) (afero.Fs, []*exec.Cmd, []*exec.Cmd)
+		Setup  func(t *testing.T, sm *store.Storemanager) ([]*exec.Cmd, []*exec.Cmd)
 		ExpErr error
 	}{
 		"all procs still running": {
@@ -116,21 +119,23 @@ func TestCleanLeftOvers(t *testing.T) {
 
 	for name, tc := range tt {
 		t.Run(name, func(t *testing.T) {
-			f, cmdsRunning, cmdsStopped := tc.Setup(t)
+			sm := &store.Storemanager{Fs: afero.NewMemMapFs(), Activedir: "./konf/active", Storedir: "./konf/store"}
+			cmdsRunning, cmdsStopped := tc.Setup(t, sm)
 
 			t.Cleanup(func() {
 				cleanUpRunningCmds(t, cmdsRunning)
 			})
 
-			err := cleanLeftOvers(f)
+			err := cleanLeftOvers(sm)
 
 			if !errors.Is(err, tc.ExpErr) {
 				t.Errorf("Want error '%s', got '%s'", tc.ExpErr, err)
 			}
 
 			for _, cmd := range cmdsRunning {
-				fpath := konf.IDFromProcessID(cmd.Process.Pid).ActivePath()
-				_, err := f.Stat(fpath)
+				id := konf.IDFromProcessID(cmd.Process.Pid)
+				fpath := sm.ActivePathFromID(id)
+				_, err := sm.Fs.Stat(fpath)
 
 				if err != nil {
 					if errors.Is(err, fs.ErrNotExist) {
@@ -142,8 +147,9 @@ func TestCleanLeftOvers(t *testing.T) {
 			}
 
 			for _, cmd := range cmdsStopped {
-				fpath := konf.IDFromProcessID(cmd.Process.Pid).ActivePath()
-				_, err := f.Stat(fpath)
+				id := konf.IDFromProcessID(cmd.Process.Pid)
+				fpath := sm.ActivePathFromID(id)
+				_, err := sm.Fs.Stat(fpath)
 
 				if !errors.Is(err, fs.ErrNotExist) {
 					t.Fatalf("Unexpected error occurred: '%s'", err)
@@ -159,12 +165,11 @@ func TestCleanLeftOvers(t *testing.T) {
 
 }
 
-func mixedFSWithAllProcs(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
+func mixedFSWithAllProcs(t *testing.T, sm *store.Storemanager) (cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
 	// we are simulating other instances of konf here
 	numOfConfs := 3
 
-	fs = afero.NewMemMapFs()
-	sm := testhelper.SampleKonfManager{}
+	skm := testhelper.SampleKonfManager{}
 
 	for i := 1; i <= numOfConfs; i++ {
 		// set sleep to an extremely high number as the argument "infinity" does not exist in all versions of the util
@@ -176,15 +181,15 @@ func mixedFSWithAllProcs(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd, cm
 		}
 		pid := cmd.Process.Pid
 		cmdsRunning = append(cmdsRunning, cmd)
-		afero.WriteFile(fs, konf.IDFromProcessID(pid).ActivePath(), []byte(sm.SingleClusterSingleContextEU()), utils.KonfPerm)
+		afero.WriteFile(sm.Fs, sm.ActivePathFromID(konf.IDFromProcessID(pid)), []byte(skm.SingleClusterSingleContextEU()), utils.KonfPerm)
 	}
 
-	return fs, cmdsRunning, nil
+	return cmdsRunning, nil
 }
 
 // returns a state where there are more fs files than cmds
-func mixedFSIncompleteProcs(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
-	fs, cmdsRunning, cmdsStopped = mixedFSWithAllProcs(t)
+func mixedFSIncompleteProcs(t *testing.T, sm *store.Storemanager) (cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
+	cmdsRunning, cmdsStopped = mixedFSWithAllProcs(t, sm)
 
 	cmdToKill := cmdsRunning[0]
 	origPID := cmdToKill.Process.Pid
@@ -210,22 +215,22 @@ func mixedFSIncompleteProcs(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd,
 	cmdsStopped = append(cmdsStopped, cmdsRunning[0])
 	cmdsRunning = cmdsRunning[1:]
 
-	return fs, cmdsRunning, cmdsStopped
+	return cmdsRunning, cmdsStopped
 }
 
-func mixedFSDirtyDir(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
-	fs, cmdsRunning, cmdsStopped = mixedFSIncompleteProcs(t)
+func mixedFSDirtyDir(t *testing.T, sm *store.Storemanager) (cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
+	cmdsRunning, cmdsStopped = mixedFSIncompleteProcs(t, sm)
 
-	afero.WriteFile(fs, konf.KonfID("/not-a-valid-process-id").ActivePath(), []byte{}, utils.KonfPerm)
+	id := konf.KonfID("/not-a-valid-process-id")
+	afero.WriteFile(sm.Fs, sm.ActivePathFromID(id), []byte{}, utils.KonfPerm)
 
-	return fs, cmdsRunning, cmdsStopped
+	return cmdsRunning, cmdsStopped
 
 }
 
-func emptyFS(t *testing.T) (fs afero.Fs, cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
-	fs = afero.NewMemMapFs()
-
-	return fs, nil, nil
+func emptyFS(t *testing.T, sm *store.Storemanager) (cmdsRunning []*exec.Cmd, cmdsStopped []*exec.Cmd) {
+	// no op
+	return nil, nil
 }
 
 func cleanUpRunningCmds(t *testing.T, cmds []*exec.Cmd) {

cmd/delete.go

@@ -79,10 +79,11 @@ func (c *deleteCmd) delete(cmd *cobra.Command, args []string) error {
 }
 
 func deleteKonfWithID(sm *store.Storemanager, id konf.KonfID) error {
-	if err := sm.Fs.Remove(id.StorePath()); err != nil {
+	path := sm.StorePathFromID(id)
+	if err := sm.Fs.Remove(path); err != nil {
 		return err
 	}
-	log.Info("Successfully deleted konf %q at %q", id, id.StorePath())
+	log.Info("Successfully deleted konf %q at %q", id, path)
 	return nil
 }
 

cmd/import.go

@@ -98,7 +98,8 @@ func (c *importCmd) importf(cmd *cobra.Command, args []string) error {
 		if err != nil {
 			return err
 		}
-		log.Info("Imported konf from %q successfully into %q\n", k.ImportPath, k.Konf.StorePath)
+		storePath := c.sm.StorePathFromID(k.Konf.Id)
+		log.Info("Imported konf from %q successfully into %q\n", k.ImportPath, storePath)
 	}
 
 	if c.move {

cmd/set.go

@@ -25,7 +25,7 @@ type setCmd struct {
 
 func newSetCommand() *setCmd {
 	fs := afero.NewOsFs()
-	sm := &store.Storemanager{Fs: fs, Activedir: config.ActiveDir(), Storedir: config.StoreDir()}
+	sm := &store.Storemanager{Fs: fs, Activedir: config.ActiveDir(), Storedir: config.StoreDir(), LatestKonfPath: config.LatestKonfFilePath()}
 	sc := &setCmd{
 		sm: sm,
 	}
@@ -61,7 +61,7 @@ func (c *setCmd) set(cmd *cobra.Command, args []string) error {
 			return err
 		}
 	} else if args[0] == "-" {
-		id, err = idOfLatestKonf(c.sm.Fs)
+		id, err = idOfLatestKonf(c.sm)
 		if err != nil {
 			return err
 		}
@@ -73,7 +73,7 @@ func (c *setCmd) set(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	err = saveLatestKonf(c.sm.Fs, id)
+	err = saveLatestKonf(c.sm, id)
 	if err != nil {
 		return fmt.Errorf("could not save latest konf. As a result 'konf set -' might not work: %q ", err)
 	}
@@ -135,8 +135,8 @@ func selectSingleKonf(sm *store.Storemanager, pf prompt.RunFunc) (konf.KonfID, e
 	return konf.IDFromClusterAndContext(sel.Cluster, sel.Context), nil
 }
 
-func idOfLatestKonf(f afero.Fs) (konf.KonfID, error) {
-	b, err := afero.ReadFile(f, config.LatestKonfFilePath())
+func idOfLatestKonf(sm *store.Storemanager) (konf.KonfID, error) {
+	b, err := afero.ReadFile(sm.Fs, sm.LatestKonfPath)
 	if err != nil {
 		if errors.Is(err, fs.ErrNotExist) {
 			return "", fmt.Errorf("could not select latest konf, because no konf was yet set")
@@ -164,8 +164,8 @@ func setContext(id konf.KonfID, f afero.Fs) (string, error) {
 
 }
 
-func saveLatestKonf(f afero.Fs, id konf.KonfID) error {
-	return afero.WriteFile(f, config.LatestKonfFilePath(), []byte(id), utils.KonfPerm)
+func saveLatestKonf(sm *store.Storemanager, id konf.KonfID) error {
+	return afero.WriteFile(sm.Fs, sm.LatestKonfPath, []byte(id), utils.KonfPerm)
 }
 
 func createSetPrompt(options []*store.Metadata) *promptui.Select {

cmd/set_test.go

@@ -44,7 +44,8 @@ func TestSelectLastKonf(t *testing.T) {
 
 	for name, tc := range tt {
 		t.Run(name, func(t *testing.T) {
-			id, err := idOfLatestKonf(tc.FSCreator())
+			sm := &store.Storemanager{Fs: tc.FSCreator(), Activedir: activeDir, Storedir: storeDir, LatestKonfPath: latestKonfPath}
+			id, err := idOfLatestKonf(sm)
 
 			if !testhelper.EqualError(tc.ExpError, err) {
 				t.Errorf("Want error %q, got %q", tc.ExpError, err)
@@ -107,7 +108,8 @@ func TestSaveLatestKonf(t *testing.T) {
 	expID := konf.KonfID("context_cluster")
 
 	f := afero.NewMemMapFs()
-	err := saveLatestKonf(f, expID)
+	sm := &store.Storemanager{Fs: f, LatestKonfPath: expFile}
+	err := saveLatestKonf(sm, expID)
 	if err != nil {
 		t.Errorf("Could not save last konf: %q", err)
 	}
@@ -195,7 +197,7 @@ func TestSelectContext(t *testing.T) {
 	activeDir := "./konf/active"
 	fm := testhelper.FilesystemManager{Storedir: storeDir, Activedir: activeDir}
 	f := testhelper.FSWithFiles(fm.StoreDir, fm.SingleClusterSingleContextEU, fm.SingleClusterSingleContextASIA)()
-	sm := &store.Storemanager{Fs: f, Activedir: config.ActiveDir(), Storedir: config.StoreDir()}
+	sm := &store.Storemanager{Fs: f, Activedir: activeDir, Storedir: storeDir}
 
 	// cases
 	// - invalid selection

config/config.go

@@ -12,12 +12,10 @@ type Config struct {
 	Silent  bool
 }
 
-// This is mainly used to provide some sane and lively defaults for unit tests
-// TODO with the new config system in place, it might make sense to rework all the test-cases and remove the "./konf" reference
+// ensure that a default config exists so we don't run into nil pointer exceptions
+// TODO remove this as soon as we got rid of id.StorePath and id.ActivePath
 func init() {
-	curConf = &Config{
-		KonfDir: "./konf",
-	}
+	curConf = &Config{}
 }
 
 // ConfFromHomeDir returns an initialized config based on the users HomeDir

go.mod

@@ -12,6 +12,7 @@ require (
 	k8s.io/api v0.22.3
 	k8s.io/apimachinery v0.22.3
 	k8s.io/client-go v0.22.3
+	k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a
 	sigs.k8s.io/yaml v1.2.0
 )
 
@@ -44,6 +45,5 @@ require (
 	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
 	k8s.io/klog/v2 v2.9.0 // indirect
 	k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e // indirect
-	k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.1.2 // indirect
 )