VertexAI: update testing to clone mock responses from shared repo

[tanzimfh]

This removes mock responses from this repo and updates the test setup process to clone them from the new shared repo.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 033f089

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions]

Changeset File Check ✅

• No modified packages are missing from the changeset file.
• No changeset formatting errors detected.

[google-oss-bot]

Size Report ¹

Affected Products

No changes between base commit (e7260e2) and merge commit (4f1565a).

Test Logs

• Base (e7260e2): https://github.com/firebase/firebase-js-sdk/actions/runs/9684655387
• Merge (4f1565a): https://github.com/firebase/firebase-js-sdk/actions/runs/9688342316

1. https://storage.googleapis.com/firebase-sdk-metric-reports/7XsSffFN4H.html

[google-oss-bot]

Size Analysis Report ¹

Affected Products

No changes between base commit (e7260e2) and merge commit (4f1565a).

Test Logs

• Base (e7260e2): https://github.com/firebase/firebase-js-sdk/actions/runs/9684655387
• Merge (4f1565a): https://github.com/firebase/firebase-js-sdk/actions/runs/9688342316

1. https://storage.googleapis.com/firebase-sdk-metric-reports/Gf8qSw99m9.html

[dlarocque]

Nice!

We talked about this earlier, but I just want to confirm: When a change to the expected responses are made, those changes will stay in a separate branch until the changes in the backend go live?

So, the process would be:

1. Breaking changes to the response format are proposed/staged, and the new responses are checked into a branch (e.g. staging) in the golden files repo.
2. When we get around to fixing how we handle the new responses, we would add git checkout staging to the testsetup step to test against the new version of the responses.
3. When the changes to the responses go live, staging is merged into master, and we make another PR, removing git checkout staging from testsetup.

If this is the case, or there is another process, is that documented somewhere?

[tanzimfh]

@dlarocque That's just an idea I had, it's not documented anywhere.

That process is a bit cumbersome, but I don't think it should be an issue if breaking changes don't come too often. Keep in mind that non-breaking changes can be merged directly to main, and the golden files repo's CI will run the SDKs' unit tests for Vertex to prevent accidental breakages (see this PR that runs the iOS tests).

@hsubox76 @andrewheard @rlazo @davidmotson Please share any thoughts. I can document this in the golden files repo when everyone agrees on a solution 🤠

[rlazo]

Nice!

We talked about this earlier, but I just want to confirm: When a change to the expected responses are made, those changes will stay in a separate branch until the changes in the backend go live?

So, the process would be:

1. Breaking changes to the response format are proposed/staged, and the new responses are checked into a branch (e.g. staging) in the golden files repo.
2. When we get around to fixing how we handle the new responses, we would add git checkout staging to the testsetup step to test against the new version of the responses.
3. When the changes to the responses go live, staging is merged into master, and we make another PR, removing git checkout staging from testsetup.

If this is the case, or there is another process, is that documented somewhere?

Hey Daniel, would this process work for you?

1. Breaking changes to the response are checked into the main branch of the golden files repo.
2. This will, potentially, break your code. Then you'll keep having failures until the code is fixed.
   2.1 If you need the code to pass tests now, because you are working on something else, you can change your code to checkout the commit before the breaking change.
   2.2 Once your code is fixed, you can start checking out main HEAD again

The main advantage of this over the previous one is that moving things around are done in only one repo (platform) rather than 2 (platform & golden files). This makes maintainance of the golden files easier as adding new tests is a single step process.

WDYT?

[dlarocque]

That process is a bit cumbersome, but I don't think it should be an issue if breaking changes don't come too often.

Agreed!

Keep in mind that non-breaking changes can be merged directly to main, and the golden files repo's CI will run the SDKs' unit tests for Vertex to prevent accidental breakages (see this PR that runs the iOS tests).

Oh I didn't see this- nice!

[dlarocque]

Hey Daniel, would this process work for you?

1. Breaking changes to the response are checked into the main branch of the golden files repo.
2. This will, potentially, break your code. Then you'll keep having failures until the code is fixed.
   2.1 If you need the code to pass tests now, because you are working on something else, you can change your code to checkout the commit before the breaking change.
   2.2 Once your code is fixed, you can start checking out main HEAD again

@rlazo That sounds good to me! Would the breaking changes only be checked into main once they go live in the backend?

[rlazo]

Ideally they would go live asap, because we can then use the tests to develop the code earlier

…

On Thu, Jun 27, 2024, 11:57 a.m. Daniel La Rocque ***@***.***> wrote: Hey Daniel, would this process work for you? 1. Breaking changes to the response are checked into the main branch of the golden files repo. 2. This will, potentially, break your code. Then you'll keep having failures until the code is fixed. 2.1 If you need the code to pass tests *now*, because you are working on something else, you can change your code to checkout the commit before the breaking change. 2.2 Once your code is fixed, you can start checking out main HEAD again That sounds good to me! Would the breaking changes only be checked into main once they go live in the backend? — Reply to this email directly, view it on GitHub <#8333 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACZ7QSHFAXW7OWVTSDI3WLZJQY7JAVCNFSM6AAAAABJYZPM7KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJVGA4DSNBQGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[andrewheard]

1. Breaking changes to the response are checked into the main branch of the golden files repo.
2. This will, potentially, break your code. Then you'll keep having failures until the code is fixed.
   2.1 If you need the code to pass tests now, because you are working on something else, you can change your code to checkout the commit before the breaking change.
   2.2 Once your code is fixed, you can start checking out main HEAD again

