Breakable Toy: Go CLI for Gitlab

Question

I started to learn programming in Go and came up with a Breakable Toy, which is a CLI for Gitlab. To get something up and running fast, I used some Go libraries:

• Ginkgo for testing
• Cobra for basic CLI stuff
• Sling for HTTP requests and JSON mapping

The project can be found in this repository on Github.

I am looking for any feedback / code review that could help me to better understand the concepts and conventions of Go. I come from the Java world and have the impression that I try to stick too much on the object oriented world. So any feedback concerning code style and best-practices is highly appreciated.

Besides general feedback, here are some specific questions:

1. Error Handling in Go

Being used to the try catch mechanism in Java, the _, err := mechanism in Go seems a little unhandy to me. To me it looks as if I have to implement the "bubble up" of exceptions all by myself. Example:

// HTTP client
func (client *GitlabClient) Do(req *http.Request, value interface{}) (*http.Response, error) {
    resp, err := client.sling.Do(req, value, nil)
    return resp, err
}

// service
func (service *ProjectsService) List() (*[]model.Project, error) {
    // ...
    _, err = service.Client.Do(req, projects)
    if err != nil {
        return nil, err
    }
    return projects, nil
}

// CLI command 
func(cmd *cobra.Command, args []string) error {
    projects, err := gitlabClient.Projects.List()
    if err != nil {
        return err
    }
    err = OutputJson(projects)
    return err
}

so there are really many LOCs only for error handling - is that the "way to go"?

2. Global Variables

The Cobra library seems to store values for CLI flags in global variables:

var id string

// ...

projectGetCmd.PersistentFlags().StringVarP(&id, "id", "i", "", "(required) Either ID of project or 'namespace/project-name'")
viper.BindPFlag("id", projectGetCmd.PersistentFlags().Lookup("id"))

This makes it really hard to figure out which flags belong to which command and it's easy to re-initialize the same variable. See group.go, line 31 for the full example.

Is there any rule of thumb / best practice of how to namespace variables in Go besides structs?

3. Mocking in Unit Tests

I wrote some integration tests with Ginkgo that also simulate an HTTP server. This works quite nicely. But now I wanted to test, whether my CLI commands are calling the expected service methods and therefore I wanted to mock the services classes in the Unit tests for the commands. I couldn't find a convincing way for how to mock objects in Go. Can you give me any hint / resources for that?

Disclaimer

I am not expecting a full code review of my application. But if you have any suggestions for improvement, I'd be glad the get them.

Answer 1

1. Yes, this is the way to do error handling in Go. By design, you can't let exceptions propagate to the callers "silently", you have to consider what makes sense at each step. I found that on large-scale projects, it makes it significantly easier to predict and test the error handling behavior of your code. It also forces you to put the error handling first, which (imho) results in more readable, less nested code.
2. Truly global variables don't exist in Go — if a package foo exports a variable Bar, other packages will have to call foo.Bar to access it. That being said, it pretty much never makes sense to do that.
   In the example you gave, cmd.go and project.go are in the same package, and all package-level variables are shared, so identifiers conflict. If you want this variable to be shared, it's more readable to put all functions that use it in the same file.
3. When you want to mock some of your functions, it's the sign that there should probably be an associated interface. You can then "mock" an interface easily, by re-implementing your interface in your tests and making it do whatever you want. I find this more readable and idiomatic than using complex frameworks.

Note on the last question: if your interface has many functions:

type Fooer interface {
    Foo1(string) (string, error)
    Foo2(int) (string, int)
    ...
    // Foo42() []byte
}

and you only want to test Foo1, you can use the following syntax

type fakeFooer struct {
    Fooer
    ret string
}

func (fi fakeFooer) Foo1(_ string) (string, error) {
     return fi.ret, nil
}

and not implement any other functions. fakeFooer{"bar"} will "implement" Fooer, calling Foo1 will do what you expect, and trying to call Foo2 on it will panic (nil pointer dereference).