Skip to content

Return struct CommandOutput rather than two strings. #2520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ser-io merged 1 commit into from
May 7, 2026
Merged

Return struct CommandOutput rather than two strings. #2520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion e2etests/cvd/bugreport_tests/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestTakeBugreport(t *testing.T) {
t.Fatal(err)
}

if _, _, err := c.RunCmd(c.TargetBin(), "host_bugreport", "--output=/tmp/host_bugreport.zip", "--include_adb_bugreport=true"); err != nil {
if _, err := c.RunCmd(c.TargetBin(), "host_bugreport", "--output=/tmp/host_bugreport.zip", "--include_adb_bugreport=true"); err != nil {
t.Fatal(err)
}

Expand Down
46 changes: 26 additions & 20 deletions e2etests/cvd/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,13 @@ type TestContext struct {
usePodcvd bool
}

func runCmdWithContextEnv(ctx context.Context, command []string, envvars map[string]string) (string, string, error) {
// Output of running a command.
type CommandOutput struct {
Stdout string
Stderr string
}

func runCmdWithContextEnv(ctx context.Context, command []string, envvars map[string]string) (CommandOutput, error) {
cmd := exec.CommandContext(ctx, command[0], command[1:]...)

stdOutBuf := bytes.Buffer{}
Expand All @@ -99,14 +105,14 @@ func runCmdWithContextEnv(ctx context.Context, command []string, envvars map[str
log.Printf("Running `%s %s`\n", strings.Join(envvarPairs, " "), strings.Join(command, " "))
err := cmd.Run()
if err != nil {
return "", "", fmt.Errorf("`%s` failed: %w", strings.Join(command, " "), err)
return CommandOutput{}, fmt.Errorf("`%s` failed: %w", strings.Join(command, " "), err)
}

return stdOutBuf.String(), stdErrBuf.String(), nil
return CommandOutput{Stdout: stdOutBuf.String(), Stderr: stdErrBuf.String()}, nil
}

// Runs the given command with the given set of envvars overrided.
func (tc *TestContext) RunCmdWithEnv(command []string, envvars map[string]string) (string, string, error) {
func (tc *TestContext) RunCmdWithEnv(command []string, envvars map[string]string) (CommandOutput, error) {
return runCmdWithContextEnv(tc.context, command, envvars)
}

Expand All @@ -119,14 +125,14 @@ func (tc *TestContext) RunAdbWaitForDevice() error {
"adb",
"wait-for-device",
}
if _, _, err := tc.RunCmd(adbCommand...); err != nil {
if _, err := tc.RunCmd(adbCommand...); err != nil {
return fmt.Errorf("timed out waiting for Cuttlefish device to connect to adb: %w", err)
}
return nil
}

// Runs the given command with the existing envvars.
func (tc *TestContext) RunCmd(args ...string) (string, string, error) {
func (tc *TestContext) RunCmd(args ...string) (CommandOutput, error) {
command := []string{}
command = append(command, args...)
return tc.RunCmdWithEnv(command, map[string]string{})
Expand Down Expand Up @@ -176,7 +182,7 @@ func (tc *TestContext) CVDFetch(args FetchArgs) error {
if credentialArg != "" {
fetchCmd = append(fetchCmd, fmt.Sprintf("--credential_source=%s", credentialArg))
}
if _, _, err := tc.RunCmd(fetchCmd...); err != nil {
if _, err := tc.RunCmd(fetchCmd...); err != nil {
log.Printf("Failed to fetch: %w", err)
return err
}
Expand Down Expand Up @@ -204,7 +210,7 @@ func (tc *TestContext) CVDCreate(args CreateArgs) error {
if len(args.Args) > 0 {
createCmd = append(createCmd, args.Args...)
}
if _, _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
if _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
log.Printf("Failed to create instance(s): %w", err)
return err
}
Expand All @@ -220,7 +226,7 @@ func (tc *TestContext) CVDStop() error {
}

stopCmd := []string{tc.TargetBin(), "stop"}
if _, _, err := tc.RunCmdWithEnv(stopCmd, tempdirEnv); err != nil {
if _, err := tc.RunCmdWithEnv(stopCmd, tempdirEnv); err != nil {
log.Printf("Failed to stop instance(s): %w", err)
return err
}
Expand All @@ -240,7 +246,7 @@ func (tc *TestContext) LaunchCVD(args CreateArgs) error {
if len(args.Args) > 0 {
createCmd = append(createCmd, args.Args...)
}
if _, _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
if _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
log.Printf("Failed to create instance(s): %w", err)
return err
}
Expand All @@ -256,7 +262,7 @@ func (tc *TestContext) StopCVD() error {
}

stopCmd := []string{"bin/stop_cvd"}
if _, _, err := tc.RunCmdWithEnv(stopCmd, tempdirEnv); err != nil {
if _, err := tc.RunCmdWithEnv(stopCmd, tempdirEnv); err != nil {
log.Printf("Failed to stop instance(s): %w", err)
return err
}
Expand All @@ -271,7 +277,7 @@ func (tc *TestContext) CVDPowerwash() error {
}

createCmd := []string{tc.TargetBin(), "powerwash"}
if _, _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
if _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
log.Printf("Failed to powerwash instance(s): %w", err)
return err
}
Expand Down Expand Up @@ -306,7 +312,7 @@ func (tc *TestContext) CVDLoad(load LoadArgs) error {
if credentialArg != "" {
loadCmd = append(loadCmd, fmt.Sprintf("--credential_source=%s", credentialArg))
}
if _, _, err := tc.RunCmd(loadCmd...); err != nil {
if _, err := tc.RunCmd(loadCmd...); err != nil {
log.Printf("Failed to perform `cvd load`: %w", err)
return err
}
Expand All @@ -317,13 +323,13 @@ func (tc *TestContext) CVDLoad(load LoadArgs) error {
}

func (tc *TestContext) GetMetricsDir() (string, error) {
stdOut, _, err := tc.RunCmd("cvd", "fleet")
res, err := tc.RunCmd("cvd", "fleet")
if err != nil {
return "", fmt.Errorf("failed to run `cvd fleet`")
}

re := regexp.MustCompile(`"metrics_dir" : "(.*)",`)
matches := re.FindStringSubmatch(stdOut)
matches := re.FindStringSubmatch(res.Stdout)
if len(matches) != 2 {
return "", fmt.Errorf("failed to find metrics directory")
}
Expand All @@ -346,7 +352,7 @@ func (tc *TestContext) SetUp(t *testing.T) {
log.Printf("Initializing %s test...", tc.t.Name())

log.Printf("Cleaning up any pre-existing instances...")
if _, _, err := tc.RunCmd(tc.TargetBin(), "reset", "-y"); err != nil {
if _, err := tc.RunCmd(tc.TargetBin(), "reset", "-y"); err != nil {
log.Printf("Failed to cleanup any pre-existing instances: %w", err)
}
log.Printf("Finished cleaning up any pre-existing instances!")
Expand Down Expand Up @@ -428,7 +434,7 @@ func (tc *TestContext) TearDown() {
matches, err := filepath.Glob(path.Join(tc.tempdir, pattern))
if err == nil {
for _, file := range matches {
_, _, err := tc.RunCmd("cp", "--dereference", file, testoutdir)
_, err := tc.RunCmd("cp", "--dereference", file, testoutdir)
if err != nil {
log.Printf("failed to copy %s to %s: %w", file, testoutdir, err)
}
Expand All @@ -447,7 +453,7 @@ func (tc *TestContext) TearDown() {
err := os.MkdirAll(outinstancedir, os.ModePerm)
if err == nil {
logdir := path.Join(instancedir, "logs")
_, _, err := runCmdWithContextEnv(context.TODO(), []string{"cp", "-r", "--dereference", logdir, outinstancedir}, map[string]string{})
_, err := runCmdWithContextEnv(context.TODO(), []string{"cp", "-r", "--dereference", logdir, outinstancedir}, map[string]string{})
if err != nil {
log.Printf("failed to copy %s to %s: %w", logdir, outinstancedir, err)
}
Expand All @@ -463,7 +469,7 @@ func (tc *TestContext) TearDown() {
outmetricsdir := path.Join(testoutdir, "metrics_files")
err := os.MkdirAll(outmetricsdir, os.ModePerm)
if err == nil {
_, _, err := runCmdWithContextEnv(context.TODO(), []string{"cp", "-r", "--dereference", metricsdir, outmetricsdir}, map[string]string{})
_, err := runCmdWithContextEnv(context.TODO(), []string{"cp", "-r", "--dereference", metricsdir, outmetricsdir}, map[string]string{})
if err != nil {
log.Printf("failed to copy %s to %s: %w", metricsdir, outmetricsdir, err)
}
Expand Down Expand Up @@ -588,7 +594,7 @@ func RunXts(t *testing.T, cuttlefishArgs FetchAndCreateArgs, xtsArgs XtsArgs) {
"--log-level-display=INFO",
}
xtsCommand = append(xtsCommand, xtsArgs.XtsArgs...)
if _, _, err := tc.RunCmd(xtsCommand...); err != nil {
if _, err := tc.RunCmd(xtsCommand...); err != nil {
t.Fatalf("failed to fully run XTS: %w", err)
}
log.Printf("Finished running XTS!")
Expand Down
6 changes: 3 additions & 3 deletions e2etests/cvd/cvd_powerwash_tests/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,19 @@ func TestCvdPowerwash(t *testing.T) {
}

const tmpFile = "/data/local/tmp/foo"
if _, _, err := c.RunCmd("adb", "shell", "touch", tmpFile); err != nil {
if _, err := c.RunCmd("adb", "shell", "touch", tmpFile); err != nil {
t.Fatalf("failed to create %s: %w", tmpFile, err)
}

if _, _, err := c.RunCmd("adb", "shell", "stat", tmpFile); err != nil {
if _, err := c.RunCmd("adb", "shell", "stat", tmpFile); err != nil {
t.Fatal("failed to verify %s created: %w", err)
}

if err := c.CVDPowerwash(); err != nil {
t.Fatal(err)
}

if _, _, err := c.RunCmd("adb", "shell", "stat", tmpFile); err == nil {
if _, err := c.RunCmd("adb", "shell", "stat", tmpFile); err == nil {
t.Fatal("failed to powerwash, %s still exists")
}
})
Expand Down
4 changes: 2 additions & 2 deletions e2etests/cvd/display_tests/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func addDisplay(c e2etests.TestContext, t *testing.T) {
t.Fatal(err)
}

if _, _, err := c.RunCmd(c.TargetBin(), "display", "add", "--display=width=500,height=500"); err != nil {
if _, err := c.RunCmd(c.TargetBin(), "display", "add", "--display=width=500,height=500"); err != nil {
t.Fatal(err)
}
}
Expand All @@ -55,7 +55,7 @@ func listDisplays(c e2etests.TestContext, t *testing.T) {
t.Fatal(err)
}

if _, _, err := c.RunCmd(c.TargetBin(), "display", "list"); err != nil {
if _, err := c.RunCmd(c.TargetBin(), "display", "list"); err != nil {
t.Fatal(err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion e2etests/cvd/env_tests/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestListEnvServices(t *testing.T) {
t.Fatal(err)
}

if _, _, err := c.RunCmd(c.TargetBin(), "env", "ls"); err != nil {
if _, err := c.RunCmd(c.TargetBin(), "env", "ls"); err != nil {
t.Fatal(err)
}
}
4 changes: 2 additions & 2 deletions e2etests/cvd/graphics_detector_tests/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ func TestLaunchingWithAutoEnablesGfxstream(t *testing.T) {
t.Fatalf("failed to wait for Cuttlefish device to connect to adb: %w", err)
}

output, _, err := c.RunCmd("adb", "shell", "getprop", "ro.hardware.egl")
res, err := c.RunCmd("adb", "shell", "getprop", "ro.hardware.egl")
if err != nil {
t.Fatalf("failed to get EGL sysprop: %w", err)
}
output = strings.TrimSpace(output)
output := strings.TrimSpace(res.Stdout)
if output != "emulation" {
t.Errorf(`"ro.hardware.egl" was "%s"; expected "emulation"`, output)
}
Expand Down
10 changes: 6 additions & 4 deletions e2etests/cvd/logs_tests/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,11 @@ func TestPrintLogs(t *testing.T) {
t.Fatal(err)
}

stdOut, _, err := c.RunCmd(c.TargetBin(), "logs")
res, err := c.RunCmd(c.TargetBin(), "logs")
if err != nil {
t.Fatal(err)
}
stdOut := res.Stdout

listedNames := []string{}
lines := strings.Split(stdOut, "\n")
Expand All @@ -61,9 +62,9 @@ func TestPrintLogs(t *testing.T) {
if len(fields) != 2 {
t.Errorf("line %q does not match format '<NAME> <PATH>'", line)
}
_, stderr, err := c.RunCmd("stat", fields[1])
res, err := c.RunCmd("stat", fields[1])
if err != nil {
t.Fatal(stderr)
t.Fatal(res.Stderr)
}
listedNames = append(listedNames, fields[0])
}
Expand All @@ -74,10 +75,11 @@ func TestPrintLogs(t *testing.T) {
}
}

stdOut, _, err = c.RunCmd(c.TargetBin(), "logs", "--print", "launcher.log")
res, err = c.RunCmd(c.TargetBin(), "logs", "--print", "launcher.log")
if err != nil {
t.Fatal(err)
}
stdOut = res.Stdout
if len(stdOut) == 0 {
t.Fatalf("empty launcher.log")
}
Expand Down
4 changes: 2 additions & 2 deletions e2etests/cvd/media_tests/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ func TestEmulatedCameraV4l2Compliance(t *testing.T) {
t.Fatalf("failed to wait for Cuttlefish device to connect to adb: %w", err)
}

if _, _, err := c.RunCmd("adb", "shell", "su", "0", "v4l2-ctl", "--list-devices"); err != nil {
if _, err := c.RunCmd("adb", "shell", "su", "0", "v4l2-ctl", "--list-devices"); err != nil {
t.Fatalf("v4l2-ctl --list-devices failed: %w", err)
}

if _, _, err := c.RunCmd("adb", "shell", "su", "0", "v4l2-compliance", "-d1", "-s"); err != nil {
if _, err := c.RunCmd("adb", "shell", "su", "0", "v4l2-compliance", "-d1", "-s"); err != nil {
t.Fatalf("v4l2-compliance failed: %w", err)
}
})
Expand Down
4 changes: 2 additions & 2 deletions e2etests/cvd/metrics_tests/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ func TestMetrics(t *testing.T) {

var metricsdir string
err := func() error {
output, _, err := c.RunCmd(c.TargetBin(), "fleet")
res, err := c.RunCmd(c.TargetBin(), "fleet")
if err != nil {
return fmt.Errorf("failed to run `cvd fleet`")
}

re := regexp.MustCompile(`"metrics_dir" : "(.*)",`)
matches := re.FindStringSubmatch(output)
matches := re.FindStringSubmatch(res.Stdout)
if len(matches) != 2 {
return fmt.Errorf("failed to find metrics directory.")
}
Expand Down
Loading