Support update test for BigQueryConnectionConnection #2342
Conversation
maqiuyujoyce
force-pushed
the
202407-bqcc-mockgcp
branch
from
ad71f29 to
b7bc14b
Compare
| serviceAccountId: "" |
There was a problem hiding this comment.
nit: I think we want an extra line in the end
There was a problem hiding this comment.
There is an extra line at the end of the file. If there is not, it'll show the red sign. E.g. https://github.com/GoogleCloudPlatform/k8s-config-connector/pull/1981/files#diff-ef5a4fb16c799f5e257555474294ae1b67dd2c733712e98c3b6db2a2e48aaf85
| projectRef: | ||
| external: ${projectId} | ||
| cloudResource: | ||
| serviceAccountId: "" |
There was a problem hiding this comment.
nit: I think we want an extra line in the end
| @@ -352,7 +352,7 @@ func testNoChange(ctx context.Context, t *testing.T, testContext testrunner.Test | |||
| func testUpdate(ctx context.Context, t *testing.T, testContext testrunner.TestContext, systemContext testrunner.SystemContext, resourceContext contexts.ResourceContext) { | |||
| // Tests with `SkipUpdate` explicitly set to 'true' or tests for | |||
| // auto-generated resources don't support update test. | |||
| if resourceContext.SkipUpdate || resourceContext.IsAutoGenerated(systemContext.SMLoader) { | |||
| if resourceContext.SkipUpdate || resourceContext.IsAutoGenerated(systemContext.SMLoader) && resourceContext.ResourceKind != "BigQueryConnectionConnection" { | |||
There was a problem hiding this comment.
Just curious why we change the test behavior here. I think we should allow the dynamic test to cover the new update test(friendlyName &description) as well?
There was a problem hiding this comment.
This resource is hacky. It is not auto-generated, but we used the autogen channel to support it as a v1alpha1 resource. Auto-generated resources don't support the update test, so I have to explicitly make this resource a exceptional case to run testUpdate().
I don't want to turn this exception into a feature because this should be the only resource like this.
I'll add a comment about it.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gemmahou The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
google-oss-prow
bot
merged commit 608567a
into
GoogleCloudPlatform:master
Change description
Updated
spec.friendlyNameandspec.descriptionin the BigQueryConnectionConnection integration test, and support the update in the mockgcp.Tests you have done
make ready-prto ensure this PR is ready for review.