qur / withmock

Automatic Go package mock generation tool
Other
71 stars 9 forks source link

Partial mocking #7

Closed imosquera closed 10 years ago

imosquera commented 10 years ago

Is there a way to do partial mocking with the "withmock" package? If not, have you thought about an approach? I'd love to help to make this happen as I see this library as an essential piece to testing in golang. Let me know.

qur commented 10 years ago

There isn't currently. It is something that I have been thinking about, but I haven't figured out how it would work internally, or how to provide the necessary configuration ...

qur commented 10 years ago

Suggestions of what you might like to be able to do would be helpful.

imosquera commented 10 years ago

It's difficult for me to come up with an approach since the whole package is mocked out and I'm somewhat unfamiliar with the internals of this package.

What we can possibly do when you re-write the classes is assign a variable to each member method. This is an approach used by rog and mentioned in the golang-nuts group right before your post about this library: https://groups.google.com/forum/#!searchin/golang-nuts/stubbing/golang-nuts/6AN1E2CJOxI/CAUDOyQ6bKUJ

I'm actually using a combination of his approach and yours to get full coverage. What do you think?

imosquera commented 10 years ago

any thoughts on my note above?

qur commented 10 years ago

To be honest I don't really understand the applicability. That post is talking about changing the code so that you can patch in different functions - but you don't need to do that if the functions are all mocks anyway.

imosquera commented 10 years ago

The applicability is this:

lets assume you have struct Person that has methods GetName(), GetFirstName() and GetLastName() and GetName()'s implementation is:

func (p *Person) GetName() string { return p.GetFirstName() + p.GetLastName() }

let's also assume GetFirstName and GetLastName have many other depedencies and makes it complicated to load. I'd like to mock out GetFirstName and GetLastName so I dont have to worry about those depdencies.

qur commented 10 years ago

Some partial mocking is now possible on using the runtime_control branch, through the use of pkg.MOCK().EnableMock("Function", "OtherFunction") and pkg.MOCK().DisableMock("Type.Method")

imosquera commented 10 years ago

I'm not sure i get: pkg.MOCK().DisableMock("Type.Method") -- this mean it'll "unmock" the method during the runtime?

At any rate, I'll be trying it shortly.

imosquera commented 10 years ago

I dont have much time to look at this at the time but I get import errors:

imported and not used: "io/ioutil"

imosquera commented 10 years ago

Were you able to reproduce the issue?

qur commented 10 years ago

There's not really enough information to reproduce the issue. What were you trying to mock? I have had some issues with imports, but I thought I had them sorted out.

Also, I forgot to mention that all external imports are now modified, so any non-stdlib package has pkg.MOCK().MockAll(), pkg.MOCK().EnableMock(<...string>) and pkg.MOCK().DisableMock(<...string>) methods. So you should be able to mock a specific function of a package for a single test. Packages that are marked for mocking default to the pkg.MOCK().MockAll(true) state, all others to the pkg.MOCK().MockAll(false) state.

imosquera commented 10 years ago

If i switch to the new branch, compile and use that "mocktest" it fails with the import errors. If i use the master branch all works well. You can re-create this by checking out my project: "go get github.com/imosquera/uploadthis", switch to the "command-org" branch and then get the test depedencies:

go get -v launchpad.net/gocheck

Then run "mocktest ./uploadthis" with the new runtime_control branch to see the errors

Hope this helps

imosquera commented 10 years ago

Okay, I figured this one out. This happens when you have a var which is a func

import "path"

var MyFunc = Func() { path.Join("path1","path2") }

because this gets replaced with

var MyFunc = Func() { panic("!TODO!") }

it doesn't use the imports. The correct behavior now is for me to stop assigning functions to var however I assume other people will be doing this as well and will run into an issue down the road.

qur commented 10 years ago

Whoops. That panic's not supposed to be there any more on the runtime_control branch ...

imosquera commented 10 years ago

Again - thanks for the quick turn-around! This is an essential project for properly mocking/testing in golang and happy to see it progressing.

Question: Is there a way to partially mock the package I'm currently testing? for example: i"m testing the commands package in commands_test.go and want to mock out some methods in the commands package?

qur commented 10 years ago

