golang / mock

GoMock is a mocking framework for the Go programming language.
Apache License 2.0
9.26k stars 608 forks source link

mock does not import the source file #77

Closed junghoahnsc closed 6 years ago

junghoahnsc commented 7 years ago

I'm trying to generate a mock from a source.

$ cat x.go
package test

type X struct{}

type S interface {
    F(X)
}
$ mockgen -source x.go
// Automatically generated by MockGen. DO NOT EDIT!
// Source: x.go

package mock_test

import (
    gomock "github.com/golang/mock/gomock"
)

// Mock of S interface
type MockS struct {
    ctrl     *gomock.Controller
    recorder *_MockSRecorder
}

// Recorder for MockS (not exported)
type _MockSRecorder struct {
    mock *MockS
}

func NewMockS(ctrl *gomock.Controller) *MockS {
    mock := &MockS{ctrl: ctrl}
    mock.recorder = &_MockSRecorder{mock}
    return mock
}

func (_m *MockS) EXPECT() *_MockSRecorder {
    return _m.recorder
}

func (_m *MockS) F(_param0 X) {
    _m.ctrl.Call(_m, "F", _param0)
}

func (_mr *_MockSRecorder) F(arg0 interface{}) *gomock.Call {
    return _mr.mock.ctrl.RecordCall(_mr.mock, "F", arg0)
}

The generated mock does not import the original source for the type X. Should I edit the generated mock? I tried some flags like -imports, but it didn't work.

Thanks,

balshetzer commented 7 years ago

I think #38 might fix this problem. Give it a try.

junghoahnsc commented 7 years ago

-source_package seems to work. Thanks! Is there any reason #38 is not merged yet?

junghoahnsc commented 7 years ago

Is there any plan to merge #38?

balshetzer commented 6 years ago

I think it might be possible to make mockgen do this automatically. That would be better. If I can't do that then I'll merge #38 instead.

bhcleek commented 6 years ago

@balshetzer I think you're right about being able to make mockgen do this automatically. Have you made any progress?

zbintliff commented 6 years ago

Can't this be fixed by adding the below import (if mock package != source package):

import (
      . "package/to/mock"
)

That way type X from the example will be scoped to current package?

taybin commented 6 years ago

I switched to using https://github.com/petergtz/pegomock which has the correct behavior and seems to be actively maintained.

junghoahnsc commented 6 years ago

Is there any plan to resolve this issue anytime soon?

mikhailkolesnik commented 6 years ago

Why in the world #38 is not merged yet!

junghoahnsc commented 6 years ago

@balshetzer could you please merge #38? Is there any issue with it?

junghoahnsc commented 6 years ago

@balshetzer Thanks!, but what flag should I use to generate correctly? I was using -source_package with #38, but #164 doesn't seem to have that flag. I tried a few flags including -self_package, but it didn't work.

balshetzer commented 6 years ago

No additional flag should be necessary. mockgen should automatically do the right thing. Take a look at this example: https://github.com/golang/mock/blob/master/mockgen/tests/import_source/definition/source.go

It shows that the mock is generated correctly using both reflect mode and source mode.

balshetzer commented 6 years ago

Also, here is the example you gave in the original report. Notice that the source is imported.

balshetzer:~/workspace/src/test $ cat x.go
package test

type X struct{}

type S interface {
        F(X)
}
balshetzer:~/workspace/src/test $ mockgen -source x.go                                                                                     
// Code generated by MockGen. DO NOT EDIT.
// Source: x.go

// Package mock_test is a generated GoMock package.
package mock_test

import (
        gomock "github.com/golang/mock/gomock"
        reflect "reflect"
        test "test"
)

// MockS is a mock of S interface
type MockS struct {
        ctrl     *gomock.Controller
        recorder *MockSMockRecorder
}

// MockSMockRecorder is the mock recorder for MockS
type MockSMockRecorder struct {
        mock *MockS
}

// NewMockS creates a new mock instance
func NewMockS(ctrl *gomock.Controller) *MockS {
        mock := &MockS{ctrl: ctrl}
        mock.recorder = &MockSMockRecorder{mock}
        return mock
}

// EXPECT returns an object that allows the caller to indicate expected use
func (m *MockS) EXPECT() *MockSMockRecorder {
        return m.recorder
}

// F mocks base method
func (m *MockS) F(arg0 test.X) {
        m.ctrl.Call(m, "F", arg0)
}

// F indicates an expected call of F
func (mr *MockSMockRecorder) F(arg0 interface{}) *gomock.Call {
        return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "F", reflect.TypeOf((*MockS)(nil).F), arg0)
}
balshetzer:~/workspace/src/test $ 
junghoahnsc commented 6 years ago

oh, in our case type X struct{} is defined in a separate file in the same package. Is it possible to specify other files in the same package for mockgen to refer? we're using source mode since it's hard to use reflect mode in our repo.

