Add coverage arg for integration tests

This PR captures the code coverage from our unit and integration tests. At the
moment it simply pushes the result to Codacy, a platform that assists with
improving code health. Right now the focus is just getting visibility but I want
to experiment with alerts on PRs when a PR causes a drop in code coverage.

To be clear: I'm not a dogmatist about this: I have no aspirations to get to
100% code coverage, and I don't consider lines-of-code-covered to be a perfect
metric, but it is a pretty good heuristic for how extensive your tests are.

The good news is that our coverage is actually pretty good which was a surprise
to me!

As a conflict of interest statement: I'm in Codacy's 'Pioneers' program which
provides funding and mentorship, and part of the arrangement is to use Codacy's
tooling on lazygit. This is something I'd have been happy to explore even
without being part of the program, and just like with any other static analysis
tool, we can tweak it to fit our use case and values.

## How we're capturing code coverage

This deserves its own section. Basically when you build the lazygit binary you
can specify that you want the binary to capture coverage information when it
runs. Then, if you run the binary with a GOCOVERDIR env var, it will write
coverage information to that directory before exiting.

It's a similar story with unit tests except with those you just specify the
directory inline via `-test.gocoverdir`.

We run both unit tests and integration tests separately in CI, _and_ we run them
parallel with different OS's and git versions. So I've got each step uploading
the coverage files as an artefact, and then in a separate step we combine all
the artefacts together and generate a combined coverage file, which we then
upload to codacy (but in future we can do other things with it like warn in a PR
if code coverage decreases too much).

Another caveat is that when running integration tests, not only do we want to
obtain code coverage from code executed by the test binary, we also want to
obtain code coverage from code executed by the test runner. Otherwise, for each
integration test you add, the setup code (which is run by the test runner, not
the test binary) will be considered un-covered and for a large setup step it may
appear that your PR _decreases_ coverage on net. Go doesn't easily let you
exclude directories from coverage reports so it's better to just track the
coverage from both the runner and the binary.

The binary expects a GOCOVERDIR env var but the test runner expects a
test.gocoverdir positional arg and if you pass the positional arg it will
internally overwrite GOCOVERDIR to some random temp directory and if you then
pass that to the test binary, it doesn't seem to actually write to it by the
time the test finishes. So to get around that we're using LAZYGIT_GOCOVERDIR and
then within the test runner we're mapping that to GOCOVERDIR before running the
test binary. So they both end up writing to the same directory. Coverage data
files are named to avoid conflicts, including something unique to the process,
so we don't need to worry about name collisions between the test runner and the
test binary's coverage files. We then merge the files together purely for the
sake of having fewer artefacts to upload.

## Misc

Initially I was able to have all the instances of '/tmp/code_coverage' confined
to the ci.yml which was good because it was all in one place but now it's spread
across ci.yml and scripts/run_integration_tests.sh and I don't feel great about
that but can't think of a way to make it cleaner.

I believe there's a use case for running scripts/run_integration_tests.sh
outside of CI (so that you can run tests against older git versions locally) so
I've made it that unless you pass the LAZYGIT_GOCOVERDIR env var to that script,
it skips all the code coverage stuff.

On a separate note: it seems that Go's coverage report is based on percentage of
statements executed, whereas codacy cares more about lines of code executed, so
codacy reports a higher percentage (e.g. 82%) than Go's own coverage report
(74%).

.github/workflows/ci.yml

@@ -36,7 +36,14 @@ jobs:
       - name: Test code
         # we're passing -short so that we skip the integration tests, which will be run in parallel below
         run: |
-          go test ./... -short
+          mkdir -p /tmp/code_coverage
+          go test ./... -short -cover -args "-test.gocoverdir=/tmp/code_coverage"
+      - name: Upload code coverage artifacts
+        uses: actions/upload-artifact@v3
+        with:
+          name: coverage-unit-${{ matrix.os }}-${{ github.run_id }}
+          path: /tmp/code_coverage
+
   integration-tests:
     strategy:
       fail-fast: false
