From 26d180a50a29f667d3af3507683c76d8d0d88897 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 9 Sep 2023 13:38:11 +0200 Subject: [PATCH 1/5] Fix minor resource leak in runCmdHeadless We still want to close the pty if the command failed. --- pkg/integration/clients/go_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/integration/clients/go_test.go b/pkg/integration/clients/go_test.go index 01f174211..89d8a0653 100644 --- a/pkg/integration/clients/go_test.go +++ b/pkg/integration/clients/go_test.go @@ -85,6 +85,7 @@ func runCmdHeadless(cmd *exec.Cmd) error { _, _ = io.Copy(io.Discard, f) if cmd.Wait() != nil { + _ = f.Close() // return an error with the stderr output return errors.New(stderr.String()) } From 8081b59a02fee986a04c359b43b5100cce36756f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 21 Sep 2023 09:29:12 +0200 Subject: [PATCH 2/5] Allow passing multiple flags to the cli runner This is useful for example to pass both -slow and -debug. Since we're about to add yet another flag in the next commit, it becomes even more important. Plus, it makes the code a little nicer too. --- cmd/integration_test/main.go | 43 +++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/cmd/integration_test/main.go b/cmd/integration_test/main.go index 7153691da..c7952e51a 100644 --- a/cmd/integration_test/main.go +++ b/cmd/integration_test/main.go @@ -26,6 +26,29 @@ Usage: > go run cmd/integration_test/main.go help ` +type flagInfo struct { + name string // name of the flag; can be used with "-" or "--" + flag *bool // a pointer to the variable that should be set to true when this flag is passed +} + +// Takes the args that you want to parse (excluding the program name and any +// subcommands), and returns the remaining args with the flags removed +func parseFlags(args []string, flags []flagInfo) []string { +outer: + for len(args) > 0 { + for _, f := range flags { + if args[0] == "-"+f.name || args[0] == "--"+f.name { + *f.flag = true + args = args[1:] + continue outer + } + } + break + } + + return args +} + func main() { if len(os.Args) < 2 { log.Fatal(usage) @@ -35,24 +58,14 @@ func main() { case "help": fmt.Println(usage) case "cli": - testNames := os.Args[2:] slow := false sandbox := false waitForDebugger := false - // get the next arg if it's --slow - if len(os.Args) > 2 { - if os.Args[2] == "--slow" || os.Args[2] == "-slow" { - testNames = os.Args[3:] - slow = true - } else if os.Args[2] == "--sandbox" || os.Args[2] == "-sandbox" { - testNames = os.Args[3:] - sandbox = true - } else if os.Args[2] == "--debug" || os.Args[2] == "-debug" { - testNames = os.Args[3:] - waitForDebugger = true - } - } - + testNames := parseFlags(os.Args[2:], []flagInfo{ + {"slow", &slow}, + {"sandbox", &sandbox}, + {"debug", &waitForDebugger}, + }) clients.RunCLI(testNames, slow, sandbox, waitForDebugger) case "tui": clients.RunTUI() From f108fd2236ad7d61f0c4ee824b265653ef1d8dad Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 8 Sep 2023 17:13:30 +0200 Subject: [PATCH 3/5] Support -race arg when running integration tests to turn on go's race detector For the "cli" and "tui" modes of the test runner there's a "-race" parameter to turn it on; for running tests on CI with go test, you turn it on by setting the environment variable LAZYGIT_RACE_DETECTOR to a non-empty value. --- cmd/integration_test/main.go | 13 +++++++++++-- pkg/integration/clients/cli.go | 3 ++- pkg/integration/clients/go_test.go | 2 ++ pkg/integration/clients/tui.go | 17 +++++++++-------- pkg/integration/components/runner.go | 21 ++++++++++++++++----- 5 files changed, 40 insertions(+), 16 deletions(-) diff --git a/cmd/integration_test/main.go b/cmd/integration_test/main.go index c7952e51a..711ce8b33 100644 --- a/cmd/integration_test/main.go +++ b/cmd/integration_test/main.go @@ -61,14 +61,23 @@ func main() { slow := false sandbox := false waitForDebugger := false + raceDetector := false testNames := parseFlags(os.Args[2:], []flagInfo{ {"slow", &slow}, {"sandbox", &sandbox}, {"debug", &waitForDebugger}, + {"race", &raceDetector}, }) - clients.RunCLI(testNames, slow, sandbox, waitForDebugger) + clients.RunCLI(testNames, slow, sandbox, waitForDebugger, raceDetector) case "tui": - clients.RunTUI() + raceDetector := false + remainingArgs := parseFlags(os.Args[2:], []flagInfo{ + {"race", &raceDetector}, + }) + if len(remainingArgs) > 0 { + log.Fatal("tui only supports the -race argument.") + } + clients.RunTUI(raceDetector) default: log.Fatal(usage) } diff --git a/pkg/integration/clients/cli.go b/pkg/integration/clients/cli.go index 571dea491..974f3493c 100644 --- a/pkg/integration/clients/cli.go +++ b/pkg/integration/clients/cli.go @@ -23,7 +23,7 @@ import ( // If invoked directly, you can specify tests to run by passing their names as positional arguments -func RunCLI(testNames []string, slow bool, sandbox bool, waitForDebugger bool) { +func RunCLI(testNames []string, slow bool, sandbox bool, waitForDebugger bool, raceDetector bool) { inputDelay := tryConvert(os.Getenv("INPUT_DELAY"), 0) if slow { inputDelay = SLOW_INPUT_DELAY @@ -36,6 +36,7 @@ func RunCLI(testNames []string, slow bool, sandbox bool, waitForDebugger bool) { runAndPrintFatalError, sandbox, waitForDebugger, + raceDetector, inputDelay, 1, ) diff --git a/pkg/integration/clients/go_test.go b/pkg/integration/clients/go_test.go index 89d8a0653..ccaae2bd9 100644 --- a/pkg/integration/clients/go_test.go +++ b/pkg/integration/clients/go_test.go @@ -27,6 +27,7 @@ func TestIntegration(t *testing.T) { parallelTotal := tryConvert(os.Getenv("PARALLEL_TOTAL"), 1) parallelIndex := tryConvert(os.Getenv("PARALLEL_INDEX"), 0) + raceDetector := os.Getenv("LAZYGIT_RACE_DETECTOR") != "" testNumber := 0 err := components.RunTests( @@ -53,6 +54,7 @@ func TestIntegration(t *testing.T) { }, false, false, + raceDetector, 0, // Allow two attempts at each test to get around flakiness 2, diff --git a/pkg/integration/clients/tui.go b/pkg/integration/clients/tui.go index 64610ccb3..7ad9cb47f 100644 --- a/pkg/integration/clients/tui.go +++ b/pkg/integration/clients/tui.go @@ -21,7 +21,7 @@ import ( var SLOW_INPUT_DELAY = 600 -func RunTUI() { +func RunTUI(raceDetector bool) { rootDir := utils.GetLazyRootDirectory() testDir := filepath.Join(rootDir, "test", "integration") @@ -85,7 +85,7 @@ func RunTUI() { return nil } - suspendAndRunTest(currentTest, true, false, 0) + suspendAndRunTest(currentTest, true, false, raceDetector, 0) return nil }); err != nil { @@ -98,7 +98,7 @@ func RunTUI() { return nil } - suspendAndRunTest(currentTest, false, false, 0) + suspendAndRunTest(currentTest, false, false, raceDetector, 0) return nil }); err != nil { @@ -111,7 +111,7 @@ func RunTUI() { return nil } - suspendAndRunTest(currentTest, false, false, SLOW_INPUT_DELAY) + suspendAndRunTest(currentTest, false, false, raceDetector, SLOW_INPUT_DELAY) return nil }); err != nil { @@ -124,7 +124,7 @@ func RunTUI() { return nil } - suspendAndRunTest(currentTest, false, true, 0) + suspendAndRunTest(currentTest, false, true, raceDetector, 0) return nil }); err != nil { @@ -284,12 +284,12 @@ func (self *app) wrapEditor(f func(v *gocui.View, key gocui.Key, ch rune, mod go } } -func suspendAndRunTest(test *components.IntegrationTest, sandbox bool, waitForDebugger bool, inputDelay int) { +func suspendAndRunTest(test *components.IntegrationTest, sandbox bool, waitForDebugger bool, raceDetector bool, inputDelay int) { if err := gocui.Screen.Suspend(); err != nil { panic(err) } - runTuiTest(test, sandbox, waitForDebugger, inputDelay) + runTuiTest(test, sandbox, waitForDebugger, raceDetector, inputDelay) fmt.Fprintf(os.Stdout, "\n%s", style.FgGreen.Sprint("press enter to return")) fmt.Scanln() // wait for enter press @@ -384,7 +384,7 @@ func quit(g *gocui.Gui, v *gocui.View) error { return gocui.ErrQuit } -func runTuiTest(test *components.IntegrationTest, sandbox bool, waitForDebugger bool, inputDelay int) { +func runTuiTest(test *components.IntegrationTest, sandbox bool, waitForDebugger bool, raceDetector bool, inputDelay int) { err := components.RunTests( []*components.IntegrationTest{test}, log.Printf, @@ -392,6 +392,7 @@ func runTuiTest(test *components.IntegrationTest, sandbox bool, waitForDebugger runAndPrintError, sandbox, waitForDebugger, + raceDetector, inputDelay, 1, ) diff --git a/pkg/integration/components/runner.go b/pkg/integration/components/runner.go index 4a9d624ad..569be8d28 100644 --- a/pkg/integration/components/runner.go +++ b/pkg/integration/components/runner.go @@ -30,6 +30,7 @@ func RunTests( testWrapper func(test *IntegrationTest, f func() error), sandbox bool, waitForDebugger bool, + raceDetector bool, inputDelay int, maxAttempts int, ) error { @@ -41,7 +42,7 @@ func RunTests( testDir := filepath.Join(projectRootDir, "test", "_results") - if err := buildLazygit(); err != nil { + if err := buildLazygit(raceDetector); err != nil { return err } @@ -131,15 +132,18 @@ func prepareTestDir( return createFixture(test, paths, rootDir) } -func buildLazygit() error { +func buildLazygit(raceDetector bool) error { // // TODO: remove this line! // // skipping this because I'm not making changes to the app code atm. // return nil + args := []string{"go", "build"} + if raceDetector { + args = append(args, "-race") + } + args = append(args, "-o", tempLazygitPath(), filepath.FromSlash("pkg/integration/clients/injector/main.go")) osCommand := oscommands.NewDummyOSCommand() - return osCommand.Cmd.New([]string{ - "go", "build", "-o", tempLazygitPath(), filepath.FromSlash("pkg/integration/clients/injector/main.go"), - }).Run() + return osCommand.Cmd.New(args).Run() } func createFixture(test *IntegrationTest, paths Paths, rootDir string) error { @@ -202,6 +206,9 @@ func getLazygitCommand(test *IntegrationTest, paths Paths, rootDir string, sandb if waitForDebugger { cmdObj.AddEnvVars("WAIT_FOR_DEBUGGER=true") } + // Set a race detector log path only to avoid spamming the terminal with the + // logs. We are not showing this anywhere yet. + cmdObj.AddEnvVars(fmt.Sprintf("GORACE=log_path=%s", raceDetectorLogsPath())) if test.ExtraEnvVars() != nil { for key, value := range test.ExtraEnvVars() { cmdObj.AddEnvVars(fmt.Sprintf("%s=%s", key, value)) @@ -221,6 +228,10 @@ func tempLazygitPath() string { return filepath.Join("/tmp", "lazygit", "test_lazygit") } +func raceDetectorLogsPath() string { + return filepath.Join("/tmp", "lazygit", "race_log") +} + func findOrCreateDir(path string) { _, err := os.Stat(path) if err != nil { From 59cc6843e6e9b969759291571cab876b0a5cdac4 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 9 Sep 2023 13:41:26 +0200 Subject: [PATCH 4/5] Print race detector logs after running a test with -race --- pkg/integration/clients/cli.go | 7 +++++-- pkg/integration/clients/go_test.go | 8 ++++---- pkg/integration/components/runner.go | 20 +++++++++++++------- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/pkg/integration/clients/cli.go b/pkg/integration/clients/cli.go index 974f3493c..d6cf5a628 100644 --- a/pkg/integration/clients/cli.go +++ b/pkg/integration/clients/cli.go @@ -88,12 +88,15 @@ outer: return testsToRun } -func runCmdInTerminal(cmd *exec.Cmd) error { +func runCmdInTerminal(cmd *exec.Cmd) (int, error) { cmd.Stdout = os.Stdout cmd.Stdin = os.Stdin cmd.Stderr = os.Stderr - return cmd.Run() + if err := cmd.Start(); err != nil { + return -1, err + } + return cmd.Process.Pid, cmd.Wait() } func tryConvert(numStr string, defaultVal int) int { diff --git a/pkg/integration/clients/go_test.go b/pkg/integration/clients/go_test.go index ccaae2bd9..6503621ba 100644 --- a/pkg/integration/clients/go_test.go +++ b/pkg/integration/clients/go_test.go @@ -63,7 +63,7 @@ func TestIntegration(t *testing.T) { assert.NoError(t, err) } -func runCmdHeadless(cmd *exec.Cmd) error { +func runCmdHeadless(cmd *exec.Cmd) (int, error) { cmd.Env = append( cmd.Env, "HEADLESS=true", @@ -81,7 +81,7 @@ func runCmdHeadless(cmd *exec.Cmd) error { // running other commands in a pty. f, err := pty.StartWithSize(cmd, &pty.Winsize{Rows: 300, Cols: 300}) if err != nil { - return err + return -1, err } _, _ = io.Copy(io.Discard, f) @@ -89,8 +89,8 @@ func runCmdHeadless(cmd *exec.Cmd) error { if cmd.Wait() != nil { _ = f.Close() // return an error with the stderr output - return errors.New(stderr.String()) + return cmd.Process.Pid, errors.New(stderr.String()) } - return f.Close() + return cmd.Process.Pid, f.Close() } diff --git a/pkg/integration/components/runner.go b/pkg/integration/components/runner.go index 569be8d28..821319dc1 100644 --- a/pkg/integration/components/runner.go +++ b/pkg/integration/components/runner.go @@ -26,7 +26,7 @@ const ( func RunTests( tests []*IntegrationTest, logf func(format string, formatArgs ...interface{}), - runCmd func(cmd *exec.Cmd) error, + runCmd func(cmd *exec.Cmd) (int, error), testWrapper func(test *IntegrationTest, f func() error), sandbox bool, waitForDebugger bool, @@ -60,7 +60,7 @@ func RunTests( ) for i := 0; i < maxAttempts; i++ { - err := runTest(test, paths, projectRootDir, logf, runCmd, sandbox, waitForDebugger, inputDelay, gitVersion) + err := runTest(test, paths, projectRootDir, logf, runCmd, sandbox, waitForDebugger, raceDetector, inputDelay, gitVersion) if err != nil { if i == maxAttempts-1 { return err @@ -83,9 +83,10 @@ func runTest( paths Paths, projectRootDir string, logf func(format string, formatArgs ...interface{}), - runCmd func(cmd *exec.Cmd) error, + runCmd func(cmd *exec.Cmd) (int, error), sandbox bool, waitForDebugger bool, + raceDetector bool, inputDelay int, gitVersion *git_commands.GitVersion, ) error { @@ -108,12 +109,17 @@ func runTest( return err } - err = runCmd(cmd) - if err != nil { - return err + pid, err := runCmd(cmd) + + // Print race detector log regardless of the command's exit status + if raceDetector { + logPath := fmt.Sprintf("%s.%d", raceDetectorLogsPath(), pid) + if bytes, err := os.ReadFile(logPath); err == nil { + logf("Race detector log:\n" + string(bytes)) + } } - return nil + return err } func prepareTestDir( From 508b869773a4d37cf24c8248a4e959bd807b1c78 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 9 Sep 2023 17:33:58 +0200 Subject: [PATCH 5/5] Pass MAKECMDGOALS to make integration-test-tui We need this to be able to pass the "-race" argument, i.e. make integration-test-tui -- -race --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f10109c19..a220b68b3 100644 --- a/Makefile +++ b/Makefile @@ -44,7 +44,7 @@ update-cheatsheet: # For more details about integration test, see https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md. .PHONY: integration-test-tui integration-test-tui: - go run cmd/integration_test/main.go tui + go run cmd/integration_test/main.go tui $(filter-out $@,$(MAKECMDGOALS)) .PHONY: integration-test-cli integration-test-cli: