infra(ai): Add script to run integration tests locally #15398
Conversation
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. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR introduces two new shell scripts for running AI integration tests locally. The scripts are well-structured and documented. I've found a few areas for improvement regarding script robustness and best practices, such as adding set -e where appropriate, checking exit codes, and quoting variables to handle potential edge cases with file paths. These changes will make the scripts more reliable and prevent confusing failures.
There was a problem hiding this comment.
@ncooke3 Do we have a particular directory structure we should adhere to for the scripts directory? I'm wondering if we should do scripts/firebaseai and also move https://github.com/firebase/firebase-ios-sdk/blob/dcba4944aa9650110f636cfc00adabc21139b045/scripts/update_vertexai_responses.sh into there.
There was a problem hiding this comment.
I think I would personally find the decrypt_secrets.sh script most useful locally since it automates most of the project setup. I'm not certain that I would personally use run.sh since I typically need to edit, run, edit, etc., in Xcode when working on the integration tests locally.
My only concern with checking in the script is that it could easily get out of date unless it's part of our CI job for running the integration tests.
There was a problem hiding this comment.
I think I would personally find the decrypt_secrets.sh script most useful locally since it automates most of the project setup. I'm not certain that I would personally use run.sh since I typically need to edit, run, edit, etc., in Xcode when working on the integration tests locally.
I agree. run.sh is more-so useful if you just wanna test run integrations tests locally before waiting on CI to provision a mac (or creating a PR in general). If you're iterating on the integration tests themselves, then opening the project in XCode would be more straightforward.
My only concern with checking in the script is that it could easily get out of date unless it's part of our CI job for running the integration tests.
I've updated the PR to have the CI job use it as well, I just wasn't sure if that's what we wanted to do. But that sound s like a good idea to ensure it stays up-to-date.
There was a problem hiding this comment.
This is a nice step forward for usability. Thanks!
A future project could be to generalize this for all products - with each product providing a list of encrypted files to the script.
Then to address Andrew's point, the same core script should be used by both CI and the local run script.
I like that Idea. Should I go ahead and do that, or would we want that in a follow up PR? |
3 participants
This PR adds a script (
scripts/tests/ai/run.sh) which can be used locally to run the integration tests for Firebase AI.The script does the following:
You can explicitly provide an Xcode version and a platform target to the script, or let it automatically determine the arguments to use.
The script includes documentation, proper error handling, and debug logging.
#no-changelog