From 89f47f8ebe29dcea11bb45e40332345276204ce2 Mon Sep 17 00:00:00 2001 From: Jo Date: Wed, 5 Apr 2023 12:27:42 +0100 Subject: [PATCH] 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 | 3 +- pkg/menus/menu.go | 1 + pkg/upgrade/sources_test.go | 4 +- preparer.go | 75 +++++++++++++++++++++++-------------- preparer_test.go | 62 ++++++++++++++++++++++++++++++ 5 files changed, 115 insertions(+), 30 deletions(-) create mode 100644 preparer_test.go diff --git a/pkg/menus/clean_menu.go b/pkg/menus/clean_menu.go index 77314bcf..8ef6b284 100644 --- a/pkg/menus/clean_menu.go +++ b/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 diff --git a/pkg/menus/menu.go b/pkg/menus/menu.go index c1e5f941..3212fbed 100644 --- a/pkg/menus/menu.go +++ b/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)"))) } diff --git a/pkg/upgrade/sources_test.go b/pkg/upgrade/sources_test.go index bf340f6c..62dbc4d0 100644 --- a/pkg/upgrade/sources_test.go +++ b/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()) }) } } diff --git a/preparer.go b/preparer.go index 93413157..fddaa4e0 100644 --- a/preparer.go +++ b/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 } diff --git a/preparer_test.go b/preparer_test.go new file mode 100644 index 00000000..ca8b9929 --- /dev/null +++ b/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) + }) + } +}