This sounds reasonable to me as well on the Swift side. I think we'll just need to make sure it's well documented in the workflows / scripts so the rest of our team members can easily spot the issue and temporarily work around it (or know to ignore CI failures).

If breakages become too frequent, one other possible process I can think of:

1. The scripts in each repo could checkout a specific commit (or semver tag) from vertexai-sdk-test-data.
2. CI workflows in each repo could add a warning comment in PRs if not using the latest commit / tag from vertexai-sdk-test-data (to suggest that you update).

cc: @paulb777 in case you have ideas or opinions on processes.

[dlarocque]

I do feel a bit uneasy about CI being guaranteed to go red any time there is a breaking change to the test responses, particularly on branches/PRs that don't make changes related to vertex, where the owner of the branch isn't aware of any of this.

I suppose we could work on a fix before the breaking changes are merged to main by modifying our test scripts to checkout the commit with the breaking changes, but then if a branch isn't rebased on top of the fix its CI will still go red anyway.

The only solution I can think of to this problem right now is checking out individual commits, but then this introduces some overhead because we'd have to update that commit every time a change is made to the golden files, which defeats a part of their purpose.

[rlazo]

I do feel a bit uneasy about CI being guaranteed to go red any time there is a breaking change to the test responses, particularly on branches/PRs that don't make changes related to vertex, where the owner of the branch isn't aware of any of this.

I suppose we could work on a fix before the breaking changes are merged to main by modifying our test scripts to checkout the commit with the breaking changes, but then if a branch isn't rebased on top of the fix its CI will still go red anyway.

Do JS run all the tests for all the products for each PR?

[rlazo]

1. Breaking changes to the response are checked into the main branch of the golden files repo.
2. This will, potentially, break your code. Then you'll keep having failures until the code is fixed.
   2.1 If you need the code to pass tests now, because you are working on something else, you can change your code to checkout the commit before the breaking change.
   2.2 Once your code is fixed, you can start checking out main HEAD again

This sounds reasonable to me as well on the Swift side. I think we'll just need to make sure it's well documented in the workflows / scripts so the rest of our team members can easily spot the issue and temporarily work around it (or know to ignore CI failures).

If breakages become too frequent, one other possible process I can think of:

1. The scripts in each repo could checkout a specific commit (or semver tag) from vertexai-sdk-test-data.
2. CI workflows in each repo could add a warning comment in PRs if not using the latest commit / tag from vertexai-sdk-test-data (to suggest that you update).

cc: @paulb777 in case you have ideas or opinions on processes.

Yes, we need to document this so it's clear what to do when these errors are found.

And as you said, these particular breaking changes should be rare, as they not only break SDKs, but anybody using the REST API directly.

[dlarocque]

I do feel a bit uneasy about CI being guaranteed to go red any time there is a breaking change to the test responses, particularly on branches/PRs that don't make changes related to vertex, where the owner of the branch isn't aware of any of this.
I suppose we could work on a fix before the breaking changes are merged to main by modifying our test scripts to checkout the commit with the breaking changes, but then if a branch isn't rebased on top of the fix its CI will still go red anyway.

Do JS run all the tests for all the products for each PR?

Yeah, we have a workflow that runs all the tests.

From this PR: https://github.com/firebase/firebase-js-sdk/actions/runs/9688342328/job/26735897472?pr=8333#step:8:1

[hsubox76]

I do feel a bit uneasy about CI being guaranteed to go red any time there is a breaking change to the test responses, particularly on branches/PRs that don't make changes related to vertex, where the owner of the branch isn't aware of any of this.
I suppose we could work on a fix before the breaking changes are merged to main by modifying our test scripts to checkout the commit with the breaking changes, but then if a branch isn't rebased on top of the fix its CI will still go red anyway.

Do JS run all the tests for all the products for each PR?

Yeah, we have a workflow that runs all the tests.

From this PR: https://github.com/firebase/firebase-js-sdk/actions/runs/9688342328/job/26735897472?pr=8333#step:8:1

Just for clarification, that isn't a required check (it's "Test All Packages" in the checks below). Contributors can still merge their PRs if it fails. It does make an alarming red X that makes contributors nervous to merge, though, and they usually double check with the core team, and we have to reassure them that it isn't related to their change, so that's a bit of friction.

I don't know if this is overengineering, but can we make the workflow smart, so that we can specify a commit/branch name to clone in the PR somewhere with like a label, or some text in the PR title in special brackets of some kind, and the workflow can put that in an env variable or something and the git clone step will clone from that commit/branch if it finds that string? And if it doesn't find any string like that, then it clones from main like normal? Then if you want the CI to use "staging" or "an old commit" or whatever you just put [[[staging]]] or [[[sagd43]] or whatever in the PR title or somewhere Github workflows can read easily.

[rlazo]

Here's a proposal:

The golden test repository will tag all commits in a semver-ish manner

• Major versions for any changes to an existing file
• Minor versions for adding new files

This way you know that the contents in the commit tagged v2.2 and v2.6 are compatible and will not break your tests.

Furthermore, you can "pin" your repo to a major version, but get updates for the minor versions. For example major version 5, by running

git tag -l 'v5.*' | tail -n1

this will return the latest tag in the v5 group. To bump to the next major version you would have to manually edit that command to use v6.* instead.

This will help keep the maintenance of the golden file repo low, while enabling platform teams to use a known "good" version.