@@ -86,8 +93,17 @@ jobs:
       - name: Print git version
         run: git --version
       - name: Test code
+        env:
+          # See https://go.dev/blog/integration-test-coverage
+          LAZYGIT_GOCOVERDIR: /tmp/code_coverage
         run: |
+          mkdir -p /tmp/code_coverage
           ./scripts/run_integration_tests.sh
+      - name: Upload code coverage artifacts
+        uses: actions/upload-artifact@v3
+        with:
+          name: coverage-integration-${{ matrix.git-version }}-${{ github.run_id }}
+          path: /tmp/code_coverage
   build:
     runs-on: ubuntu-latest
     env:
@@ -169,3 +185,31 @@ jobs:
           mode: exactly
           count: 1
           labels: "ignore-for-release, feature, enhancement, bug, maintenance, docs, i18n"
+  upload-coverage:
+    # List all jobs that produce coverage files
+    needs: [unit-tests, integration-tests]
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v3
+
+      - name: Download all coverage artifacts
+        uses: actions/download-artifact@v3
+        with:
+          path: /tmp/code_coverage
+
+      - name: Combine coverage files
+        run: |
+          # Find all directories in /tmp/code_coverage and create a comma-separated list
+          COVERAGE_DIRS=$(find /tmp/code_coverage -mindepth 1 -maxdepth 1 -type d -printf '/tmp/code_coverage/%f,' | sed 's/,$//')
+          echo "Coverage directories: $COVERAGE_DIRS"
+          # Run the combine command with the generated list
+          go tool covdata textfmt -i=$COVERAGE_DIRS -o coverage.out
+          echo "Combined coverage:"
+          go tool cover -func coverage.out | tail -1 | awk '{print $3}'
+
+      - name: Upload to Codacy
+        run: |
+          CODACY_PROJECT_TOKEN=${{ secrets.CODACY_PROJECT_TOKEN }} \
+            bash <(curl -Ls https://coverage.codacy.com/get.sh) report \
+            --force-coverage-parser go -r coverage.out

.gitignore

@@ -42,3 +42,5 @@ __debug_bin
 
 .worktrees
 demo/output/*
+
+coverage.out

pkg/integration/clients/cli.go

@@ -37,6 +37,7 @@ func RunCLI(testNames []string, slow bool, sandbox bool, waitForDebugger bool, r
 		Sandbox:         sandbox,
 		WaitForDebugger: waitForDebugger,
 		RaceDetector:    raceDetector,
+		CodeCoverageDir: "",
 		InputDelay:      inputDelay,
 		MaxAttempts:     1,
 	})

pkg/integration/clients/go_test.go

@@ -28,6 +28,10 @@ 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") != ""
+	// LAZYGIT_GOCOVERDIR is the directory where we write coverage files to. If this directory
+	// is defined, go binaries built with the -cover flag will write coverage files to
+	// to it.
+	codeCoverageDir := os.Getenv("LAZYGIT_GOCOVERDIR")
 	testNumber := 0
 
 	err := components.RunTests(components.RunTestArgs{
@@ -55,6 +59,7 @@ func TestIntegration(t *testing.T) {
 		Sandbox:         false,
 		WaitForDebugger: false,
 		RaceDetector:    raceDetector,
+		CodeCoverageDir: codeCoverageDir,
 		InputDelay:      0,
 		// Allow two attempts at each test to get around flakiness
 		MaxAttempts: 2,

pkg/integration/clients/tui.go

@@ -393,6 +393,7 @@ func runTuiTest(test *components.IntegrationTest, sandbox bool, waitForDebugger
 		Sandbox:         sandbox,
 		WaitForDebugger: waitForDebugger,
 		RaceDetector:    raceDetector,
+		CodeCoverageDir: "",
 		InputDelay:      inputDelay,
 		MaxAttempts:     1,
 	})

pkg/integration/components/runner.go

@@ -28,6 +28,7 @@ type RunTestArgs struct {
 	Sandbox         bool
 	WaitForDebugger bool
 	RaceDetector    bool
+	CodeCoverageDir string
 	InputDelay      int
 	MaxAttempts     int
 }
@@ -44,7 +45,7 @@ func RunTests(args RunTestArgs) error {
 	}
 
 	testDir := filepath.Join(projectRootDir, "test", "_results")
-	if err := buildLazygit(args.WaitForDebugger, args.RaceDetector); err != nil {
+	if err := buildLazygit(args); err != nil {
 		return err
 	}
 
@@ -62,7 +63,7 @@ func RunTests(args RunTestArgs) error {
 			)
 
 			for i := 0; i < args.MaxAttempts; i++ {
-				err := runTest(test, paths, projectRootDir, args.Logf, args.RunCmd, args.Sandbox, args.WaitForDebugger, args.RaceDetector, args.InputDelay, gitVersion)
+				err := runTest(test, args, paths, projectRootDir, gitVersion)
 				if err != nil {
 					if i == args.MaxAttempts-1 {
 						return err
@@ -82,23 +83,18 @@ func RunTests(args RunTestArgs) error {
 
 func runTest(
 	test *IntegrationTest,
+	args RunTestArgs,
 	paths Paths,
 	projectRootDir string,
-	logf func(format string, formatArgs ...interface{}),
-	runCmd func(cmd *exec.Cmd) (int, error),
-	sandbox bool,
-	waitForDebugger bool,
-	raceDetector bool,
-	inputDelay int,
 	gitVersion *git_commands.GitVersion,
 ) error {
 	if test.Skip() {
-		logf("Skipping test %s", test.Name())
+		args.Logf("Skipping test %s", test.Name())
 		return nil
 	}
 
 	if !test.ShouldRunForGitVersion(gitVersion) {
-		logf("Skipping test %s for git version %d.%d.%d", test.Name(), gitVersion.Major, gitVersion.Minor, gitVersion.Patch)
+		args.Logf("Skipping test %s for git version %d.%d.%d", test.Name(), gitVersion.Major, gitVersion.Minor, gitVersion.Patch)
 		return nil
 	}
 
@@ -106,18 +102,18 @@ func runTest(
 		return err
 	}
 
-	cmd, err := getLazygitCommand(test, paths, projectRootDir, sandbox, waitForDebugger, inputDelay)
+	cmd, err := getLazygitCommand(test, args, paths, projectRootDir)
 	if err != nil {
 		return err
 	}
 
-	pid, err := runCmd(cmd)
+	pid, err := args.RunCmd(cmd)
 
 	// Print race detector log regardless of the command's exit status
-	if raceDetector {
+	if args.RaceDetector {
 		logPath := fmt.Sprintf("%s.%d", raceDetectorLogsPath(), pid)
 		if bytes, err := os.ReadFile(logPath); err == nil {
-			logf("Race detector log:\n" + string(bytes))
+			args.Logf("Race detector log:\n" + string(bytes))
 		}
 	}
 
@@ -140,20 +136,19 @@ func prepareTestDir(
 	return createFixture(test, paths, rootDir)
 }
 
-func buildLazygit(debug bool, raceDetector bool) error {
-	// // TODO: remove this line!
-	// // skipping this because I'm not making changes to the app code atm.
-	// return nil
-
+func buildLazygit(testArgs RunTestArgs) error {
 	args := []string{"go", "build"}
-	if debug {
+	if testArgs.WaitForDebugger {
 		// Disable compiler optimizations (-N) and inlining (-l) because this
 		// makes debugging work better
 		args = append(args, "-gcflags=all=-N -l")
 	}
-	if raceDetector {
+	if testArgs.RaceDetector {
 		args = append(args, "-race")
 	}
+	if testArgs.CodeCoverageDir != "" {
+		args = append(args, "-cover")
+	}
 	args = append(args, "-o", tempLazygitPath(), filepath.FromSlash("pkg/integration/clients/injector/main.go"))
 	osCommand := oscommands.NewDummyOSCommand()
 	return osCommand.Cmd.New(args).Run()
@@ -184,7 +179,7 @@ func getGitVersion() (*git_commands.GitVersion, error) {
 	return git_commands.ParseGitVersion(versionStr)
 }
 
-func getLazygitCommand(test *IntegrationTest, paths Paths, rootDir string, sandbox bool, waitForDebugger bool, inputDelay int) (*exec.Cmd, error) {
+func getLazygitCommand(test *IntegrationTest, args RunTestArgs, paths Paths, rootDir string) (*exec.Cmd, error) {
 	osCommand := oscommands.NewDummyOSCommand()
 
 	err := os.RemoveAll(paths.Config())
@@ -212,11 +207,18 @@ func getLazygitCommand(test *IntegrationTest, paths Paths, rootDir string, sandb
 
 	cmdObj := osCommand.Cmd.New(cmdArgs)
 
+	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
+		// it writes to and so if we pass GOCOVERDIR to that, it will be overwritten.
+		cmdObj.AddEnvVars("GOCOVERDIR=" + args.CodeCoverageDir)
+	}
+
 	cmdObj.AddEnvVars(fmt.Sprintf("%s=%s", TEST_NAME_ENV_VAR, test.Name()))
-	if sandbox {
+	if args.Sandbox {
 		cmdObj.AddEnvVars(fmt.Sprintf("%s=%s", SANDBOX_ENV_VAR, "true"))
 	}
-	if waitForDebugger {
+	if args.WaitForDebugger {
 		cmdObj.AddEnvVars(fmt.Sprintf("%s=true", WAIT_FOR_DEBUGGER_ENV_VAR))
 	}
 	// Set a race detector log path only to avoid spamming the terminal with the
@@ -228,8 +230,8 @@ func getLazygitCommand(test *IntegrationTest, paths Paths, rootDir string, sandb
 		}
 	}
 
-	if inputDelay > 0 {
-		cmdObj.AddEnvVars(fmt.Sprintf("INPUT_DELAY=%d", inputDelay))
+	if args.InputDelay > 0 {
+		cmdObj.AddEnvVars(fmt.Sprintf("INPUT_DELAY=%d", args.InputDelay))
 	}
 
 	cmdObj.AddEnvVars(fmt.Sprintf("%s=%s", GIT_CONFIG_GLOBAL_ENV_VAR, globalGitConfigPath(rootDir)))

scripts/run_integration_tests.sh

@@ -10,7 +10,25 @@ fi
 
 cp test/global_git_config ~/.gitconfig
 
-go test pkg/integration/clients/*.go
+# if the LAZYGIT_GOCOVERDIR env var is set, we'll capture code coverage data
+if [ -n "$LAZYGIT_GOCOVERDIR" ]; then
+  # Go expects us to either be running the test binary directly or running `go test`, but because
+  # we're doing both and because we want to combine coverage data for both, we need to be a little
+  # hacky. To capture the coverage data for the test runner we pass the test.gocoverdir positional
+  # arg, but if we do that then the GOCOVERDIR env var (which you typically pass to the test binary) will be overwritten by the test runner. So we're passing LAZYGIT_COCOVERDIR instead
+  # and then internally passing that to the test binary as GOCOVERDIR.
+  go test -cover -coverpkg=github.com/jesseduffield/lazygit/pkg/... pkg/integration/clients/*.go -args -test.gocoverdir="/tmp/code_coverage"
+
+  # We're merging the coverage data for the sake of having fewer artefacts to upload.
+  # We can't merge inline so we're merging to a tmp dir then moving back to the original.
+  mkdir -p /tmp/code_coverage_merged
+  go tool covdata merge -i=/tmp/code_coverage -o=/tmp/code_coverage_merged
+  rm -rf /tmp/code_coverage
+  mv /tmp/code_coverage_merged /tmp/code_coverage
+else
+  go test pkg/integration/clients/*.go
+fi
+
 EXITCODE=$?
 
 if test -f ~/.gitconfig.lazygit.bak; then