gardenctl
gardenctl copied to clipboard
reduce usage of checkError to make it possible to write tests
We have a very generic function checkError
in utils.go
file.
The fatal flaw of this function that if there a non-nil error then it call for log.Fatalf(err.Error())
and exit.
It's mean that if this function is used somewhere then it makes it not possible to test the calling function because it will exit during the test execution.
It also prevents us from doing some cleaning after the executions if we created some files or other stateful manipulations.
A generic approach would be to check for errors during the code flow and if there is an error then pass it up to the stack call. Somewhere at the very top of the call chain, we can execute this function and finally exit if there an error.
At the moment we call this function 394 times
💯
yes also observe this in https://github.com/gardener/gardenctl/pull/404#discussion_r514777299. the checkError
not return and exit after invoked, haven't got an idea of how to improve it at that time.
Also the os.Exit calls should be removed or at least reduced. See also this comment from @tedteng https://github.com/gardener/gardenctl/issues/328#issuecomment-734033925 Instead, like @andrei-panov wrote, the error should bubble up the call stack.
Do we have any guidelines? Since it's not possible to fix all places in one shot at least we can collect and document some desired (soft, flexible...) directions of future development.