Add tests for DeleteRelease #1438
Conversation
Add SKIP switch to tests in /kubeops/internal/handler Signed-off-by: Latiif <abdullatif.alshriaf@combitech.se>
| } | ||
|
|
||
| for _, test := range tests { | ||
| t.Run(test.Description, func(t *testing.T) { | ||
| // TODO Remove this `if` statement after the memory driver bug is fixed |
There was a problem hiding this comment.
Thanks @latiif . Can you reference the actual Issue/PR (PR in this case) to make it easier to audit later? Thanks. (both in this PR and the previous test one)
pkg/agent/agent_test.go
Outdated
| makeReleases(t, cfg, tc.releases) | ||
| err := DeleteRelease(cfg, tc.releaseToDelete, true) | ||
| t.Logf("error: %v", err) | ||
| if isOk := err != nil == tc.shouldFail; !isOk { |
There was a problem hiding this comment.
This might be easier to read as:
if didFail := err != nil; didFail != tc.shouldFail {
}
| @@ -140,6 +214,8 @@ func TestActions(t *testing.T) { | |||
| switch test.Action { | |||
| case "create": | |||
| CreateRelease(*cfg, response, req, test.Params) | |||
| case "delete": | |||
| DeleteRelease(*cfg, response, req, test.Params) | |||
There was a problem hiding this comment.
See what you think, but in #1439 I've used a separate test case for Rollback, rather than adding to this one. As mentioned there, I know it's probably just following the precedent from tiller-proxy, but it means tests for the individual handlers can't be focused. For example, here we've had to add a t.SkipNow() which is only relevant for a certain Delete action. Anyway, happy for this to land as is, just pointing out in case you choose to separate them. I can re-base my diff once yours lands.
There was a problem hiding this comment.
Smaller functions are always better.
To avoid overhead complications with multiple test PR's coming in right now, and the eventuality of merge conflicts. I plan to postpone splitting up the TestActions function to smaller ones, once the test PR's have landed. If that's okay with you.
There was a problem hiding this comment.
Yep, that's fine. Let's land this one (and your other one), then tomorrow I'll rebase my Rollback one with the separate test and we can go from there.
Signed-off-by: Latiif <abdullatif.alshriaf@combitech.se>
Description of the change
Tests new functionality introduced by #1420
Benefits
Possible drawbacks
Skip tests blocked by memory driver bug
Applicable issues
Additional information
Contains a
TODOin/cmd/kubeops/internal/handler/handler_test.gowith instructions on what to do when the memory driver bug is fixed.