Not currently. I'm being very careful to not change the code under test, because I'm actually using gocov for all my testing, and the coverage data uses byte offsets into the file.

I have thought about adding a flag to mocktest to enable it though (and make it incompatible with -gocov).

imosquera commented 10 years ago

I'm using gocov too, so that would make it non-usable for me as well. It also appears I should probably be using more interfaces and structs vs package functions.

Side-question - are you interested in a work opportunity? If so please email me at isaac@sharethis.com - we're hiring and looking for talented Go engineers.

qur commented 10 years ago

func literals should now have the real code rather than being replaced by panics now.

imosquera commented 10 years ago

Thanks Julian.

Now when I use the runtime_control branch it creates issue with my existing gomock generated mocks.

.../uploadthis/util/mocks/mocks_mock.go:56: (*MockCommander).EXPECT redeclared in this block previous declaration at ....uploadthis/util/mocks/commands_mocks.go:43

This is because I keep my generated mocks in the util/mocks package using the "mockgen" tool and I believe this is due to the change you mentioned above: "Also, I forgot to mention that all external imports are now modified"

When I go to run the tests it doesn't compile because these methods are being generated again and hence redeclared. I really only need gomock mocks for the package thats currently being tested not external packages. Is it possible to generate them before runtime and place them somewhere in the $GOPATH?

thoughts?

qur commented 10 years ago

Only mocking things marked for mocking is the approach taken originally on the master branch. It has a serious failure case, because you can end up with a single binary using two versions of what should be the same package. You try to pass a value around and get compilation errors say than x is a "foo".Foo is not a "foo".Foo (due to the output rewriting restoring the mocked package name to the original version). Part of the reason for taking the approach of rewriting everything (or rather, all packages in the dependancy chain from the code under test) is that it's the only way to ensure that there is only one version of every package.

I deliberately modelled the API of the generated mocks after that created by mockgen because withmock was intended to be a more powerful replacement (though it currently doesn't actually generate a mock type for an interface, though there is a test for it) ... so compatibility with mockgen output has never been a goal. In fact, one of my aims is actually to be able to throw away all the code we currently have that was generated with mockgen - since it needs to be kept in sync with the product code (which is why this package takes an completely automatic approach).

I could probably not touch any package than imports gomock in non-test code, on the grounds that it must be test support code - and therefore shouldn't need mocking, but I'm not a big fan of tools that try and guess the correct behaviour, but an option to control the skipping would probably be okay.

qur commented 10 years ago

Or perhaps an "-exclude filename", which would prevent any package listed in the specified file from being mocked - even if marked in the test code.

imosquera commented 10 years ago

The -exclude is a good idea just incase we want to never have something mocked, that would work in this case but I'd hate to type that in every time I want to test.

re: In fact, one of my aims is actually to be able to throw away all the code we currently have that was generated with mockgen

If there was a way for me to get access to mocks interfaces, that would be great and wouldn't need to generate mocks at all using mockgen. For instance, If i'm testing package Foo I'd like to have NewMockBarInterface which doesn't have to live in package Foo but is derived from package Foo. Similar to the way I have my mocks now in /util/mocks available to me somehow. Is this possible to do within the goals you've listed above?

re: It has a serious failure case, because you can end up with a single binary using two versions of what should be the same package. You try to pass a value around and get compilation errors say than x is a "foo".Foo is not a "foo".Foo

btw, I currently get this error every now and then on the master branch.

qur commented 10 years ago

I have just pushed the -exclude flag, as it seemed like a sensible control to expose. I would say that it doesn't rate higher than workaround for your specific problem. In fact it may only ever be useful as a workaround - but it's always nice to actually have a workaround ... ;)

My plan is to have code that will write a mock that satisfies an interface. I had been thinking that a package bar that defined an interface Foo would have a new method bar.MOCK().NewFoo() that would return an instance of a generated mock type as a bar.Foo. I just need to figure out how to compute the complete method set of an interface given the information that I have.

