Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mocks cannot be generated from *_test.go files #83

Open
cloudlena opened this issue Nov 30, 2018 · 9 comments
Open

Mocks cannot be generated from *_test.go files #83

cloudlena opened this issue Nov 30, 2018 · 9 comments

Comments

@cloudlena
Copy link

cloudlena commented Nov 30, 2018

I would like to mock an external interface provided by the AWS SDK. To do so, I create a local type alias:

// 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

type DynamoDBAPI = dynamodbiface.DynamoDBAPI

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 package dynamo_test. However, when I do so, moq complains about the interface not being found:

// dynamo_test.go
package dynamo_test

import "github.com/aws/aws-sdk-go-v2/service/dynamodb/dynamodbiface"

//go:generate moq -out ../mocks/dynamodb.go -pkg mocks . DynamoDBAPI

type DynamoDBAPI = dynamodbiface.DynamoDBAPI
$ go generate ./...

cannot find interface DynamoDBAPI
moq [flags] destination interface [interface2 [interface3 [...]]]
  -out string
        output file (default stdout)
  -pkg string
        package name (default will infer)
dynamo_test.go:5: running "moq": exit status 1
@matryer
Copy link
Owner

matryer commented Jan 21, 2019

I'd accept a fix for this if you fancied doing a PR @mastertinner ?

@breml
Copy link
Contributor

breml commented Jan 22, 2019

@mastertinner I did not look into the details, but maybe it is just removing

moq/pkg/moq/moq.go

Lines 81 to 83 in 81c463c

noTestFiles := func(i os.FileInfo) bool {
return !strings.HasSuffix(i.Name(), "_test.go")
}
(or make it optional with a flag).

Edit: updated the link

@baderkha
Copy 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

@matryer
Copy link
Owner

matryer commented Feb 14, 2021

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

@breml
Copy link
Contributor

breml commented Feb 15, 2021

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 (#83 (comment)) is no longer relevant.

After a quick look at the code, I think the relevant spot is

pkgs, err := packages.Load(&packages.Config{
Mode: mode,
Dir: srcDir,
})

where we would need to add Tests: true like this:

pkgs, err := packages.Load(&packages.Config{
	Mode: mode,
	Dir:  srcDir,
	Tests: true,
})

@sudo-suhas
Copy link
Collaborator

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

@sudo-suhas
Copy link
Collaborator

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

@breml
Copy link
Contributor

breml commented Feb 16, 2021

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

@cldmstr
Copy link

cldmstr commented Nov 16, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants