test: add browser agent integration tests#21151
test: add browser agent integration tests#21151gsquared94 merged 13 commits intogoogle-gemini:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the testing suite by introducing comprehensive end-to-end integration tests for the browser agent. These tests validate the entire workflow from user prompt to browser interaction via Chrome DevTools, ensuring robust functionality beyond what mocked unit tests can provide. The addition of these tests improves the reliability and stability of the browser agent feature by verifying its full operational flow. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable set of integration tests for the browser agent, verifying the end-to-end flow, which is a great addition for ensuring reliability. No security vulnerabilities were found in this change. Feedback includes a suggestion to make the Chrome availability check on macOS more robust to improve the developer experience when running these tests.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable set of integration tests for the browser agent, verifying the end-to-end flow which is a great addition for ensuring reliability. The tests are well-structured, using a TestRig for setup and teardown, and correctly skip execution if Chrome is not available. I have one suggestion to improve the cleanup test by removing an unnecessary delay, which will make the test suite faster without compromising its effectiveness.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable set of end-to-end integration tests for the browser agent, verifying the complete flow from prompt to browser interaction. The tests are well-structured, using a test rig and fake responses for deterministic execution. The inclusion of a check to skip tests gracefully on environments without Chrome is a good practice. My review includes one suggestion to enhance the Chrome availability check to include support for Windows, which would improve cross-platform test coverage.
|
/gemini review |
There was a problem hiding this comment.
Code Review
The original review comments proposed using statSync for Chrome detection on Windows. However, this directly contradicts the repository rule that mandates the use of asynchronous file system operations (e.g., fs.promises.stat) to avoid blocking the event loop. As such, both comments have been dropped to adhere to the established guidelines.
|
hii @jacob314, @gsquared94 can you please take some time to review this PR and let me know if any changes are required? |
|
Hey, thanks for these tests—they're a solid addition. Some issues:
Could you enable Actions on your fork and run the full E2E suite there and share a link to a successful run to help verify these changes. |
- Fix Windows npx bug: use npx.cmd on win32 in browserManager.ts - Fix Chrome discovery: check C:\Program Files paths with fs.existsSync - Fix Node 20.0 compat: replace import.meta.dirname with fileURLToPath - Add Chrome availability step to e2e_windows CI job - Update browserManager unit tests for platform-conditional npx
This reverts commit 545b786.
@gsquared94 thank you for the feedback |
Head branch was pushed to by a user without write access
|
@gsquared94 really sorry, there was formatting issue due to which merge was blocked, have tested it now locally, could you please enable the merge |
Summary
Adds end-to-end integration tests for the browser agent to verify the full flow (prompt → browser_agent delegation → MCP → Chrome DevTools → result) that isn't covered by existing mocked unit tests.
Details
describe.skipIf) on environments without Chrome/Chromium.e2e_linuxjob inchained_e2e.yml.Related Issues
Closes #21120
How to Validate
Pre-Merge Checklist