Don't let a broken fixture take down the whole test binary

A failing setup step called Shell.fail, which panicked. Tests run as
parallel subtests, so that panic aborted the entire test binary: one bad
fixture cost us the results of all ~500 tests, and the failure was
reported as a stack trace rather than against the test that caused it.

Keep panicking to skip the remaining setup steps -- they would only
produce follow-on failures -- but recover in createFixture and return the
message as that test's error. All three clients already propagate an
error from a test, so they report it the way they report any other
failure.
This commit is contained in:
Stefan Haller 2026-08-05 17:02:06 +02:00
parent 4ec91a0bf5
commit 34da956f5d
2 changed files with 48 additions and 6 deletions

View file

@ -1,6 +1,7 @@
package components package components
import ( import (
"errors"
"fmt" "fmt"
"os" "os"
"os/exec" "os/exec"
@ -159,9 +160,7 @@ func prepareTestDir(
return "", err return "", err
} }
workingDir := createFixture(test, paths, rootDir) return createFixture(test, paths, rootDir)
return workingDir, nil
} }
func buildLazygit(testArgs RunTestArgs) error { func buildLazygit(testArgs RunTestArgs) error {
@ -182,22 +181,41 @@ func buildLazygit(testArgs RunTestArgs) error {
return osCommand.Cmd.New(args).Run() return osCommand.Cmd.New(args).Run()
} }
// A failing setup step panics with this so that the remaining steps, which
// would only produce follow-on failures, are skipped.
type fixtureFailure string
// Sets up the fixture for test and returns the working directory to invoke // Sets up the fixture for test and returns the working directory to invoke
// lazygit in. // lazygit in.
func createFixture(test *IntegrationTest, paths Paths, rootDir string) string { func createFixture(test *IntegrationTest, paths Paths, rootDir string) (workingDir string, err error) {
// Tests run as parallel subtests, and a panic escaping one of them takes
// down the whole test binary, discarding every other test's result along
// with it. Report a broken fixture as this test's error instead.
defer func() {
panicValue := recover()
if panicValue == nil {
return
}
failure, ok := panicValue.(fixtureFailure)
if !ok {
panic(panicValue)
}
err = errors.New(string(failure))
}()
env := NewTestEnvironment(rootDir) env := NewTestEnvironment(rootDir)
env = append(env, fmt.Sprintf("%s=%s", PWD, paths.ActualRepo())) env = append(env, fmt.Sprintf("%s=%s", PWD, paths.ActualRepo()))
shell := NewShell( shell := NewShell(
paths.ActualRepo(), paths.ActualRepo(),
env, env,
func(errorMsg string) { panic(errorMsg) }, func(errorMsg string) { panic(fixtureFailure(errorMsg)) },
) )
shell.Init() shell.Init()
test.SetupRepo(shell) test.SetupRepo(shell)
return shell.dir return shell.dir, nil
} }
func testPath(rootdir string) string { func testPath(rootdir string) string {

View file

@ -1,8 +1,11 @@
package components package components
import ( import (
"os"
"path/filepath"
"testing" "testing"
lazycoreUtils "github.com/jesseduffield/lazycore/pkg/utils"
"github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/git_commands"
"github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/models"
"github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/config"
@ -158,6 +161,27 @@ func TestSuccess(t *testing.T) {
assert.Equal(t, "", driver.failureMessage) assert.Equal(t, "", driver.failureMessage)
} }
func TestFailingFixture(t *testing.T) {
test := NewIntegrationTest(NewIntegrationTestArgs{
Description: unitTestDescription,
SetupRepo: func(shell *Shell) {
shell.RunCommand([]string{"git", "checkout", "no-such-branch"})
shell.CreateFile("reached.txt", "")
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {},
})
paths := NewPaths(t.TempDir())
assert.NoError(t, os.MkdirAll(paths.ActualRepo(), 0o777))
workingDir, err := createFixture(test, paths, lazycoreUtils.GetLazyRootDirectory())
assert.ErrorContains(t, err, "git checkout no-such-branch")
assert.Empty(t, workingDir)
// the steps following the failing one are skipped
assert.NoFileExists(t, filepath.Join(paths.ActualRepo(), "reached.txt"))
}
func TestGitVersionRestriction(t *testing.T) { func TestGitVersionRestriction(t *testing.T) {
scenarios := []struct { scenarios := []struct {
testName string testName string