From 8c716184c15226b617ffb4831cef66f0b93779e7 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Fri, 12 Jan 2024 19:52:19 +1100 Subject: [PATCH 1/2] Set working directory in lazygit test command We need to fetch our list of tests both outside of our test binary and within. We need to get the list from within so that we can run the code that drives the test and runs assertions. To get the list of tests we need to know where the root of the lazygit repo is, given that the tests live in files under that root. So far, we've used this GetLazyRootDirectory() function for that, but it assumes that we're not in a test directory (it just looks for the first .git dir it can find). Because we didn't want to properly fix this before, we've been setting the working directory of the test command to the lazygit root, and using the --path CLI arg to override it when the test itself ran. This was a terrible hack. Now, we're passing the lazygit root directory as an env var to the integration test, so that we can set the working directory to the actual path of the test repo; removing the need to use the --path arg. --- pkg/integration/README.md | 2 + pkg/integration/clients/cli.go | 3 +- pkg/integration/clients/go_test.go | 3 +- pkg/integration/clients/injector/main.go | 3 +- pkg/integration/clients/tui.go | 2 +- pkg/integration/components/runner.go | 36 +++++++++++----- pkg/integration/components/test.go | 42 ++++++++----------- pkg/integration/tests/tests.go | 5 +-- pkg/integration/tests/worktree/bare_repo.go | 2 + .../tests/worktree/dotfile_bare_repo.go | 4 +- 10 files changed, 57 insertions(+), 45 deletions(-) diff --git a/pkg/integration/README.md b/pkg/integration/README.md index 44b9459d1..0c50d8f4e 100644 --- a/pkg/integration/README.md +++ b/pkg/integration/README.md @@ -24,6 +24,8 @@ Each test has two important steps: the setup step and the run step. In the setup step, we prepare a repo with shell commands, for example, creating a merge conflict that will need to be resolved upon opening lazygit. This is all done via the `shell` argument. +When the test runs, lazygit will open in the same working directory that the shell ends up in (so if you want to start lazygit somewhere other than the default location, you can use `shell.Chdir()` at the end of the setup step to set that working directory. + ### Run step The run step has two arguments passed in: diff --git a/pkg/integration/clients/cli.go b/pkg/integration/clients/cli.go index 9ff5453f0..10958c164 100644 --- a/pkg/integration/clients/cli.go +++ b/pkg/integration/clients/cli.go @@ -8,6 +8,7 @@ import ( "strconv" "strings" + "github.com/jesseduffield/lazycore/pkg/utils" "github.com/jesseduffield/lazygit/pkg/integration/components" "github.com/jesseduffield/lazygit/pkg/integration/tests" "github.com/samber/lo" @@ -53,7 +54,7 @@ func runAndPrintFatalError(test *components.IntegrationTest, f func() error) { } func getTestsToRun(testNames []string) []*components.IntegrationTest { - allIntegrationTests := tests.GetTests() + allIntegrationTests := tests.GetTests(utils.GetLazyRootDirectory()) var testsToRun []*components.IntegrationTest if len(testNames) == 0 { diff --git a/pkg/integration/clients/go_test.go b/pkg/integration/clients/go_test.go index d6fecdb2d..6984e0deb 100644 --- a/pkg/integration/clients/go_test.go +++ b/pkg/integration/clients/go_test.go @@ -15,6 +15,7 @@ import ( "testing" "github.com/creack/pty" + "github.com/jesseduffield/lazycore/pkg/utils" "github.com/jesseduffield/lazygit/pkg/integration/components" "github.com/jesseduffield/lazygit/pkg/integration/tests" "github.com/stretchr/testify/assert" @@ -35,7 +36,7 @@ func TestIntegration(t *testing.T) { testNumber := 0 err := components.RunTests(components.RunTestArgs{ - Tests: tests.GetTests(), + Tests: tests.GetTests(utils.GetLazyRootDirectory()), Logf: t.Logf, RunCmd: runCmdHeadless, TestWrapper: func(test *components.IntegrationTest, f func() error) { diff --git a/pkg/integration/clients/injector/main.go b/pkg/integration/clients/injector/main.go index 223ff4ecb..1f4c845e8 100644 --- a/pkg/integration/clients/injector/main.go +++ b/pkg/integration/clients/injector/main.go @@ -58,7 +58,8 @@ func getIntegrationTest() integrationTypes.IntegrationTest { )) } - allTests := tests.GetTests() + lazygitRootDir := os.Getenv(components.LAZYGIT_ROOT_DIR) + allTests := tests.GetTests(lazygitRootDir) for _, candidateTest := range allTests { if candidateTest.Name() == integrationTestName { return candidateTest diff --git a/pkg/integration/clients/tui.go b/pkg/integration/clients/tui.go index 05db98991..ded94f74d 100644 --- a/pkg/integration/clients/tui.go +++ b/pkg/integration/clients/tui.go @@ -233,7 +233,7 @@ type app struct { } func newApp(testDir string) *app { - return &app{testDir: testDir, allTests: tests.GetTests()} + return &app{testDir: testDir, allTests: tests.GetTests(utils.GetLazyRootDirectory())} } func (self *app) getCurrentTest() *components.IntegrationTest { diff --git a/pkg/integration/components/runner.go b/pkg/integration/components/runner.go index fd3ce7bf0..064dd5c5b 100644 --- a/pkg/integration/components/runner.go +++ b/pkg/integration/components/runner.go @@ -14,6 +14,7 @@ import ( ) const ( + LAZYGIT_ROOT_DIR = "LAZYGIT_ROOT_DIR" TEST_NAME_ENV_VAR = "TEST_NAME" SANDBOX_ENV_VAR = "SANDBOX" WAIT_FOR_DEBUGGER_ENV_VAR = "WAIT_FOR_DEBUGGER" @@ -98,11 +99,12 @@ func runTest( return nil } - if err := prepareTestDir(test, paths, projectRootDir); err != nil { + workingDir, err := prepareTestDir(test, paths, projectRootDir) + if err != nil { return err } - cmd, err := getLazygitCommand(test, args, paths, projectRootDir) + cmd, err := getLazygitCommand(test, args, paths, projectRootDir, workingDir) if err != nil { return err } @@ -124,16 +126,18 @@ func prepareTestDir( test *IntegrationTest, paths Paths, rootDir string, -) error { +) (string, error) { findOrCreateDir(paths.Root()) deleteAndRecreateEmptyDir(paths.Actual()) err := os.Mkdir(paths.ActualRepo(), 0o777) if err != nil { - return err + return "", err } - return createFixture(test, paths, rootDir) + workingDir := createFixture(test, paths, rootDir) + + return workingDir, nil } func buildLazygit(testArgs RunTestArgs) error { @@ -154,7 +158,9 @@ func buildLazygit(testArgs RunTestArgs) error { return osCommand.Cmd.New(args).Run() } -func createFixture(test *IntegrationTest, paths Paths, rootDir string) error { +// Sets up the fixture for test and returns the working directory to invoke +// lazygit in. +func createFixture(test *IntegrationTest, paths Paths, rootDir string) string { shell := NewShell(paths.ActualRepo(), func(errorMsg string) { panic(errorMsg) }) shell.Init() @@ -162,7 +168,7 @@ func createFixture(test *IntegrationTest, paths Paths, rootDir string) error { test.SetupRepo(shell) - return nil + return shell.dir } func globalGitConfigPath(rootDir string) string { @@ -179,7 +185,13 @@ func getGitVersion() (*git_commands.GitVersion, error) { return git_commands.ParseGitVersion(versionStr) } -func getLazygitCommand(test *IntegrationTest, args RunTestArgs, paths Paths, rootDir string) (*exec.Cmd, error) { +func getLazygitCommand( + test *IntegrationTest, + args RunTestArgs, + paths Paths, + rootDir string, + workingDir string, +) (*exec.Cmd, error) { osCommand := oscommands.NewDummyOSCommand() err := os.RemoveAll(paths.Config()) @@ -194,9 +206,7 @@ func getLazygitCommand(test *IntegrationTest, args RunTestArgs, paths Paths, roo } cmdArgs := []string{tempLazygitPath(), "-debug", "--use-config-dir=" + paths.Config()} - if !test.useCustomPath { - cmdArgs = append(cmdArgs, "--path="+paths.ActualRepo()) - } + resolvedExtraArgs := lo.Map(test.ExtraCmdArgs(), func(arg string, _ int) string { return utils.ResolvePlaceholderString(arg, map[string]string{ "actualPath": paths.Actual(), @@ -207,6 +217,10 @@ func getLazygitCommand(test *IntegrationTest, args RunTestArgs, paths Paths, roo cmdObj := osCommand.Cmd.New(cmdArgs) + cmdObj.SetWd(workingDir) + + cmdObj.AddEnvVars(fmt.Sprintf("%s=%s", LAZYGIT_ROOT_DIR, rootDir)) + if args.CodeCoverageDir != "" { // We set this explicitly here rather than inherit it from the test runner's // environment because the test runner has its own coverage directory that diff --git a/pkg/integration/components/test.go b/pkg/integration/components/test.go index 7a088c80a..c837d8be8 100644 --- a/pkg/integration/components/test.go +++ b/pkg/integration/components/test.go @@ -35,11 +35,10 @@ type IntegrationTest struct { testDriver *TestDriver, keys config.KeybindingConfig, ) - gitVersion GitVersionRestriction - width int - height int - isDemo bool - useCustomPath bool + gitVersion GitVersionRestriction + width int + height int + isDemo bool } var _ integrationTypes.IntegrationTest = &IntegrationTest{} @@ -55,9 +54,9 @@ type NewIntegrationTestArgs struct { Run func(t *TestDriver, keys config.KeybindingConfig) // additional args passed to lazygit ExtraCmdArgs []string - // for when a test is flakey ExtraEnvVars map[string]string - Skip bool + // for when a test is flakey + Skip bool // to run a test only on certain git versions GitVersion GitVersionRestriction // width and height when running in headless mode, for testing @@ -67,10 +66,6 @@ type NewIntegrationTestArgs struct { Height int // If true, this is not a test but a demo to be added to our docs IsDemo bool - // If true, the test won't invoke lazygit with the --path arg. - // Useful for when we're passing --git-dir and --work-tree (because --path is - // incompatible with those args) - UseCustomPath bool } type GitVersionRestriction struct { @@ -130,19 +125,18 @@ func NewIntegrationTest(args NewIntegrationTestArgs) *IntegrationTest { } return &IntegrationTest{ - name: name, - description: args.Description, - extraCmdArgs: args.ExtraCmdArgs, - extraEnvVars: args.ExtraEnvVars, - skip: args.Skip, - setupRepo: args.SetupRepo, - setupConfig: args.SetupConfig, - run: args.Run, - gitVersion: args.GitVersion, - width: args.Width, - height: args.Height, - isDemo: args.IsDemo, - useCustomPath: args.UseCustomPath, + name: name, + description: args.Description, + extraCmdArgs: args.ExtraCmdArgs, + extraEnvVars: args.ExtraEnvVars, + skip: args.Skip, + setupRepo: args.SetupRepo, + setupConfig: args.SetupConfig, + run: args.Run, + gitVersion: args.GitVersion, + width: args.Width, + height: args.Height, + isDemo: args.IsDemo, } } diff --git a/pkg/integration/tests/tests.go b/pkg/integration/tests/tests.go index 600c98af6..22ca7d8d7 100644 --- a/pkg/integration/tests/tests.go +++ b/pkg/integration/tests/tests.go @@ -9,12 +9,11 @@ import ( "strings" "github.com/jesseduffield/generics/set" - "github.com/jesseduffield/lazycore/pkg/utils" "github.com/jesseduffield/lazygit/pkg/integration/components" "github.com/samber/lo" ) -func GetTests() []*components.IntegrationTest { +func GetTests(lazygitRootDir string) []*components.IntegrationTest { // first we ensure that each test in this directory has actually been added to the above list. testCount := 0 @@ -27,7 +26,7 @@ func GetTests() []*components.IntegrationTest { missingTestNames := []string{} - if err := filepath.Walk(filepath.Join(utils.GetLazyRootDirectory(), "pkg/integration/tests"), func(path string, info os.FileInfo, err error) error { + if err := filepath.Walk(filepath.Join(lazygitRootDir, "pkg/integration/tests"), func(path string, info os.FileInfo, err error) error { if !info.IsDir() && strings.HasSuffix(path, ".go") { // ignoring non-test files if filepath.Base(path) == "tests.go" || filepath.Base(path) == "test_list.go" || filepath.Base(path) == "test_list_generator.go" { diff --git a/pkg/integration/tests/worktree/bare_repo.go b/pkg/integration/tests/worktree/bare_repo.go index 2343b112e..cfcaa0280 100644 --- a/pkg/integration/tests/worktree/bare_repo.go +++ b/pkg/integration/tests/worktree/bare_repo.go @@ -38,6 +38,8 @@ var BareRepo = NewIntegrationTest(NewIntegrationTestArgs{ shell.RunCommand([]string{"git", "--git-dir", ".bare", "worktree", "add", "-b", "repo", "repo", "mybranch"}) shell.RunCommand([]string{"git", "--git-dir", ".bare", "worktree", "add", "-b", "worktree2", "worktree2", "mybranch"}) + + shell.Chdir("repo") }, Run: func(t *TestDriver, keys config.KeybindingConfig) { t.Views().Branches(). diff --git a/pkg/integration/tests/worktree/dotfile_bare_repo.go b/pkg/integration/tests/worktree/dotfile_bare_repo.go index 86b2a0fb4..7b8f68af1 100644 --- a/pkg/integration/tests/worktree/dotfile_bare_repo.go +++ b/pkg/integration/tests/worktree/dotfile_bare_repo.go @@ -12,9 +12,7 @@ var DotfileBareRepo = NewIntegrationTest(NewIntegrationTestArgs{ Description: "Open lazygit in the worktree of a dotfile bare repo and add a file and commit", ExtraCmdArgs: []string{"--git-dir={{.actualPath}}/.bare", "--work-tree={{.actualPath}}/repo"}, Skip: false, - // passing this because we're explicitly passing --git-dir and --work-tree args - UseCustomPath: true, - SetupConfig: func(config *config.AppConfig) {}, + SetupConfig: func(config *config.AppConfig) {}, SetupRepo: func(shell *Shell) { // we're going to have a directory structure like this: // project From a1ce6029c13ab69ae6e7d08a8f9593435abb5b92 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Fri, 12 Jan 2024 20:14:26 +1100 Subject: [PATCH 2/2] Pass -f as single arg in integration test For some bizarre reason `pkg/integration/tests/filter_by_path/cli_arg.go` is failing as of 8c716184 like so: ``` test_lazygit Usage: test_lazygit [git-arg] Positional Variables: git-arg Panel to focus upon opening lazygit. Accepted values (based on git terminology): status, branch, log, stash. Ignored if --filter arg is passed. Flags: -h --help Displays help with available flag, subcommand, and positional value parameters. -p --path Path of git repo. (equivalent to --work-tree= --git-dir=/.git/) -f --filter Path to filter on in `git log -- `. When in filter mode, the commits, reflog, and stash are filtered based on the given path, and some operations are restricted -v --version Print the current version -d --debug Run in debug mode with logging (see --logs flag below). Use the LOG_LEVEL env var to set the log level (debug/info/warn/error) (default: false) -l --logs Tail lazygit logs (intended to be used when `lazygit --debug` is called in a separate terminal tab) -c --config Print the default config -cd --print-config-dir Print the config directory -ucd --use-config-dir override default config directory with provided directory -w --work-tree equivalent of the --work-tree git argument -g --git-dir equivalent of the --git-dir git argument -ucf --use-config-file Comma separated list to custom config file(s) Unknown arguments supplied: filterFile ``` where the CLI args are: ``` ([]string) (len=5 cap=5) { (string) (len=25) "/tmp/lazygit/test_lazygit", (string) (len=6) "-debug", (string) (len=108) "--use-config-dir=/Users/jesseduffieldduffield/repos/lazygit/test/_results/filter_by_path/cli_arg/used_config", (string) (len=2) "-f", (string) (len=10) "filterFile" } ``` This appears to be a bug in flaggy itself. I've updated to the latest version but it still breaks. Bizarrely it works fine on CI and only fails locally. Running lazygit locally with `lg -f pkg/gui/controllers/helpers/refresh_helper.go` it works fine. So I don't know what's going on there. At any rate, I'm just going to get the test passing by passing `-f=filterFile` as a single argument. --- pkg/integration/tests/filter_by_path/cli_arg.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/integration/tests/filter_by_path/cli_arg.go b/pkg/integration/tests/filter_by_path/cli_arg.go index de368c17d..5b3912829 100644 --- a/pkg/integration/tests/filter_by_path/cli_arg.go +++ b/pkg/integration/tests/filter_by_path/cli_arg.go @@ -7,7 +7,7 @@ import ( var CliArg = NewIntegrationTest(NewIntegrationTestArgs{ Description: "Filter commits by file path, using CLI arg", - ExtraCmdArgs: []string{"-f", "filterFile"}, + ExtraCmdArgs: []string{"-f=filterFile"}, Skip: false, SetupConfig: func(config *config.AppConfig) { },