balshetzer commented 6 years ago

Yes, you can use -aux_files <package_name>=<file_name> to specify the other file in the same package. I'll take a look at making this automated as well.

It's too bad you can't use reflect mode. Can you tell me why it's hard to use reflect mode in your repo?

balshetzer commented 6 years ago

Here is a full example:

balshetzer:~/workspace/src/test $ cat x.go
package test

type S interface {
        F(X)
}
balshetzer:~/workspace/src/test $ cat y.go
package test

type X struct{}
balshetzer:~/workspace/src/test $ mockgen -source x.go -aux_files test=y.go
// Code generated by MockGen. DO NOT EDIT.
// Source: x.go

// Package mock_test is a generated GoMock package.
package mock_test

import (
        gomock "github.com/golang/mock/gomock"
        reflect "reflect"
        test "test"
)

// MockS is a mock of S interface
type MockS struct {
        ctrl     *gomock.Controller
        recorder *MockSMockRecorder
}

// MockSMockRecorder is the mock recorder for MockS
type MockSMockRecorder struct {
        mock *MockS
}

// NewMockS creates a new mock instance
func NewMockS(ctrl *gomock.Controller) *MockS {
        mock := &MockS{ctrl: ctrl}
        mock.recorder = &MockSMockRecorder{mock}
        return mock
}

// EXPECT returns an object that allows the caller to indicate expected use
func (m *MockS) EXPECT() *MockSMockRecorder {
        return m.recorder
}

// F mocks base method
func (m *MockS) F(arg0 test.X) {
        m.ctrl.Call(m, "F", arg0)
}

// F indicates an expected call of F
func (mr *MockSMockRecorder) F(arg0 interface{}) *gomock.Call {
        return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "F", reflect.TypeOf((*MockS)(nil).F), arg0)
}
junghoahnsc commented 6 years ago

It's weird. I tried exactly same as you did, but it didn't work. I was using the mockgen at HEAD.

And does -aux_files work when there are multiple aux files in the same pkg like -aus_files test=y.go,test=z.go?

We're using bazel to support multiple languages in our repo, and the repo hierarchy doesn't follow the stand Go path. I didn't try the reflect mode enough so I have no idea whether I can use it in our case.

balshetzer commented 6 years ago

Yes, aux_files should support that.

Can you show me what you tried and what happened?

junghoahnsc commented 6 years ago

sure, it's same as you did.

$ cat x.go
package test

type S interface {
    F(X)
}

$ cat y.go
package test

type X struct{}

$ mockgen -source x.go -aux_files test=y.go
// Code generated by MockGen. DO NOT EDIT.
// Source: x.go

// Package mock_test is a generated GoMock package.
package mock_test

import (
    gomock "github.com/golang/mock/gomock"
    reflect "reflect"
)

// MockS is a mock of S interface
type MockS struct {
    ctrl     *gomock.Controller
    recorder *MockSMockRecorder
}

// MockSMockRecorder is the mock recorder for MockS
type MockSMockRecorder struct {
    mock *MockS
}

// NewMockS creates a new mock instance
func NewMockS(ctrl *gomock.Controller) *MockS {
    mock := &MockS{ctrl: ctrl}
    mock.recorder = &MockSMockRecorder{mock}
    return mock
}

// EXPECT returns an object that allows the caller to indicate expected use
func (m *MockS) EXPECT() *MockSMockRecorder {
    return m.recorder
}

// F mocks base method
func (m *MockS) F(arg0 X) {
    m.ctrl.Call(m, "F", arg0)
}

// F indicates an expected call of F
func (mr *MockSMockRecorder) F(arg0 interface{}) *gomock.Call {
    return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "F", reflect.TypeOf((*MockS)(nil).F), arg0)
}
balshetzer commented 6 years ago

That's confusing...

Can you confirm that you ran go install github.com/golang/mock/mockgen on the newest version first?

If that's not it then, what's the path for your test directory and what's your $GOPATH?

junghoahnsc commented 6 years ago

Yes, I did like

$ GOPATH=$HOME/go-new go get -u github.com/golang/mock/mockgen
$ ~/go-new/bin/mockgen -source x.go -aux_files test=y.go
// Code generated by MockGen. DO NOT EDIT.
// Source: x.go

// Package mock_test is a generated GoMock package.
package mock_test

import (
    gomock "github.com/golang/mock/gomock"
    reflect "reflect"
)

// MockS is a mock of S interface
type MockS struct {
    ctrl     *gomock.Controller
    recorder *MockSMockRecorder
}

// MockSMockRecorder is the mock recorder for MockS
type MockSMockRecorder struct {
    mock *MockS
}

// NewMockS creates a new mock instance
func NewMockS(ctrl *gomock.Controller) *MockS {
    mock := &MockS{ctrl: ctrl}
    mock.recorder = &MockSMockRecorder{mock}
    return mock
}

