Cover agentconfig functionality by unit tests#925
Cover agentconfig functionality by unit tests#925iliatsuprik wants to merge 5 commits intoGoogleCloudPlatform:masterfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: iliatsuprik 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 |
|
Hi @iliatsuprik. 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. |
|
/gcbrun |
2 similar comments
|
/gcbrun |
|
/gcbrun |
9b2e8c2 to
c555736
Compare
|
/gcbrun |
burov
left a comment
There was a problem hiding this comment.
I think we might benefit from using table driven testing more often in this CL, as well i see a lot of code duplication in test set-up, some of the code repetitions are low hanging fruits and can be just extracted to separate function as is.
dd95306 to
3310076
Compare
|
@burov, thanks for the comprehensive review. I refactored most of the tests according to your comments. Could you check them again? |
|
/gcbrun |
3310076 to
6abe8ad
Compare
This PR contains unit tests and minor changes in code for github.com/GoogleCloudPlatform/osconfig/agentconfig package.
Initial coverage: 58.1%, current coverage: 96.4%
Changes in the code of main app: