Skip to content
This repository has been archived by the owner on Jul 25, 2022. It is now read-only.

reduce usage of checkError to make it possible to write tests #458

Open
andrei-panov opened this issue Nov 24, 2020 · 4 comments
Open

reduce usage of checkError to make it possible to write tests #458

andrei-panov opened this issue Nov 24, 2020 · 4 comments
Labels
lifecycle/rotten Nobody worked on this for 12 months (final aging stage)

Comments

@andrei-panov
Copy link
Contributor

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

@petersutter
Copy link
Contributor

💯

@tedteng
Copy link
Contributor

tedteng commented Nov 25, 2020

yes also observe this in #404 (comment). the checkError not return and exit after invoked, haven't got an idea of how to improve it at that time.

@petersutter
Copy link
Contributor

Also the os.Exit calls should be removed or at least reduced. See also this comment from @tedteng #328 (comment)
Instead, like @andrei-panov wrote, the error should bubble up the call stack.

@andrei-panov
Copy link
Contributor Author

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.

@gardener-robot gardener-robot added the lifecycle/stale Nobody worked on this for 6 months (will further age) label Sep 22, 2021
@gardener-robot gardener-robot added lifecycle/rotten Nobody worked on this for 12 months (final aging stage) and removed lifecycle/stale Nobody worked on this for 6 months (will further age) labels Mar 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lifecycle/rotten Nobody worked on this for 12 months (final aging stage)
Projects
None yet
Development

No branches or pull requests

4 participants