Skip to content

Stabilize browser e2e setup #233

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
rgarcia merged 1 commit into from
Apr 30, 2026
+36 −0
Merged

Stabilize browser e2e setup #233

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
14 changes: 14 additions & 0 deletions server/e2e/e2e_chromium_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,27 @@ func ensurePlaywrightDeps(t *testing.T) {

playwrightDepsOnce.Do(func() {
nodeModulesPath := getPlaywrightPath() + "/node_modules"
tsxPath := getPlaywrightPath() + "/node_modules/tsx/dist/cli.mjs"
if _, err := os.Stat(nodeModulesPath); os.IsNotExist(err) {
t.Log("Installing playwright dependencies...")
cmd := exec.Command("pnpm", "install")
cmd.Dir = getPlaywrightPath()
output, err := cmd.CombinedOutput()
if err != nil {
playwrightDepsErr = fmt.Errorf("failed to install playwright dependencies: %w\noutput: %s", err, string(output))
return
}
t.Log("Playwright dependencies installed successfully")
} else if _, err := os.Stat(tsxPath); os.IsNotExist(err) {
t.Log("Installing playwright dependencies...")
cmd := exec.Command("pnpm", "install")
cmd.Dir = getPlaywrightPath()
output, err := cmd.CombinedOutput()
if err != nil {
playwrightDepsErr = fmt.Errorf("failed to install playwright dependencies: %w\noutput: %s", err, string(output))
return
}
t.Log("Playwright dependencies installed successfully")
Copy link
Copy Markdown

@cursor cursor Bot Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated install logic in two conditional branches

Low Severity

The if branch (node_modules missing) and the else if branch (tsx missing) contain identical pnpm install logic — same command, same error handling, same log messages. If install behavior ever needs updating, both branches must be changed in lockstep, risking divergence. A single needsInstall boolean computed from both os.Stat checks, followed by one install block, would eliminate the duplication.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit fa5ed61. Configure here.

}
})

Expand Down
22 changes: 22 additions & 0 deletions server/e2e/e2e_zip_transfer_bench_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func TestZipTransferTiming(t *testing.T) {
err = populateUserData(ctx, client)
require.NoError(t, err, "failed to populate user-data")
t.Logf("User-data population took %dms", time.Since(populateStart).Milliseconds())
stopChromiumForStableUserData(t, ctx, client)

// Get initial directory size for reference
dirSize, fileCount, err := getDirStats(ctx, client, "/home/kernel/user-data")
Expand Down Expand Up @@ -139,6 +140,25 @@ func populateUserData(ctx context.Context, client *instanceoapi.ClientWithRespon
return nil
}

func stopChromiumForStableUserData(t *testing.T, ctx context.Context, client *instanceoapi.ClientWithResponses) {
t.Helper()

args := []string{"-c", "/etc/supervisor/supervisord.conf", "stop", "chromium"}
req := instanceoapi.ProcessExecJSONRequestBody{
Command: "supervisorctl",
Args: &args,
}
rsp, err := client.ProcessExecWithResponse(ctx, req)
require.NoError(t, err, "failed to stop chromium before reading user-data")
require.Equal(t, http.StatusOK, rsp.StatusCode(), "unexpected status stopping chromium: %s body=%s", rsp.Status(), string(rsp.Body))
if rsp.JSON200 != nil && rsp.JSON200.ExitCode != nil {
require.Equal(t, 0, *rsp.JSON200.ExitCode, "supervisorctl stop chromium failed")
}

// Give Chromium's profile databases a brief moment to settle after shutdown.
time.Sleep(500 * time.Millisecond)
}

// getDirStats returns approximate size and file count of a directory
func getDirStats(ctx context.Context, client *instanceoapi.ClientWithResponses, path string) (int64, int, error) {
// Use du command via process exec to get accurate size
Expand Down Expand Up @@ -286,6 +306,7 @@ func TestZstdTransferTiming(t *testing.T) {
err = populateUserData(ctx, client)
require.NoError(t, err, "failed to populate user-data")
t.Logf("User-data population took %dms", time.Since(populateStart).Milliseconds())
stopChromiumForStableUserData(t, ctx, client)

// Get directory stats for reference
dirSize, fileCount, err := getDirStats(ctx, client, "/home/kernel/user-data")
Expand Down Expand Up @@ -436,6 +457,7 @@ func TestZipVsZstdComparison(t *testing.T) {
t.Logf("Populating user-data by browsing...")
err = populateUserData(ctx, client)
require.NoError(t, err, "failed to populate user-data")
stopChromiumForStableUserData(t, ctx, client)

// Get directory stats
dirSize, fileCount, err := getDirStats(ctx, client, "/home/kernel/user-data")
Expand Down
Loading