Add flag to specify build tags

[namreg]

I use minimock for my project and really like it. In that project, I have a lot of mocks. I want to move these mocks to separate package and exclude from the build for production. Hence, I need an ability to specify build tags during generating mocks.

[namreg]

@hexdigest Any thoughts? :)

[hexdigest]

@namreg is this package you want to move mocks to will contain only these mocks? If so, I believe it would be excluded from the production build naturally since you will import this package only from your tests.

[namreg]

@hexdigest Oh... I read go documentation inattentively. Thanks! So, there are no needs for me in this issue, I think you may close it :)

[namreg]

@hexdigest Sorry, it seems I harried up with conclusions in the previous comment. I could not move mocks to separate package because of I've got the cyclic dependency. So, I decided to put mock together with an interface. Hence, I still need an ability to specify build tags for generated mock

[hexdigest]

@namreg You still don't need build tags for that. By default minimock generates *_mock_test.go files so they are ignored by the compiler when you make build for production. If you specified output files names manually via "-o" flag then you can just change files names for you mocks so they end up with _test.go suffix.

[namreg]

@hexdigest as I know, file with suffix _test.go compiles only when tests running in a package where this _test.go file is located. But I need an ability to use mocks in other packages, for example:

├───storage
│      ├───storage.go
│      └───storage_mock.go <-- Here is my mock
├───server
│      ├───server.go
│      └───server_test.go <-- I need storage_mock here
└───command
       ├───command.go
       └───command_test.go <-- I need storage_mock here

[hexdigest]

@namreg I saw this situation many times. Typically it means that you have an interface declaration in the storage package (which is a bit against Go duck typing concept) and share this interface between server and command packages instead of defining separate interfaces in both packages.

Do server and command packages use exactly same subset of methods of an interface declared in storage package? If these packages use different set of methods it means that they communicate with the storage client via different interfaces and it worth declaring separate interface for each package listing the only methods that are actually used and put this interface to the corresponding package. However both of the interfaces will be implemented by the storage client struct.

[namreg]

@hexdigest I understand what you mean, but I believe not always this can be achieved. So, what if I have a small interface, let's say Clock, that has 2 methods: Now() and StartEpoch(). These methods are being used by a few others packages. What can I do in this situation?

[hexdigest]

@namreg If you pass this interface as a parameter to the functions/methods defined in different packages than you can share it. This approach is widely used in standard library and common interfaces like io.Reader, io.Writer and so on. But if you use this interface as a dependency then I would copy the interface declaration rather than introduce another dependency.

A little copying is better than a little dependency. (c) Rob Pike

If you add build tags to these mocks you will either have to make production build using a build tag or run your tests using build tags.

I honestly think that copying interface declaration is better approach to the problem but if you insist I can add this functionality :)

[namreg]

@hexdigest I am using the clock interface as a dependency in a few packages. I create this dependency in the main function and pass it to that packages. So, if I will copy the clock interface, let's say 2 times, then I will require to create 2 implementations of this interface which will be the same. In the main function the code will be like:

func main() {
    serverClock := server.NewClock()
    storageClock := storage.NewClock()

    strg := memory.New(memory.WithClock(storageClock))
    srv := server.New(server.WithClock(serverClock))

    srv.Run(strg)
}

It seems that the code looks a little bit ugly :)

I can't insist that you should accept my PR. You are the main maintainer (and as I know you are also creator) of this software and only YOU can decide what functionality this software will have. Build tags is useful for me and I believe that it can be useful for others.

Finally, I can use forked version :) If I understand correctly, LICENSE doesn't disallow this.

Nevertheless, thank you for your detailed answers 👍

[hexdigest]

@namreg I don't think you need two implementations. You need to define two interfaces: one in the memory package and one in the server package. Then you can have only one implementation of the clock which will implement both of the interfaces due to the duck typing.

So in result you will have something like:

func main() {
    clk := clock.New()

    strg := memory.WithClock(clk)
    srv := server.WithClock(clk)

    srv.Run(strg)
}

