vektra / mockery

A mock code autogenerator for Go
https://vektra.github.io/mockery/
BSD 3-Clause "New" or "Revised" License
5.95k stars 402 forks source link

Mock external package #243

Closed oke11o closed 1 year ago

oke11o commented 5 years ago

How I can to mock an external package interface? For example, github.com/patrickmn/go-cache

sagikazarmark commented 5 years ago

Generally speaking, it's a bad idea to mock something that you don't own. A better approach is introducing an interface of your own.

KrzysztofMadejski commented 5 years ago

I think it's a great idea to mock something I don't own, ie. connection to S3 or SFTP. Even main example contain that (https://github.com/jaytaylor/mockery-example/blob/master/mocks/S3API.go), but there is no documentation how to run mockery to get it.

See #210

sagikazarmark commented 5 years ago

ie. connection to S3 or SFTP

The problem with that is you don't actually test anything that way. If you mock an external dependency, you basically limit testing to calling the right functions in the right order.

This is especially true for S3 or SFTP: the use of external resources should be integration tested. Otherwise you don't actually know that your code works. You just know that an arbitrary interface is called with the right arguments. Actually, you can't even say that, because mocks cannot verify that your input parameters are correct or not.

Compared to mocking external resources, if you create your own interface, you can test the consuming code using mocks. If the interface is yours, chances are the implementations are too. The implementations can be integration tested.

The additional advantage of a custom interface is that your code does not have any external dependencies.

I know people like mocking external resources. It's extremely comfortable...but it doesn't make it a good idea.

Mocking was originally invented around extreme programming and it's sole purpose was making interface design easier: you write the consuming code against an unimplemented interface, that you design on the fly and create a dummy implementation with a mocking tool.

HaywardMorihara commented 5 years ago

Actually, you can't even say that, because mocks cannot verify that your input parameters are correct or not.

You should be able to verify that input parameters are correct on mocks, right? For instance with testify's AssertExpectations()

Might be getting a bit off topic, but when do you suggest using mocks? Just for interface design? My understanding was that you should use them to stub out dependencies so you can write independent unit tests (while also have integration test coverage that doesn't use mocks, as you say).

sagikazarmark commented 5 years ago

You should be able to verify that input parameters are correct on mocks, right?

No, that only accounts for the number of parameters and type safety, but not for semantic correctness (eg. data validation).

Yes, in my opinion mocks should primarily used for interface design. I tend to refactor tests to use actual implementations later (for example, in case of a persistence store for my service I start with a mock and once my service is ready and tested, I write an in-memory store implementation and refactor the tests to use this implementation (which is also covered by tests)) or at least write a stub.

There are other cases where mocks might be useful. The main difference between mocks and other test doubles (like stubs) is the fact that mocks verify the behavior (besides the fact that mocks are usually created/generated by mock frameworks while stubs are usually written by hand) and sometimes it's exactly what you need. For example, when you have a very thin integration between two layers of code, it doesn't really make sense to have semantic correctness of the data and the behavior, because that intermediate layer has nothing to do with it. Go-kit's endpoint layer between services and transports is a great example for that.

To be completely fair, we are talking about two questions here:

Whether you use mocks or stubs or whatever test doubles for your own interfaces is completely your choice. It's worth keeping in mind that mocking was created for interface design, but a test with a mock is better than no test at all.

The answer to the second question is no. That's something that both the "mockist" and the "classical" TDD camps agree with.

KrzysztofMadejski commented 5 years ago

I found a nice read on creating minimal interfaces next to your code that consumes them. https://mycodesmells.com/post/accept-interfaces-return-struct-in-go I'm convinced by these and your @sagikazarmark arguments.

I agree that you should do integration tests also.

On original question: just run mockery in relevant vendor folder using -dir

HaywardMorihara commented 5 years ago

@sagikazarmark thank you for elaborating!

Agronis commented 4 years ago

@oke11o fwiw, I combine go generate in some instances..

//go:generate mockery -dir $GOPATH/github.com/patrickmn/go-cache -output $GOPATH/my/project

LandonTClipp commented 4 years ago

FYI what I typically do to work around this is create an internal interface that copies the necessary external interface. For example:

import "github.com/kewl/repo"

// Above package exports the interface "Foo"
type RepoFoo interface {
    repo.Foo
}

This way you will get an exact copy of repo.Foo interface in your project and mocks. It's not a great solution but it works for now.

I do want to officially support automatically creating mocks in external repos, however.

nmiyake commented 4 years ago

I've been thinking about this problem, and in the abstract I think it could be solved pretty cleanly if inputs were considered in the form of packages rather than directories. Since this tool existed from the GOPATH days, the construction of considering input in the form of file paths (--dir) made pretty good sense. However, in a module world, I think there would be some nice properties coming from considering inputs in the form of packages rather than physical directories.

If the tool receives package paths, it can pass them directly to packages.Load and depend on that call to figure out the specifics of where the files for the package are located (local to project, in local vendor directory, in local Go module cache) and run based on that. This would be as opposed to the current logic, which has mockery transform the input arguments to file paths and then uses pkgs, err := packages.Load(&p.conf, "file="+fpath).

In a mode like this, mockery could be supplied with input packages and it wouldn't need to make any distinction between "internal" or "external" packages -- "k8s.io/client-go/kubernetes/typed/core/v1" and "./myPkg" would just simply be loaded by the packages.Load call (which already handles things like determining whether the Go files for that package come from the vendor directory or package cache etc.).

I think a valid concern here is that adding a "package mode" to the current mode would be a pretty big bifurcation, and changing all of the current logic to use package paths rather than file paths for current flags (like --input) would likely introduce breaks in some cases (although I think a majority of cases would continue to work, since relative paths are valid package paths).

LandonTClipp commented 4 years ago

in the abstract I think it could be solved pretty cleanly if inputs were considered in the form of packages rather than directories.

I think that's actually a wonderful idea. You are correct that mockery did not have the niceties of the module world, so now we are stuck with all these issues that are tricky to solve. I will add this as a TODO to the v3 project.

I think a valid concern here is that adding a "package mode" to the current mode would be a pretty big bifurcation, and changing all of the current logic to use package paths rather than file paths for current flags (like --input) would likely introduce breaks in some cases (although I think a majority of cases would continue to work, since relative paths are valid package paths).

I'm of the feeling that we really shouldn't support old technology. Modules are the future so we should exclusively support that. I am curious how much work it would be to remove dependency on the file= argument to packages.Load? Could we move over to this with minimal effort?

nmiyake commented 4 years ago

Thanks for the feedback! I think the amount of work largely depends on the design of how the packages logic interacts with the other pieces of mockery and the current set of flags.

If you literally just replace the current packages.Load call from a file to a package, the work needed is probably pretty low. However, I think this also loses out on some of the possible benefits -- for example, currently the "Walker" walks a directory structure manually and calls packages.Load on every matching path, which I think is actually pretty wasteful/expensive.

I think the cleaner approach would be to really rearchitect the design here -- the current design is pretty heavily predicated on starting with the inputs as paths and then doing manual walking and package loading based on the filesystem. I think it would actually be much cleaner to rework this to load fully based on packages (so for example, if we wanted to generate for all ./pkg, rather than walking that manually we could just load ./pkg/...) and go from there.

Would be curious to hear your thoughts and general philosophy here. I've been considering writing my own generator to use the approach described above, with the thought that it would give me more freedom to use this architecture without necessarily having to support all of the current flags/configurations that I assumed mockery would need/want to support for backcompat purposes. However, if this is something that you'd be directionally amenable to for a v3, would be happy to sketch out a proposal for what I think that would look like for feedback.

benjaminclauss commented 2 years ago

@sagikazarmark

This is especially true for S3 or SFTP: the use of external resources should be integration tested. Otherwise you don't actually know that your code works. You just know that an arbitrary interface is called with the right arguments. Actually, you can't even say that, because mocks cannot verify that your input parameters are correct or not.

Compared to mocking external resources, if you create your own interface, you can test the consuming code using mocks. If the interface is yours, chances are the implementations are too. The implementations can be integration tested.

Could you please elaborate here and perhaps elucidate with an example?

For example, the AWS SDK has a s3iface.

Package s3iface provides an interface to enable mocking the Amazon Simple Storage Service service client for testing your code.

Are you proposing, if I have some code that interacts with S3, I create my own interface with the methods I use on an actual s3.S3 (similar to @LandonTClipp above)?

If so, then why would the SDK provide this interface? Likewise, when testing, why not generate a mock using this interface?

sagikazarmark commented 2 years ago

@benjaminclauss sure!

I think S3 is a great example, for several reasons.

From a consumer perspective, the problem with relying on external interfaces is that they don't (or rarely) capture intent of the consumer, so they aren't really abstractions. For example, consumers rarely want to "put an object to S3", they want to save some data related to their business processes. If you want to launch your application on Google Cloud, now you are stuck with an interface that maybe you can implement for GCS, but will be completely unnatural.

Another problem with interfaces in general is that they can't express behavior. They don't express how input is validated, they don't express what errors are returned. This is why it's generally a good idea to create abstractions owned by consumers: you can make sure that the implementations adhere behavioral requirements that you cannot represent in the interface itself.

Now let's say you've built a nice abstraction around S3 for your business code:

type Storage interface {
    Save(path string, data []byte) error
    Get(path string) ([]byte, error)
}

You can now write tests for your code against an interface that you own, but you still need to create an s3 implementation and test that implementation:

type s3Storage struct {
    s3 s3.S3 // you could use an interface here
}

func (s s3Storage) Get(path string) ([]byte, error) {
    // I simplified it a bit
    s.s3.GetObject(path)
}

You could very well argue at this point that you could just unit test s3Storage by mocking the interface. But it actually suffers from the same problem I mentioned earlier: it doesn't capture behavior.

For example, let's say GetObject expects the directory separator to be a colon or a pipe. Is your test going to fail if you mock the GetObject method? Not, it isn't. It's going to pass and when you manually test the code, you will realize that the unit test written for s3Storage simply can't cover this, because you don't actually test the behavior of the code, you test if a method is being called with a specific parameter. One could argue this is a useless test though.

But let's say you know that path needs to be colon separated and you write code, that transforms slashes to colons.

What happens when the implementation changes? Eg. you can't use GetObject for objects larger than 5MB anymore, you have to use GetLargeObject for that. You probably won't even notice that during manual tests. You might even go to production with that change and then a customer calls a week later complaining about errors.

I hope these two examples demonstrate why unit testing external dependencies is basically useless: you can't test their behavior.

Integration and e2e tests are way more useful for these components, because they test actual behavior and can catch errors that unit tests with mocked dependencies can't.

You might have heard from others: "test behavior, not the implementation". Well, in this case a unit test+a mock is exactly that: testing implementation, making sure that your little wrapper calls the right method of s3.S3 with the (perceived) right parameters.

I'd like to point out again, that mocking is an interface design tool from the XP era: it's mostly useful when you are designing a storage interface for instance, while writing the business code (and tests for it). And the thing is: they are not even that useful in Go as you have to actually define the interface first in order to it to compile.

For example, the AWS SDK has a s3iface.

I believe they actually dropped the interface in the v2 SDK. You can define your interfaces, because of the implicit interface implementation, but in other languages that might not be available.

What do you think?

benjaminclauss commented 2 years ago

I apologize for the delay.

If you want to launch your application on Google Cloud, now you are stuck with an interface that maybe you can implement for GCS, but will be completely unnatural.

This has bit me before. 😃

you will realize that the unit test written for s3Storage simply can't cover this, because you don't actually test the behavior of the code, you test if a method is being called with a specific parameter.

This really hit the nail on the head.

matthiasbruns commented 2 years ago

What's the status here?

eugeneradionov commented 2 years ago

Found an option --srcpkg string source pkg to search for interfaces

//go:generate mockery --srcpkg=github.com/user/pkg/subpkg --name=Interface

cc @matthiasbruns

LandonTClipp commented 1 year ago

For folks in this ticket: this problem can obviously be resolved by using --srcpkg, but keep in mind that the packages feature will allow really simple semantics since mocks are defined based on package name, which can be anything in your own repo or anything in the go ecosystem. I'm closing this ticket since technically this feature has already been implemented.

If there are any confusions in the official docs, please submit a PR and we can have it added.