Skip to content
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

tests:chore: update for logbucketmetric #1795

Merged
merged 1 commit into from
May 15, 2024

Conversation

acpana
Copy link
Collaborator

@acpana acpana commented May 14, 2024

Path adds update struct for logbucketmetric test.

$ go test -v -tags=integration ./pkg/controller/dynamic/ -timeout 1600s -test.count=1 -test.run 'TestCreateNoChangeUpdateDelete/logging/basic-logbucketmetric'  2>&1
...
--- PASS: TestCreateNoChangeUpdateDelete (0.12s)
    --- PASS: TestCreateNoChangeUpdateDelete/logging (0.00s)
        --- PASS: TestCreateNoChangeUpdateDelete/logging/basic-logbucketmetric (79.51s)
PASS
...
ok      github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/dynamic      89.455s

@acpana
Copy link
Collaborator Author

acpana commented May 14, 2024

/hold want the base ref to merge first : acpana/test-reconcile-direct

EDIT: add base ref acpana/test-reconcile-direct

@acpana acpana added this to the 1.118 milestone May 14, 2024
@justinsb
Copy link
Collaborator

What's the base ref?

@acpana
Copy link
Collaborator Author

acpana commented May 15, 2024

Copy link
Collaborator

@yuwenma yuwenma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

One minor comment just for maintenance nit.

pkg/test/resourcefixture/contexts/logging_contexts.go Outdated Show resolved Hide resolved
@yuwenma
Copy link
Collaborator

yuwenma commented May 15, 2024

/approve

@acpana acpana force-pushed the acpana/test-reconcile-direct branch from 68c43d2 to 8c4339e Compare May 15, 2024 18:09
@acpana acpana changed the base branch from acpana/test-reconcile-direct to master May 15, 2024 18:58
@justinsb justinsb force-pushed the acpana/direct-logbucketref branch from 5662151 to ae3984b Compare May 15, 2024 19:09
@google-oss-prow google-oss-prow bot removed the lgtm label May 15, 2024
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yuwenma

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@yuwenma
Copy link
Collaborator

yuwenma commented May 15, 2024

/hold

Need further discussion about the skip update

@acpana acpana force-pushed the acpana/direct-logbucketref branch from ae3984b to 567f6cc Compare May 15, 2024 19:45
@acpana
Copy link
Collaborator Author

acpana commented May 15, 2024

Added update.yaml and re-ran the test ✅

@yuwenma
Copy link
Collaborator

yuwenma commented May 15, 2024

Regarding b/331421837, do we have any test to have the LoggingLogMetric using the new field spec.logginglogbucketRef? Maybe I miss it, but I don't see any examples under resourcefixture/testdata.

@acpana acpana changed the title tests:chore: add resource context tests:chore: update for logbucketmetric May 15, 2024
@yuwenma
Copy link
Collaborator

yuwenma commented May 15, 2024

/hold cancel

@yuwenma
Copy link
Collaborator

yuwenma commented May 15, 2024

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label May 15, 2024
@google-oss-prow google-oss-prow bot merged commit 71f936c into master May 15, 2024
13 checks passed
@acpana acpana deleted the acpana/direct-logbucketref branch May 20, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants