Open cloudlena opened 6 years ago
I'd accept a fix for this if you fancied doing a PR @mastertinner ?
@mastertinner I did not look into the details, but maybe it is just removing https://github.com/matryer/moq/blob/81c463c9b96c2edb0be1f6a462928a286b7379f9/pkg/moq/moq.go#L81-L83 (or make it optional with a flag).
Edit: updated the link
you usually don't mock an entire db interface . you just need better architecture at that point . Ie hide that querying logic in the repository and mock the repository, your repository would be an interface that has the stuff your app uses ie findAllUsers .. things like that . you shouldn't really be mocking code you don't own . that's my two cents
@sudo-suhas What do you think about this? I feel like moq
shouldn't prevent this, and leave it as a decision to the user?
I also think, moq
should not prevent this use case.
The new code, works differently in regards to loading the package information, so the hint from above (https://github.com/matryer/moq/issues/83#issuecomment-456559011) is no longer relevant.
After a quick look at the code, I think the relevant spot is https://github.com/matryer/moq/blob/126389e52f4542cd0a61962ba6f5ecda0b3e15fe/internal/registry/registry.go#L124-L127
where we would need to add Tests: true
like this:
pkgs, err := packages.Load(&packages.Config{
Mode: mode,
Dir: srcDir,
Tests: true,
})
Yes, I also agree that moq
shouldn't prevent this. However, I am not sure if the changes required are quite as simple. If we set Tests: true
while loading packages, we would get back 2 packages and error out on this line: https://github.com/matryer/moq/blob/v0.2.1/internal/registry/registry.go#L134-L136
A possible workaround is to use a private interface definition:
// dynamo.go
package dynamo
import "github.com/aws/aws-sdk-go-v2/service/dynamodb/dynamodbiface"
//go:generate moq -out ../mocks/dynamodb.go -pkg mocks . dynamoDBAPI:DynamoDBAPI
type dynamoDBAPI = dynamodbiface.DynamoDBAPI
@sudo-suhas That is an interesting tweak with the alias.
If one is using such a type alias (as described in the initial issue), I think it would still be better, if it could be placed in the _test
package.
Because I was in the area where this issue would be solved, I had a quick look. I think it would require a bit of refactoring and added complexity to make this work:
For one the registry would need to keep references to and iterate over several packages to get the type and interface definitions. Currently, the Registry keeps just the loaded package:
type Registry struct {
srcPkg *packages.Package
https://github.com/matryer/moq/blob/v0.2.1/internal/registry/registry.go#L36
The main question in my mind would be, how do you handle interfaces or types with the same name in the test and original package when returning the definitions and doing the lookups. The answer to this question would probably also be dependent on the package the mock is being generated into, i.e. do you prefer the _test
package definitions when generating into the _test
package and vice-versa?
Also, running packages.Load(..)
with Tests: true
at https://github.com/matryer/moq/blob/v0.2.1/internal/registry/registry.go#L124-L127
returns 3 packages and not 2 as expected. <package>
, <package>_test
, and <package>.test
. According to the docs the last one is the package compiled for tests. This makes answering the questions about disambiguation just a bit more complex.
I currently started working with @cloudlena who started this issue and we are using the workaround suggested above and are quite happy with that solution.
TL;DR: I'm not sure if the added flexibility would be worth the effort.
I would like to mock an external interface provided by the AWS SDK. To do so, I create a local type alias:
However, this increases the API surface of my
dynamo
package by adding another public interface to it which can and should not be used except for mocking. To mitigate that, I would like to move the code above into the respective testing packagedynamo_test
. However, when I do so, moq complains about the interface not being found: