Add unit tests for config/package_resource.go PART 1#1003
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bromivipo The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
bromivipo
changed the title
|
Hi @bromivipo. Thanks for your PR. I'm waiting for a GoogleCloudPlatform member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
| OSPolicy_Resource: &agentendpointpb.OSPolicy_Resource{ | ||
| ResourceType: &agentendpointpb.OSPolicy_Resource_Pkg{Pkg: aptInstalledPR}, | ||
| }, | ||
| } |
There was a problem hiding this comment.
maybe lets move all these setup comands in a separate func, e.g. setupPackageResourceCleanupTest, to increase test readability?
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusNotFound) | ||
| })) | ||
| defer server.Close() |
There was a problem hiding this comment.
t.Cleanup(server.Close), in most cases t.Cleanup is most preferable then defer by several reasons
| wantErr error | ||
| }{ | ||
| { | ||
| name: "Apt does not exist, expect error", |
There was a problem hiding this comment.
nitpick: update "expect error" in cases, like "expect apt install error" or "expect 404 error"
bromivipo
force-pushed
the
package_resource
branch
from
120d3f4 to
4211b40
Compare
2 participants
Add unit tests for Validate and Cleanup methods