feat: add unit tests for auth token#106
feat: add unit tests for auth token#106helayoty merged 1 commit intoAzure:mainfrom helayoty:config-tests
Conversation
Codecov upload limit reached
|
minhng22
left a comment
minhng22
left a comment
There was a problem hiding this comment.
Some minor comments. The refreshToken code I don't have good understanding right now so I could review next round but I would be slower in reviewing.
|
Semantics looks good. Token writer looks good. I don't have good understanding of token refresher so please get a third eye. If by midday tomorrow no body else takes it I would dig into the code and review that part. |
minhng22
left a comment
minhng22
left a comment
There was a problem hiding this comment.
Semantics looks good. Token writer looks good. I don't have good understanding of token refresher so please get a third eye. If by midday tomorrow no body else takes it I would dig into the code and review that part.
| } | ||
|
|
||
| // TestRefreshTokenOnce test to refresh/rewrite token for one time | ||
| func TestRefreshTokenOnce(t *testing.T) { |
There was a problem hiding this comment.
I wonder why is this UT not following the table driven test pattern in all the other UTs in the repo?
| return nil | ||
| } | ||
|
|
||
| func TestWriteToken(t *testing.T) { |
There was a problem hiding this comment.
fs related utility function should take an interface as a parameter or use a local function var. All the added interface/factory/function pointers and the mock just to produce one test case to cover the happy case do not seem to be worth the extra code and complexity. This is not the idiomatic go way.
3 participants
Description of your changes
No unit tests for
azure_msi. It will be covered in the integration/e2e testsI have:
Run
make reviewableto ensure this PR is ready for review.How has this code been tested
Special notes for your reviewer