For related reasons I am already generating bar.MOCK().Newwibble() which returns a bar.Mock_wibble (where Mock_wibble embeds wibble to inherit it's methods, and only exists to create a public type with the same method set) when I see that wibble has methods defined. The thought behind this being that bar.NewWriter() might return an io.WriteCloser that is actually a *bar.wibble. You can then do:

    ...
    wibble := bar.MOCK().Newwibble()
    bar.EXPECT().NewWriter().Return(wibble, nil)
    wibble.EXPECT().Frob(20).Return(nil)
    ...
imosquera commented 10 years ago

re: My plan is to have code that will write a mock that satisfies an interface. That would be great as long as it's available for the package we're currently testing. If we're testin package bar then having bar.MOCK().NewMockInterface() would be awesome somehow. again, it could even be another package mock_bar.MOCK().NewMockInterface() if you don't want to add code to the existing package.

re: I just need to figure out how to compute the complete method set of an interface given the information that I have. Isn't this what mockgen does already? Could we just take that code?

re: Newwibble() Thats awesome!

anyways-- thanks again. Please let me know if there is anyways I can help

qur commented 10 years ago

I hadn't thought about interfaces in the code under test, but I agree that it's not only possible, but quite desirable. Probably is best to create a new package just to keep the code clean.

It is what mockgen does (in source mode anyway, reflect mode is quite different), but I don't think there's likely to be much code I can reuse. It's not really so much about solving the problem of getting the information, as it is making sure that the code structure makes the information available. In particular I think I might be throwing away some of the information I want because until now I was done with it.

imosquera commented 10 years ago

Sounds good... should we close this ticket then?

qur commented 10 years ago

Not until this stuff is implemented and merged to master ... ;)

imosquera commented 10 years ago

Note: the "-exclude" command line flag conflicts with gocov's command line flag:

"additional -exclude flag, e.g. gocov test -deps -exclude comma,separated,packages."

qur commented 10 years ago

It's not really a conflict, since there is no overlap with the gocov flags. withmock flags have to be specified before the command, and mocktest currently has no way to pass any flags to gocov (other than -v).

imosquera commented 10 years ago

So I've finally hit an impasse with the library because it's incompatible with genmock.

Here is my situation, some of which is described above in various messages but this is in one place:

package bar

type Settings struct{}

type Fooer interface {
     UpdateSettings(s Settings)
}

type Foo struct{}

func(f *Foo)UpdateSettings(s Settings)

But when I go to generate the mocks and place them in another package, lets say mock_bar it remains with:

func(f *MockFoo)UpdateSettings(s Settings)

But the problem is that Settings is part of the bar package and the method signature should be

func(f *MockFoo)UpdateSettings(s bar.Settings)

questions:

  1. Is there a way to get gomock to add the right package bar?
  2. Using "-exclude" won't work here because I would have to place the mocks in their respective package, in this case bar and exclude wouldn't mock the entire package, but thats not the desired effect. yes?

Thoughts?

qur commented 10 years ago

I'm confused. How are you generating the mocks? How are you placing them in another package?

I'm afraid that I don't see what you are doing with genmock or withmock/mocktest.

imosquera commented 10 years ago

Let me further explain:

If I have a package bar with bar.go I would generate the mocks like this:

mockgen -package=mocks -source=./bar/bar.go > util/mocks/bar_mocks.go

But then I get the problem I stated in the previous example, where mockgen doesn't add the package name to the parameters in the method signatures.

If I place the mocks inside of package bar then mocktest has problems because it regenerates the mocks and causes the duplicate symbols error. Hope this helps.

qur commented 10 years ago

Have you tried using the reflection mode of mockgen? That's what I've used and it seems to adjust the types appropriately. If I run

mockgen some/import/path/to/bar Fooer > bar_mocks.go

then I get the output I would expect, with Settings changed to bar.Settings, and the appropriate import statement added.

I'm nearly done with the initial attempt at generating mocks for interfaces in withmock, but it's 02:15 so it'll have to wait 'til tomorrow (technically this evening ... :S).

imosquera commented 10 years ago

Thanks Julian! again, I can't thank you enough for this library. Let me know if you ever want a job =)

imosquera commented 10 years ago

btw, just did the mockgen in relfect mode and it did work. thanks for the tip.

imosquera commented 10 years ago

damn, spoke too soon: have LoadConfig(string, conf.UploadthisConfig) want LoadConfig(string, UploadthisConfig)

At any rate, I'll just wait to see how the generating the mocks for interfaces goes...

qur commented 10 years ago

The first part of interface mock generation (basically interfaces not in the code under test) is now on the runtime_control branch.

