Conversation
Made-with: Cursor
|
Firetiger deploy monitoring skipped This PR didn't match the auto-monitor filter configured on your GitHub connection:
Reason: PR only modifies browser e2e testing setup and dependencies, with no changes to API endpoints or Temporal workflows. To monitor this PR anyway, reply with |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue. You can view the agent here.
Reviewed by Cursor Bugbot for commit fa5ed61. Configure here.
| playwrightDepsErr = fmt.Errorf("failed to install playwright dependencies: %w\noutput: %s", err, string(output)) | ||
| return | ||
| } | ||
| t.Log("Playwright dependencies installed successfully") |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit fa5ed61. Configure here.
ulziibay-kernel
left a comment
ulziibay-kernel
left a comment
There was a problem hiding this comment.
was this issue observed and reported somewherE?
2 participants
Summary
node_modulesexists buttsxis missing/home/kernel/user-data, avoiding live profile writes during zip/zstd creationTest plan
PATH="/usr/local/go/bin:$PATH" go test -v -race ./e2e/ -run 'TestZipTransferTiming|TestZstdTransferTiming|TestZipVsZstdComparison' -count=1PATH="/usr/local/go/bin:$PATH" go test -v -race $(go list ./... | rg -v '/e2e$')Made with Cursor
Note
Low Risk
Low risk: changes are limited to e2e/benchmark test setup, mainly improving dependency installation checks and reducing flakiness from live Chromium profile writes.
Overview
Improves e2e Chromium test reliability by re-running
pnpm installwhen Playwright’snode_modulesexists but thetsxCLI is missing, with clearer install logging and early error return.Stabilizes the zip/zstd transfer benchmark tests by explicitly stopping
chromiumviasupervisorctl(and briefly sleeping) before reading/archiving/home/kernel/user-data, avoiding races with in-flight profile writes during compression.Reviewed by Cursor Bugbot for commit fa5ed61. Bugbot is set up for automated code reviews on this repo. Configure here.