fix(yay): correct operation order for preparer (#2071)

* fix(yay): reset to origin/HEAD for clean_menu

* fix(yay): correct operation order for preparer

* test(yay): fix flaky test

pkg/menus/clean_menu.go

@@ -35,6 +35,7 @@ func CleanFn(ctx context.Context, config *settings.Configuration, w io.Writer, p
 
 	skipFunc := func(pkg string) bool {
 		dir := pkgbuildDirsByBase[pkg]
+		// TOFIX: new install engine dir will always exist, check if unclean instead
 		if _, err := os.Stat(dir); os.IsNotExist(err) {
 			return true
 		}
@@ -59,7 +60,7 @@ func CleanFn(ctx context.Context, config *settings.Configuration, w io.Writer, p
 		dir := pkgbuildDirsByBase[base]
 		text.OperationInfoln(gotext.Get("Deleting (%d/%d): %s", i+1, len(toClean), text.Cyan(dir)))
 
-		if err := config.Runtime.CmdBuilder.Show(config.Runtime.CmdBuilder.BuildGitCmd(ctx, dir, "reset", "--hard", "HEAD")); err != nil {
+		if err := config.Runtime.CmdBuilder.Show(config.Runtime.CmdBuilder.BuildGitCmd(ctx, dir, "reset", "--hard", "origin/HEAD")); err != nil {
 			text.Warnln(gotext.Get("Unable to clean:"), dir)
 
 			return err

pkg/menus/menu.go

@@ -26,6 +26,7 @@ func pkgbuildNumberMenu(w io.Writer, pkgbuildDirs map[string]string, bases []str
 			toPrint += text.Bold(text.Green(gotext.Get(" (Installed)")))
 		}
 
+		// TODO: remove or refactor to check if git dir is unclean
 		if _, err := os.Stat(dir); !os.IsNotExist(err) {
 			toPrint += text.Bold(text.Green(gotext.Get(" (Build Files Exist)")))
 		}

pkg/upgrade/sources_test.go

@@ -124,7 +124,9 @@ func Test_upAUR(t *testing.T) {
 
 			got := UpAUR(text.NewLogger(io.Discard, strings.NewReader(""), false, "test"),
 				tt.args.remote, tt.args.aurdata, tt.args.timeUpdate, tt.args.enableDowngrade)
-			assert.EqualValues(t, tt.want, got)
+			assert.ElementsMatch(t, tt.want.Repos, got.Repos)
+			assert.ElementsMatch(t, tt.want.Up, got.Up)
+			assert.Equal(t, tt.want.Len(), got.Len())
 		})
 	}
 }

preparer.go

@@ -22,37 +22,60 @@ import (
 	"github.com/leonelquinteros/gotext"
 )
 
-type PreparerHookFunc func(ctx context.Context, config *settings.Configuration, w io.Writer, pkgbuildDirsByBase map[string]string) error
+type HookType string
+
+const (
+	// PreDownloadSourcesHook is called before sourcing a package
+	PreDownloadSourcesHook HookType = "pre-download-sources"
+)
+
+type HookFn func(ctx context.Context, config *settings.Configuration, w io.Writer, pkgbuildDirsByBase map[string]string) error
+
+type Hook struct {
+	Name   string
+	Hookfn HookFn
+	Type   HookType
+}
 
 type Preparer struct {
-	dbExecutor        db.Executor
-	cmdBuilder        exe.ICmdBuilder
-	cfg               *settings.Configuration
-	postDownloadHooks []PreparerHookFunc
-	postMergeHooks    []PreparerHookFunc
+	dbExecutor db.Executor
+	cmdBuilder exe.ICmdBuilder
+	cfg        *settings.Configuration
+	hooks      []Hook
 
 	makeDeps []string
 }
 
 func NewPreparer(dbExecutor db.Executor, cmdBuilder exe.ICmdBuilder, cfg *settings.Configuration) *Preparer {
 	preper := &Preparer{
-		dbExecutor:        dbExecutor,
-		cmdBuilder:        cmdBuilder,
-		cfg:               cfg,
-		postDownloadHooks: []PreparerHookFunc{},
-		postMergeHooks:    []PreparerHookFunc{},
+		dbExecutor: dbExecutor,
+		cmdBuilder: cmdBuilder,
+		cfg:        cfg,
+		hooks:      []Hook{},
 	}
 
 	if cfg.CleanMenu {
-		preper.postDownloadHooks = append(preper.postDownloadHooks, menus.CleanFn)
+		preper.hooks = append(preper.hooks, Hook{
+			Name:   "clean",
+			Hookfn: menus.CleanFn,
+			Type:   PreDownloadSourcesHook,
+		})
 	}
 
 	if cfg.DiffMenu {
-		preper.postMergeHooks = append(preper.postMergeHooks, menus.DiffFn)
+		preper.hooks = append(preper.hooks, Hook{
+			Name:   "diff",
+			Hookfn: menus.DiffFn,
+			Type:   PreDownloadSourcesHook,
+		})
 	}
 
 	if cfg.EditMenu {
-		preper.postMergeHooks = append(preper.postMergeHooks, menus.EditFn)
+		preper.hooks = append(preper.hooks, Hook{
+			Name:   "edit",
+			Hookfn: menus.EditFn,
+			Type:   PreDownloadSourcesHook,
+		})
 	}
 
 	return preper
@@ -171,27 +194,23 @@ func (preper *Preparer) PrepareWorkspace(ctx context.Context, targets []map[stri
 		return nil, errA
 	}
 
-	if errP := downloadPKGBUILDSourceFanout(ctx, preper.cmdBuilder,
-		pkgBuildDirsByBase, false, preper.cfg.MaxConcurrentDownloads); errP != nil {
-		text.Errorln(errP)
-	}
-
-	for _, hookFn := range preper.postDownloadHooks {
-		if err := hookFn(ctx, preper.cfg, os.Stdout, pkgBuildDirsByBase); err != nil {
-			return nil, err
-		}
-	}
-
 	if err := mergePkgbuilds(ctx, preper.cmdBuilder, pkgBuildDirsByBase); err != nil {
 		return nil, err
 	}
 
-	for _, hookFn := range preper.postMergeHooks {
-		if err := hookFn(ctx, preper.cfg, os.Stdout, pkgBuildDirsByBase); err != nil {
-			return nil, err
+	for _, hookFn := range preper.hooks {
+		if hookFn.Type == PreDownloadSourcesHook {
+			if err := hookFn.Hookfn(ctx, preper.cfg, os.Stdout, pkgBuildDirsByBase); err != nil {
+				return nil, err
+			}
 		}
 	}
 
+	if errP := downloadPKGBUILDSourceFanout(ctx, preper.cmdBuilder,
+		pkgBuildDirsByBase, false, preper.cfg.MaxConcurrentDownloads); errP != nil {
+		text.Errorln(errP)
+	}
+
 	return pkgBuildDirsByBase, nil
 }
 

preparer_test.go

@@ -0,0 +1,62 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/Jguer/yay/v12/pkg/settings"
+)
+
+// Test order of pre-download-sources hooks
+func TestPreDownloadSourcesHooks(t *testing.T) {
+	testCases := []struct {
+		name     string
+		cfg      *settings.Configuration
+		wantHook []string
+	}{
+		{
+			name: "clean, diff, edit",
+			cfg: &settings.Configuration{
+				CleanMenu: true,
+				DiffMenu:  true,
+				EditMenu:  true,
+			},
+			wantHook: []string{"clean", "diff", "edit"},
+		},
+		{
+			name: "clean, edit",
+			cfg: &settings.Configuration{
+				CleanMenu: true,
+				DiffMenu:  false,
+				EditMenu:  true,
+			},
+			wantHook: []string{"clean", "edit"},
+		},
+		{
+			name: "clean, diff",
+			cfg: &settings.Configuration{
+				CleanMenu: true,
+				DiffMenu:  true,
+				EditMenu:  false,
+			},
+			wantHook: []string{"clean", "diff"},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			preper := NewPreparer(nil, nil, tc.cfg)
+
+			assert.Len(t, preper.hooks, len(tc.wantHook))
+
+			got := make([]string, 0, len(preper.hooks))
+
+			for _, hook := range preper.hooks {
+				got = append(got, hook.Name)
+			}
+
+			assert.Equal(t, tc.wantHook, got)
+		})
+	}
+}