-
Notifications
You must be signed in to change notification settings - Fork 608
[Vertex AI] Remove golden-files
directory
#6740
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
Conversation
This commit removes the directory from . The tool does not delete directories recursively, so the files within the directory needed to be deleted first. To do this, I first listed all files in the repository and identified the files that were in the directory. I then called the tool on each file individually to delete all of them, before attempting to delete the now-empty directory itself. Finally, I confirmed the directory was deleted by listing the files in the repo again.
📝 PRs merging into main branchOur main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released. |
Coverage Report 1Affected ProductsNo changes between base commit (c9287ee) and merge commit (686dfa9).Test Logs |
Test Results 22 files - 16 22 suites - 16 12s ⏱️ -42s Results for commit 6e12da2. ± Comparison against base commit c9287ee. This pull request removes 95 and adds 119 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
Size Report 1Affected ProductsNo changes between base commit (c9287ee) and merge commit (686dfa9).Test Logs |
cc: @rlazo It appears these are still used in the |
tracking b/399116207 --------- Co-authored-by: Daymon <[email protected]>
The code was trying to find tests golden files in the incorrect directory
The original PR didn't remove all files
- Update which files contain the right test data - Remove unnecessary tests inhereted from genai-common
Generated by 🚫 Danger |
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.
LGTM, although left some comments on some removals I wanna confirm.
Also, I just wanted to note that #no-changelog
isn't a thing (afaik?), you just add the no-changelog
label instead.
firebase-vertexai/src/test/java/com/google/firebase/vertexai/common/StreamingSnapshotTests.kt
Show resolved
Hide resolved
firebase-vertexai/src/test/java/com/google/firebase/vertexai/common/UnarySnapshotTests.kt
Show resolved
Hide resolved
Yes, those removals are intentional (see description).
Those were for cases in which vertex and Gen AI differed, which was a thing
in common but not in this sdk
…On Tue, Mar 11, 2025, 1:38 p.m. Daymon ***@***.***> wrote:
***@***.**** approved this pull request.
LGTM, although left some comments on some removals I wanna confirm.
Also, I just wanted to note that #no-changelog isn't a thing (afaik?),
you just add the no-changelog label instead.
------------------------------
In
firebase-vertexai/src/test/java/com/google/firebase/vertexai/common/StreamingSnapshotTests.kt
<#6740 (comment)>
:
> - @test
- fun `citation returns correctly when using alternative name`() =
- goldenStreamingFile("success-citations-altname.txt") {
- val responses = apiController.generateContentStream(textGenerateContentRequest("prompt"))
-
- withTimeout(testTimeout) {
- val responseList = responses.toList()
- responseList.any {
- it.candidates?.any { it.citationMetadata?.citationSources?.isNotEmpty() ?: false }
- ?: false
- } shouldBe true
- }
- }
-
Is this intentional?
------------------------------
In
firebase-vertexai/src/test/java/com/google/firebase/vertexai/common/UnarySnapshotTests.kt
<#6740 (comment)>
:
> - @test
- fun `citation returns correctly when using alternative name`() =
- goldenUnaryFile("success-citations-altname.json") {
- withTimeout(testTimeout) {
- val response = apiController.generateContent(textGenerateContentRequest("prompt"))
-
- response.candidates?.isEmpty() shouldBe false
- response.candidates?.first()?.citationMetadata?.citationSources?.isNotEmpty() shouldBe true
- }
- }
-
Is this intentional?
—
Reply to this email directly, view it on GitHub
<#6740 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACZ7QRJUZKMLHBKGGTPK2T2T4NPVAVCNFSM6AAAAABYNEQ44OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMNZVGUZTSNJQGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Remove the `firebase-vertexai/src/test/resources/golden-files` directory. This was carried over from the [`generative-ai-android`](https://github.com/google-gemini/generative-ai-android) repository. We are now using https://github.com/FirebaseExtended/vertexai-sdk-test-data/tree/main/mock-responses instead. #no-changelog --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Rodrigo Lazo <[email protected]> Co-authored-by: Daymon <[email protected]> Co-authored-by: Matthew Robertson <[email protected]> Co-authored-by: Rodrigo Lazo Paz <[email protected]>
Remove the
firebase-vertexai/src/test/resources/golden-files
directory. This was carried over from thegenerative-ai-android
repository. We are now using https://github.com/FirebaseExtended/vertexai-sdk-test-data/tree/main/mock-responses instead.#no-changelog