Giter Site home page Giter Site logo

Comments (32)

balshetzer avatar balshetzer commented on August 28, 2024 5

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.

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024 4

Is there any plan to resolve this issue anytime soon?

from mock.

mikhailkolesnik avatar mikhailkolesnik commented on August 28, 2024 3

Why in the world #38 is not merged yet!

from mock.

taybin avatar taybin commented on August 28, 2024 2

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

from mock.

balshetzer avatar balshetzer commented on August 28, 2024 1

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.

from mock.

balshetzer avatar balshetzer commented on August 28, 2024

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

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

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

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

Is there any plan to merge #38?

from mock.

bhcleek avatar bhcleek commented on August 28, 2024

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

from mock.

zbintliff avatar zbintliff commented on August 28, 2024

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?

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

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

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

@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.

from mock.

balshetzer avatar balshetzer commented on August 28, 2024

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.

from mock.

balshetzer avatar balshetzer commented on August 28, 2024

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 $ 

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

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.

from mock.

balshetzer avatar balshetzer commented on August 28, 2024

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?

from mock.

balshetzer avatar balshetzer commented on August 28, 2024

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)
}

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

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.

from mock.

balshetzer avatar balshetzer commented on August 28, 2024

Yes, aux_files should support that.

Can you show me what you tried and what happened?

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

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)
}

from mock.

balshetzer avatar balshetzer commented on August 28, 2024

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?

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

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)
}

from mock.

balshetzer avatar balshetzer commented on August 28, 2024

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

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

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.

from mock.

balshetzer avatar balshetzer commented on August 28, 2024

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.

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

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?

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

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

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

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

from mock.

balshetzer avatar balshetzer commented on August 28, 2024

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.

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

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

from mock.

balshetzer avatar balshetzer commented on August 28, 2024

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

from mock.

junghoahnsc avatar junghoahnsc commented on August 28, 2024

@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.

from mock.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.