Fix apiserver_id 'get' method #184
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/M
Codecov Report
@@ Coverage Diff @@
## master #184 +/- ##
=========================================
- Coverage 92.46% 92.17% -0.3%
=========================================
Files 13 13
Lines 1474 1470 -4
=========================================
- Hits 1363 1355 -8
- Misses 111 115 +4
Continue to review full report at Codecov.
|
|
/assign @yliaog |
| provider = loader._user['auth-provider'] | ||
| self.assertTrue(loader._azure_is_expired(provider)) | ||
|
|
||
| def test_azure_with_bad_apiserver(self): |
There was a problem hiding this comment.
The code fix looks good to me. Could you point out which other test cases cover these pruned cases?
There was a problem hiding this comment.
These two tests are calling the same function as test_azure_with_expired_num with the same arguments. The apiserver values here aren't actually used in _azure_is_expired and _refresh_azure_token is not currently covered by tests (which is why this slipped through in the first place, and the test coverage didn't really change)
https://github.com/kubernetes-client/python-base/pull/184/files#diff-3123bf09ac4950ea8bf8de46fb38eb83L1132
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fooka03, roycaihw 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 |
k8s-ci-robot
added
the
approved
Fixes #179
Not sure how I missed this in testing locally but ConfigNode definitely does not have a .get method. This change instead relies on getitem and it throwing a ConfigException to fall back to the default value.
This commit also prunes some excess tests that are already covered by other cases.