Update agent controller tests #167
Conversation
linkas45
requested review from
saumas,
mindaugasCast,
dariuspie,
zilvinasu and
KarolisL
as code owners
| tests := map[string]struct { | ||
| objectCount int | ||
| }{ | ||
| "happy path": { | ||
| objectCount: 14, | ||
| }, | ||
| } |
There was a problem hiding this comment.
question: do we need a test case for the single test?
I assume you would need this in follow-up changes. Otherwise, this is premature optimization and it doesn't add any value. I would better if I got intent of this PR while reading it.
There was a problem hiding this comment.
yeah, this is a refactor for other changes so there would be less visible changes when actual bug fix is implemented
There was a problem hiding this comment.
it does not add too much value as a separate PR when I looked into #168.
there would be less visible changes when actual bug fix is implemented
agreed, can be kept as it is but still feels like not adds enough benefit to tests
There was a problem hiding this comment.
well if I wouldn't separate it would be more difficult to read the actual changes because there is test refactor where data is moving and additional logic to support bug fix
| tests := map[string]struct { | ||
| objectCount int | ||
| }{ | ||
| "happy path": { |
There was a problem hiding this comment.
minor: please use more explicit names for test cases.
happy path is too vague, don't give me a hint about what is conditions and meaning of happy path
No description provided.