test: add VertexAI integration tests #8853
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Report 1Affected ProductsNo changes between base commit (5718838) and merge commit (6c1cabe).Test Logs |
hsubox76
reviewed
Mar 19, 2025
hsubox76
reviewed
Mar 19, 2025
|
|
||
| # Vertex AI integration test Firebase project config | ||
| packages/vertexai/integration/firebase-config.ts |
There was a problem hiding this comment.
Maybe just include this file with an empty object exported so that the IDE doesn't get mad, and if you run it locally, you overwrite it, and if you run it in CI, the workflow overwrites it. Maybe we should do this in e2e/ too.
There was a problem hiding this comment.
That's a good idea, done
There was a problem hiding this comment.
@hsubox76 I did this, but it turns out that if it's tracked in git, changes will be tracked even if it's in the .gitignore. this means if someone updates this with the testing config, they may accidentally commit the change and upload it to git.
I believe we can't track this file with git while ignoring changes. I think we should just remove the file for now.
hsubox76
reviewed
Mar 19, 2025
| @@ -39,6 +39,7 @@ | |||
| "test:ci": "yarn testsetup && node ../../scripts/run_tests_in_ci.js -s test", | |||
| "test:skip-clone": "karma start", | |||
| "test:browser": "yarn testsetup && karma start", | |||
| "test:integration": "karma start --integration", | |||
There was a problem hiding this comment.
You'll have to call this from "test" (line 38) if you want to run it on PRs or do you not want to do that yet?
There was a problem hiding this comment.
I would prefer not to add this to test since then it'll be ran when someone runs test in the root directory- which is going to fail if someone hasn't defined the config that's used in these tests. Could be annoying for people working on other SDKs
Size Analysis Report 1Affected Products
Test Logs |
dlarocque
force-pushed
the
dl/vertex-integration
branch
from
346de4e to
7ad7833
Compare
DellaBitta
approved these changes
Mar 24, 2025
dlarocque
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add integration tests using a secret config defined in
integration/firebase-config.ts. I'm not using JSCore sandbox here because in the future we'll want to run these integration tests using a Firebase project that has access to APIs that aren't pubilc yet (which is also why the config is 'secret').When running tests locally, we can manually create a file that exports the firebase config. When running tests in CI, we'll have to add a step to write the config from a GH secret to a file using something like
echo "export const FIREBASE_CONFIG = '$(echo "$FIREBASE_CONFIG" | jq -r .)'" > packages/vertexai/integration/firebase-config.ts.The next step is to get this running in CI on a daily CRON schedule, and in PRs that make changes to vertexAI.