// EXPECT returns an object that allows the caller to indicate expected use
func (m *MockS) EXPECT() *MockSMockRecorder {
    return m.recorder
}

// F mocks base method
func (m *MockS) F(arg0 X) {
    m.ctrl.Call(m, "F", arg0)
}

// F indicates an expected call of F
func (mr *MockSMockRecorder) F(arg0 interface{}) *gomock.Call {
    return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "F", reflect.TypeOf((*MockS)(nil).F), arg0)
}
balshetzer commented 6 years ago

In what directory are x.go and y.go? What's their path relative to the $GOPATH when you run mockgen?

junghoahnsc commented 6 years ago

Oh, no, they are in a temp directory. does it require to put all sources under $GOPATH? if so, it would be problematic since we're using bazel and do not follow GOPATH as I mentioned.

balshetzer commented 6 years ago

I'm not sure. I'm still trying to figure out why you and I have different output with the same input. I tried running outside of $GOPATH and I still got different output.

I'm very confused. I'm not sure what else to try.

junghoahnsc commented 6 years ago

It's really weird. I ran the above test with go version go1.10 linux/amd64. Now I tried with go version go1.10 darwin/amd64 in osx, but it generates a different result like:

$ mockgen -source x.go -aux_files test=y.go
// Code generated by MockGen. DO NOT EDIT.
// Source: x.go

// Package mock_test is a generated GoMock package.
package mock_test

import (
    x "."
    gomock "github.com/golang/mock/gomock"
    reflect "reflect"
)

// MockS is a mock of S interface
type MockS struct {
    ctrl     *gomock.Controller
    recorder *MockSMockRecorder
}

// MockSMockRecorder is the mock recorder for MockS
type MockSMockRecorder struct {
    mock *MockS
}

// NewMockS creates a new mock instance
func NewMockS(ctrl *gomock.Controller) *MockS {
    mock := &MockS{ctrl: ctrl}
    mock.recorder = &MockSMockRecorder{mock}
    return mock
}

// EXPECT returns an object that allows the caller to indicate expected use
func (m *MockS) EXPECT() *MockSMockRecorder {
    return m.recorder
}

// F mocks base method
func (m *MockS) F(arg0 x.X) {
    m.ctrl.Call(m, "F", arg0)
}

// F indicates an expected call of F
func (mr *MockSMockRecorder) F(arg0 interface{}) *gomock.Call {
    return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "F", reflect.TypeOf((*MockS)(nil).F), arg0)
}

This is still incorrect, but has x "." at least. Does this depend on Go version?

junghoahnsc commented 6 years ago

@balshetzer do you have any idea why the behavior is different per Go version (platform)?

junghoahnsc commented 6 years ago

@balshetzer Is there any chance of addressing this issue soon?

balshetzer commented 6 years ago

I think the difference in behavior on different platforms must have to do with Go itself. We call Go libraries to load packages and maybe that behaves differently across platforms. Let's put that aside for a moment.

The problem is that you want to generate mocks from code that is laid out in a way that is not compliant with the way Go code should be laid out. I don't think that the right way to fix it is to make gomock figure out what to do since the set of possibilities are infinite for weird ways in which code could be laid out. It makes sense for gomock to assume that Go code follows Go rules.

I think the right solution is to make a bazel rule to run gomock.

We run in a bazel environment too and so our code is also not laid out in the way that Go code is supposed to be laid out. But the go_library and go_binary build rules create the appropriate hierarchy and then call the go compiler. We could do something similar to run gomock.

We don't do that exactly. Our gomock rule creates a go_binary target that depends on the original library being mocked and the main is generated from mockgen using the -prog_only flag. Then the script has a genrule that runs mockgen with -exec_only using the first target above as the generating program. This rule generates the mock. We don't check in our mock. We just have the test depend on the mock rule so it's generated when we run our tests.

junghoahnsc commented 6 years ago

Thanks for the detailed answer. Do you recommend to use reflect mode in Go compatible hierarchy that Go bazel rules generate?

balshetzer commented 6 years ago

Yes, I recommend using reflect mode. That's what we use.

junghoahnsc commented 6 years ago

@balshetzer I'm wondering if we can add a flag to overwrite packageImport at https://github.com/golang/mock/blob/master/mockgen/parse.go#L49.

I know you're recommending to use reflect mode, but this would be also convenient when it's not trivial to use the reflect mode.

balshetzer commented 6 years ago

I am trying to go in the other direction and reduce the number of flags. Nobody understands what these flags are for or how to use them. They are also very hacky and have very subtle, hard to explain, effects. How would you even explain what the purpose of this flag is? Would anyone who hasn't read the entire source of mockgen understand what it does?

Instead, I want mockgen to be completely automated without any need for special flags.

The problem you're describing is that your src tree does not conform to what golang requires. I understand and am willing to help. I can even help write the bazel rule. But I don't think providing lots of hooks inside mockgen is the right way to go.