-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add app testing agent tools and prompts #9409
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.
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
base: master
Are you sure you want to change the base?
Conversation
* Scaffolding for new appdistribution MCP tool. * Refactor business logic out of the appdistribution CLI so that it can be used by an MCP tool. * Wire new appdistribution tool up to the business logic. * Fix linting errors. * Update src/appdistribution/distribution.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: Jamie Rothfeder <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Rename appdistribution directory to apptesting * Make variables consistent with directory rename. --------- Co-authored-by: Jamie Rothfeder <[email protected]>
…9280) * Use a datastructure to represent test devices rather than a string. * Update src/mcp/tools/apptesting/tests.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update src/mcp/tools/apptesting/tests.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Pretty --------- Co-authored-by: Jamie Rothfeder <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Add initial MCP prompt for running automated tests * Fix typos
* Create a on-off test and execute. * Can now create a on-off test. * Update src/appdistribution/client.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update src/mcp/tools/apptesting/tests.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * PR feedback * Separate test check in to a different tool so that gemini can orchestrate running and checking for completion. * Set the devices field to optional --------- Co-authored-by: Jamie Rothfeder <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
… description (#9322) * Use the same default device that's used in the Console * Update prompt to support generating a test case when there is no test description passed
* Add custom auto-enablement for app testing * Address gemini code assist comments * Fix intersection bug * Fix issues with test
Summary of ChangesHello @tagboola, 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 introduces comprehensive support for the Firebase App Testing agent within the Firebase CLI, specifically for AI-guided mobile app testing. It adds new client-side capabilities to define and execute AI-driven test cases, along with corresponding prompts and tools integrated into the Managed Code Platform (MCP). This feature, enabled under an experiment flag, allows developers to define detailed test steps with goals, hints, and success criteria, and then run these tests on specified devices, with the CLI providing status updates and links to results. Highlights
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces the App Testing agent tools and prompts, enabling automated mobile app testing within the Firebase ecosystem. The changes include new commands, types, and utilities to facilitate test execution and result analysis. The review focuses on correctness and maintainability, with an emphasis on error handling and code organization.
| aiInstructions: aiInstruction, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The aiInstructions parameter is being passed directly from the request to the backend. Ensure that this data is properly validated and sanitized to prevent potential injection attacks or unexpected behavior on the backend. Consider adding validation logic to the client to enforce a specific structure or allowed values for the AI instructions.
| Only include specific details in the test case if you are certain they will be available to the agent, otherwise the | ||
| agent will likely fail if it tries to follow specific guidance that doesn't work (e.g. click the 'Play' button but the | ||
| button isn't visible to the app testing agent). Do not include Android resource ids in the test case. Include |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text mentions that the agent will likely fail if it tries to follow specific guidance that doesn't work. It would be better to proactively validate the availability of the elements or actions before including them in the test case. This could involve using the get_devices tool to query the available devices and their capabilities, and then tailoring the test case accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some questions - this is looking awesome!
| ); | ||
| } | ||
| } | ||
| const releaseTests = await Promise.all(releaseTestPromises); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: It looks like this'll make a request in parallel for each promise. Could this cause problems if you have say 100 requests in flight at once?
There are some good libraries for this, eg https://www.npmjs.com/package/p-limit. It looks like src/database/import.ts already uses it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might, however, it's also unchanged for the original code. The change here was to refactor this code so that it could be used by both the command line tool and the MCP server.
Maybe we can create an issue to track separately?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do it
| this is not an Android app, instruct the user that this command can't be used with this app. | ||
| 2. **Make sure the user is logged in. No App Testing tools will work if the user is not logged in.** | ||
| a. Use the \`firebase_get_environment\` tool to verify that the user is logged in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not blocking: I think Joe is working on a way to unite how we do the "setup" part of prompts. I think an iteration of "sign in, get / create the user's project and app" is used in Crashlytics and other prompts
| Here are a list of prerequisite steps that must be completed before running a test. | ||
| 1. **Make sure this is an Android app**. The App Testing agent only works with Android apps. If | ||
| this is not an Android app, instruct the user that this command can't be used with this app. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small thing / not blocking at all: You don't need to include newlines, which might make future edits easier for you all
| export const get_devices = tool( | ||
| "apptesting", | ||
| { | ||
| name: "get_devices", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a use case for this in the run_test prompt?
Separately, I'm wondering if there should be a combined apptesting_get_status tool that shows available devices and the current in-progress tests, or if this tool should be part of the CLI params instead of an MCP tool. Thinking about this in the spirit of reducing the number of MCP tools
| export const check_test = tool( | ||
| "apptesting", | ||
| { | ||
| name: "check_test", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar question here to get_devices - should the prompt be advising on using this tool to check the status of tests?
| description: | ||
| "Get available devices that can be used for automated tests using the app testing agent", | ||
| inputSchema: z.object({ | ||
| type: z.enum(["ANDROID"]).describe("The type of device"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be platform, or is type more idiomatic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we omit this at this point? We're not supporting anything else here yet are we?
Description
Add prompt and corresponding tools for running mobile automated tests with the Firebase App Testing agent behind the
mcalphaexperiment.Scenarios Tested
Tested locally with the Gemini CLI.
Sample Commands