native press key, scroll, drag mouse #78
Conversation
Mesa DescriptionNote Adds new input endpoints (press_key, scroll, drag_mouse), serializes input actions, updates OpenAPI/client, installs xdotool, and expands/refactors e2e tests with dependency bumps.
Written by Cursor Bugbot for commit 2b85972. This will update automatically on new commits. Configure here. Description generated by Mesa. Update settings |
![mesa-dot-dev[bot]](https://avatars.githubusercontent.com/in/1050077?s=60&v=4){kind=small}
There was a problem hiding this comment.
Performed full review of 0260b79...cce4f45
Analysis
-
The PR adds a dependency on xdotool for input automation, which may create platform compatibility issues or increase the Docker image size.
-
While smoke tests are included, there's no mention of comprehensive unit tests or integration tests for these critical input simulation functions, potentially allowing edge cases to slip through.
-
The PR refactors E2E tests by removing "flaky" tests, which might inadvertently reduce test coverage of important scenarios.
-
The implementation of sophisticated features like configurable paths and delays for DragMouse could introduce performance overhead or timing-related issues in different environments.
Tip
⚡ Quick Actions
This review was generated by Mesa.
Actions:
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
7 files reviewed | 0 comments | Review on Mesa | Edit Reviewer Settings
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| } | ||
| } | ||
|
|
||
| func TestChromiumHeadfulUserDataSaving(t *testing.T) { |
There was a problem hiding this comment.
these flaky tests are not needed?
There was a problem hiding this comment.
yeah getting rid of them since it tests a fairly complicated way of restoring user data that we don't actually use
|
ahh good catch. Added mutex: 2b85972 |
| _ = dumpContainerDiagnostics(ctx, name) | ||
| t.Fatalf("api not ready: %v", err) | ||
| } | ||
| require.NoError(t, waitHTTPOrExit(ctx, apiBaseURL+"/spec.yaml", exitCh), "api not ready: %v", err) |
There was a problem hiding this comment.
Bug: Misleading Error Messages in require.NoError Calls
Several require.NoError calls, particularly those checking waitHTTPOrExit, use an err variable from an earlier scope in their failure message. This can lead to misleading error output, as the message might report a previous error (e.g., docker lookup) instead of the actual waitHTTPOrExit failure. This occurs on lines 114, 214, 308, 359, and 404.
Note
Adds native input APIs (press key, scroll, drag mouse), serializes input ops, updates OpenAPI/client/server stubs, installs xdotool, and refines tests/deps.
POST /computer/press_key,POST /computer/scroll,POST /computer/drag_mousewith bounds/validation and xdotool-backed behavior.inputMuto serialize mouse/keyboard/screenshot actions.ClickMouseRequestButton*enum names.PressKeyRequest,ScrollRequest,DragMouseRequestand wire routes into generated client/server.xdotoolin chromium-headless image.requirein e2e; minor test cleanups.github.com/samber/lo; bumpstretchr/testify; updatex/sync, addx/text.Written by Cursor Bugbot for commit b12249d. This will update automatically on new commits. Configure here.