So a package that defines an interface Foo will have pkg.MOCK().NewFoo() that returns a *pkg.MockFoo mock instance that implements that interface.

imosquera commented 10 years ago

Okay thanks Julian - I'll go ahead and take a look!

imosquera commented 10 years ago

That seems to work! I've added this project as a way to test without affecting my other project.

https://github.com/imosquera/withmock-examples

you can do a checkout. Use "run.sh -n" to download all the relevant test packages (since go get doesn't pull them automatically for 1.1)

imosquera commented 10 years ago

These changes look good now. Again let me know if there is anyway I can help.

imosquera commented 10 years ago

Julian - do you think you'll be able to generate a mock for the package under test? Is there a way I can help with that piece?

qur commented 10 years ago

That's currently in development. I don't think it should be too hard, it's mostly just figuring out what needs to be done for the generated code to be a valid package in it's own right. Currently I am planning to create a package foo/mocks from package foo with the name foo_mocks - unless you can suggest something better.

Your testing and feedback is invaluable. It's much easier with an immediate need, I would have to refactor a bunch of tests to actually hammer on this stuff right now. Actually - that is an area that could use help, if you can think of extra scenarios (see the scenarios directory at the top of the checkout) to be testing, and send pull requests for them.

imosquera commented 10 years ago

That sounds good. From now on whenever I get a problem instead of reporting it here, or creating another test repo I'll build the scenario in my forked repo of this project and let it fail until the bug is fixed.

Additionally I'd like to help with documentation so we can get more people involved. Am I right to say that mocktest is completely undocumented?

qur commented 10 years ago

Yeah, basically nothing has happened to the documentation since I originally put the code up. My focus has really been on getting to the point where I can actually use it on the real codebase that I want to test. :(

qur commented 10 years ago

So, I think most of the mocking functionality discussed is now available on master. Is there anything you still need that is missing?

Actually, since there is no documentation for the mocks generated from the code under test - it's probably not actually usable right now ... so other than that.

imosquera commented 10 years ago

This is awesome, thanks again!

There is one more thing, concerning EXPECT but i'll put that in a new ticket!

this week i'm working through some other tech issues but i'll make sure to look at the master branch when I get a chance. Thanks!

On Tue, Oct 15, 2013 at 10:30 AM, Julian Phillips notifications@github.comwrote:

So, I think most of the mocking functionality discussed is now available on master. Is there anything you still need that is missing?

Actually, since there is no documentation for the mocks generated from the code under test - it's probably not actually usable right now ... so other than that.

— Reply to this email directly or view it on GitHubhttps://github.com/qur/withmock/issues/7#issuecomment-26355006 .

Isaac Mosquera Mobile, Director, ShareThis e: isaac@sharethis.com m: 703.795.5322

ShareThis is hiringhttp://www.sharethis.com/about/careers/#sthash.DUQgdtQv.dpbs! Check out the openings on our new website.http://www.sharethis.com/#sthash.AWy1FY7t.dpbs

https://www.facebook.com/sharethis https://twitter.com/ShareThis http://www.linkedin.com/company/sharethis http://vimeo.com/sharethis http://pinterest.com/sharethis/ http://instagram.com/sharethis http://www.youtube.com/user/sharethis

imosquera commented 10 years ago

re: since there is no documentation for the mocks generated from the code under test - it's probably not actually usable right now ...

This statement confuses me a bit, do I get mocks for interfaces under test on the master branch? If so, can you give me a small example?

qur commented 10 years ago

Mocks are generated for interfaces in the code under test in a new package "import/path/of/code/under/test/mocks". You have to have your test code in a XXX_test package to use them, as the generated mock package imports the code under test package to get access to the types.

The interface_pkg scenario is the place to look for an example.

imosquera commented 10 years ago

Now that we have to place our test code in another package XXX_test does this mean we can't test private functions & variables?

qur commented 10 years ago

The necessity of using XXX_test only applies when you want to import the mocks generated package. So any file that doesn't import it doesn't need to be in XXX_test. I don't know how the go testing framework feels about having tests in XXX and XXX_test, but I do know that you can have code and no tests in a whatever_test.go file - so you could always have a export_to_test.go file that made private items public for use in the XXX_test package.