Code review changes

cmd/krew/cmd/root.go

@@ -150,8 +150,14 @@ func preRun(cmd *cobra.Command, _ []string) error {
 	}
 
 	if _, ok := os.LookupEnv(constants.EnableMultiIndexSwitch); ok {
-		if err := indexmigration.Migrate(paths); err != nil {
-			return err
+		isMigrated, err := indexmigration.Done(paths)
+		if err != nil {
+			return errors.Wrap(err, "failed to check if index migration is complete")
+		}
+		if !isMigrated {
+			if err := indexmigration.Migrate(paths); err != nil {
+				return errors.Wrap(err, "index migration failed")
+			}
 		}
 	}
 

integration_test/migration_test.go

@@ -66,6 +66,7 @@ func isIndexMigrated(it *ITest) bool {
 	return err == nil
 }
 
+// TODO remove when testing indexmigration is no longer necessary
 func prepareOldIndexLayout(it *ITest) {
 	indexPath := it.TempDir().Path("index/default")
 	tmpPath := it.TempDir().Path("tmp_index")

internal/indexmigration/migration.go

@@ -24,27 +24,20 @@ import (
 	"sigs.k8s.io/krew/internal/environment"
 )
 
-// done checks if the krew installation requires a migration to support multiple indexes.
+// Done checks if the krew installation requires a migration to support multiple indexes.
 // A migration is necessary when the index directory contains a ".git" directory.
-func done(paths environment.Paths) (bool, error) {
+func Done(paths environment.Paths) (bool, error) {
+	klog.V(2).Info("Checking if index migration is needed.")
 	_, err := os.Stat(filepath.Join(paths.IndexBase(), ".git"))
 	if err != nil && os.IsNotExist(err) {
+		klog.V(2).Infoln("Index already migrated.")
 		return true, nil
 	}
 	return false, err
 }
 
-// Migrate removes the index directory and then clones krew-index to the new default index path.
+// Migrate moves the index directory to the new default index path.
 func Migrate(paths environment.Paths) error {
-	isMigrated, err := done(paths)
-	if err != nil {
-		return errors.Wrap(err, "failed to check if index migration is complete")
-	}
-	if isMigrated {
-		klog.V(2).Infoln("Already migrated.")
-		return nil
-	}
-
 	klog.Info("Migrating krew index layout.")
 	indexPath := paths.IndexBase()
 	tmpPath := filepath.Join(paths.BasePath(), "tmp_index_migration")

internal/indexmigration/migration_test.go

@@ -51,7 +51,7 @@ func TestIsMigrated(t *testing.T) {
 			}
 
 			newPaths := environment.NewPaths(tmpDir.Root())
-			actual, err := done(newPaths)
+			actual, err := Done(newPaths)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -63,30 +63,18 @@ func TestIsMigrated(t *testing.T) {
 }
 
 func TestMigrate(t *testing.T) {
-	var tests = []struct {
-		name   string
-		gitDir string
-	}{
-		{name: "migration is necessary", gitDir: "index/.git"},
-		{name: "no migration is necessary", gitDir: "index/default/.git"},
-	}
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			tmpDir, cleanup := testutil.NewTempDir(t)
-			defer cleanup()
+	tmpDir, cleanup := testutil.NewTempDir(t)
+	defer cleanup()
 
-			tmpDir.Write(tt.gitDir, nil)
+	tmpDir.Write("index/.git", nil)
 
-			newPaths := environment.NewPaths(tmpDir.Root())
-			err := Migrate(newPaths)
-			if err != nil {
-				t.Fatal(err)
-			}
-			migrationDone, err := done(newPaths)
-			if err != nil || !migrationDone {
-				t.Errorf("expected migration to be done: %s", err)
-			}
-		})
+	newPaths := environment.NewPaths(tmpDir.Root())
+	err := Migrate(newPaths)
+	if err != nil {
+		t.Fatal(err)
+	}
+	done, err := Done(newPaths)
+	if err != nil || !done {
+		t.Errorf("expected migration to be done: %s", err)
 	}
 }