[namreg]

@hexdigest Ok, I agree with this approach relative to the clock interface. So, what if my interface has more than 2-3 methods? Let's go back to the Storage interface. Suppose, it has the following methods:

package storage

type Storage interface {
    Put(Key, Value) error
    Get(Key) (*Value, error)
    Del(key Key) error
    Keys() ([]Key, error)
    All() (map[Key]*Value, error)
}

Let's say I will segregate this interface into 2 interfaces that located in other packages:

package server

type Storage interface {
    Keys() ([]Key, error)
    All() (map[Key]*Value, error)
}

package command

type Storage interface {
    Put(Key, Value) error
    Get(Key) (*Value, error)
    Del(key Key) error
}

So, there are several questions that I don't like:

1. I will need to duplicate Key and Value in those three packages (storage, command, server).
2. How can I be ensured that my implementation of the storage (memory, for example) satisfies for both interfaces? I could create a Storage interface in the storage package like that:

   
   package memory

var _ Storage = &Memory{}

type Storage interface { server.Storage command.Storage }

But, in this case, I will get a cyclic import error due to the storage interface imports server package and vice versa;

[hexdigest]

@namreg

1) You may not want to duplicate Key and Value but it depends on how these types look like in your case. If they are just an aliases of the interface{} I would end up using interface{} instead of these types. 2) You don't have to do it in this way because you will use memory.Memory type somewhere in the main package (I guess) where it will be used as both server.Storage and command.Storage types so your implementation will be type-checked anyways.

package main

memoryStorage := memory.New(...)
srv := server.New(memoryStorage)
command := command.New(memoryStorage)

Just a side note. With duck typing you don't know how many interfaces will be defined on top of your memory.Memory type. With 5 public methods there can be at least 5! = 120 different interfaces. Also you don't know in which packages these interfaces will be defined and their names. So this kind of check seems meaningless in this case.

[hexdigest]

@namreg Any thoughts on above comment? I'm just trying to convince you that using build tags is almost always a bad idea except the main use case - different implementations of something for different platforms.

[namreg]

@hexdigest

1. No, my Key and Value are not aliases of the interface{}. Key is the alias for string, but Value is a struct type with some fields: data, ttl,datatType`
2. I agree with you, but when I implement a new storage I would like to know what methods I should implement. And I see this approach many times. Maybe it is wrong. Golang faq has some comments about that https://golang.org/doc/faq#guarantee_satisfies_interface.

Most code doesn't make use of such constraints since they limit the utility of the interface idea. Sometimes, though, they're necessary to resolve ambiguities among similar interfaces.

[hexdigest]

@namreg

I've also seen these constraints many times but most of the times it was a constraint either on local (non-public) interface implementation or on some implementation of the interfaces from the standard library. If you still want this constraint in your code here is the complete Go way solution to your problem without using build tags:

$ tree
.
├── cmd
│   └── program
│       └── program.go
├── command
│   └── command.go
├── server
│   └── server.go
└── storage
    ├── memory
    │   └── memory.go
    └── storage.go

6 directories, 5 files

$ cat cmd/program/program.go

package main

import (
    "fmt"

    "github.com/test/command"
    "github.com/test/server"
    "github.com/test/storage/memory"
)

func main() {
    storage := &memory.Memory{}

    srv := server.New(storage)
    cmd := command.New(storage)

    fmt.Println(srv, cmd)
}

$ cat command/command.go

package command

import "github.com/test/storage"

type Command struct {
    s Storage
}

//Generate mock for this interface to test Command
type Storage interface {
    Put(storage.Key, storage.Value) error
    Get(storage.Key) (*storage.Value, error)
    Del(storage.Key) error
}

func New(s Storage) *Command {
    return &Command{s: s}
}

$ cat server/server.go

package server

import "github.com/test/storage"

type Server struct {
    s Storage
}

//Generate mock for this interface to test Server
type Storage interface {
    Keys() ([]storage.Key, error)
    All() (map[storage.Key]*storage.Value, error)
}

func New(s Storage) *Server {
    return &Server{s: s}
}

$ cat storage/storage.go

package storage

import "time"

// Do not generate mocks for this interface it will be used only for type checking
// of ths Storage implementations
type Storage interface {
    Put(Key, Value) error
    Get(Key) (*Value, error)
    Del(key Key) error
    Keys() ([]Key, error)
    All() (map[Key]*Value, error)
}

type Key string

type Value struct {
    Data     interface{}
    DataType string
    TTL      time.Duration
}

$ cat storage/memory/memory.go

package memory

import "github.com/test/storage"

var _ storage.Storage = &Memory{}

type Memory struct {
}

func (m *Memory) Put(storage.Key, storage.Value) error {
    return nil
}

func (m *Memory) Get(storage.Key) (*storage.Value, error) {
    return nil, nil
}

func (m *Memory) Del(storage.Key) error {
    return nil
}

func (m *Memory) Keys() ([]storage.Key, error) {
    return nil, nil
}

func (m *Memory) All() (map[storage.Key]*storage.Value, error) {
    return nil, nil
}

[namreg]

@hexdigest Big thanks for your detailed explanation! I really appreciate you for that. I will try to refactor my code taking into account the above-mentioned comments.

[namreg]

@hexdigest So, this approach is not working for me, again... My problem is that Server is responsible for creating a Command. Hence, the Server interface is not suitable for the Command interface:

//package server
func New(s Storage) *Server {
    return Server{s :s}
}
func (srv *Server) Run() {
   // get input from user
  ....
   cmd, args, err := command.Parse(input)
   cmd.Execute(srv.s, args...) // <---- Here I have an error
}

Maybe I do something wrong and my architecture is completely not satisfied to go way

[hexdigest]

@namreg

I think something wrong with architecture. First of all not all commands may need storage, maybe you want to introduce something like "ping" command or "stats" command in the future that don't need storage at all.

That's what I would do:

package command

type Command interface {
  Run(args []string) error
}

type Storage interface {
    Put(storage.Key, storage.Value) error
    Get(storage.Key) (*storage.Value, error)
    Del(storage.Key) error
}

type Parser struct {
   s Storage
}

func (p Parser) Parse(s string) (Command, []string, error) {
   //parses command name and args from s and creates corresponding
   //implementation of a command
   ...
   switch name {
   case "ping":
      return PingCommand{}, nil, nil
   case "get":
      return GetCommand{s: p.s}, args, nil
   default:
      return nil, nil, ErrUnknownCommand
   }
}

package server

type Server struct {
    s storage
        parser commandParser
}

//Generate mock for this interface to test Server
type storage interface {
    Keys() ([]storage.Key, error)
    All() (map[storage.Key]*storage.Value, error)
}

type commandParser interface {
  Parse(string) (cmd command.Command, args []string, error)
}

func New(s Storage, parser commandParser) *Server{
  return &Server{s: s, parser: parser}
}

func (srv *Server) Run() {
   // get input from user
  ....

   cmd, args, err := srv.parser.Parse(input)
   fmt.Printf("running %T command with args: %v\n", cmd, args)
   cmd.Run(args)
}

package main

func main() {
    storage := &memory.Memory{}
        parser := command.Parser{s: storage}

    srv := server.New(storage, parser)
        srv.Run()
}

I hope you get the idea. With this approach you can also mock a command parser so it can return an implementation of a command.Command with required behaviour. Also you can test you real implementations of the commands separately.

[namreg]

@hexdigest Finally, I have done it! :) And I think It seems pretty cool. Thank you very much! Can you recommend any resources about duck typing concepts?

[hexdigest]

Well I came from the Ruby background so this concept was pretty natural for me. This article seems ok: https://theburningmonk.com/2015/05/why-i-like-golang-interfaces/

Also there is "accept interfaces, return structs" idiom in Go. And of course https://en.wikipedia.org/wiki/Interface_segregation_principle