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

gomock_generator. Add --source-path option #930

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions gomock_generator/gomockgenerator/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ type GenerationConfiguration struct {
ConcurrentGoroutines int
// NoGoMod by default we'll consider go modules is enabled, mockgen will be called with -mod=mod to read interfaces in modules instead of default GOPATH
NoGoMod bool
// SourcePath override the output base path of the mocks
SourcePath string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: I'm not sure I understand the choice of words here for the variable name. It mentions Source but it seems to be the location of what this program outputs. What do you think about MocksPath or MocksRootPath?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not precious about the naming... but it is the location of your codebase. MocksRootPath works. I'll update it.

}

// MocksConfiguration contains the configuration of the mocks to generate.
Expand Down Expand Up @@ -67,7 +69,12 @@ func GenerateMocks(ctx context.Context, gcfg GenerationConfiguration, mocksCfg M
if mocksCfg.BaseDirectory == "" {
mocksCfg.BaseDirectory = mocksCfg.BasePackage
}
err = os.Chdir(path.Join(os.Getenv("GOPATH"), "src", mocksCfg.BaseDirectory))

mocksPath := path.Join(path.Join(os.Getenv("GOPATH"), "src"), mocksCfg.BaseDirectory)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: path.Join receives variadic arguments

Suggested change
mocksPath := path.Join(path.Join(os.Getenv("GOPATH"), "src"), mocksCfg.BaseDirectory)
mocksPath := path.Join(os.Getenv("GOPATH"), "src", mocksCfg.BaseDirectory)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. indeed.

if gcfg.SourcePath != "" {
mocksPath = gcfg.SourcePath
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: IMO the check with the argument SourcePath should only be done once. The fact that you need to do the check over and over again is a sign that there is a lack of optimization somewhere. Doing this check here, at the beginning of the mocks generation is ok. Doing it multiple times makes me feel like something is wrong. Can you have a look to optimize this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I'll deal with this.

err = os.Chdir(mocksPath)
if err != nil {
return errors.Wrap(err, "fail to move to base package directory")
}
Expand All @@ -79,7 +86,10 @@ func GenerateMocks(ctx context.Context, gcfg GenerationConfiguration, mocksCfg M
}).Infof("Generating %v mocks", len(mocksCfg.Mocks))

var mockSigs map[string]string
mockSigsPath := path.Join(os.Getenv("GOPATH"), "src", mocksCfg.BaseDirectory, gcfg.SignaturesFilename)
mockSigsPath := path.Join(gcfg.SourcePath, mocksCfg.BaseDirectory, gcfg.SignaturesFilename)
if gcfg.SourcePath != "" {
mockSigsPath = path.Join(gcfg.SourcePath, gcfg.SignaturesFilename)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: from what I understand, it shouldn't be needed at this point to still have a condition. mocksPath contains the value we need.

Suggested change
mockSigsPath := path.Join(gcfg.SourcePath, mocksCfg.BaseDirectory, gcfg.SignaturesFilename)
if gcfg.SourcePath != "" {
mockSigsPath = path.Join(gcfg.SourcePath, gcfg.SignaturesFilename)
}
mockSigsPath := path.Join(mocksPath, gcfg.SignaturesFilename)


sigs, err := os.ReadFile(mockSigsPath)
if os.IsNotExist(err) {
Expand Down Expand Up @@ -168,7 +178,10 @@ func generateMock(ctx context.Context, gcfg GenerationConfiguration, baseDirecto
mock.SrcPackage = path.Join(basePackage, mock.SrcPackage)
}

mockPath := filepath.Join(os.Getenv("GOPATH"), "src", baseDirectory, mock.MockFile)
mockPath := filepath.Join(gcfg.SourcePath, baseDirectory, mock.MockFile)
if gcfg.SourcePath != "" {
mockPath = path.Join(gcfg.SourcePath, mock.MockFile)
}
log = log.WithFields(logrus.Fields{
"mock_file": mock.MockFile,
"interface": mock.Interface,
Expand Down Expand Up @@ -198,7 +211,11 @@ func generateMock(ctx context.Context, gcfg GenerationConfiguration, baseDirecto
mockSrcPath := strings.Replace(mock.SrcPackage, basePackage, baseDirectory, -1)

hashKey := fmt.Sprintf("%s.%s", mockSrcPath, mock.Interface)
hash, err := interfaceHash(mockSrcPath, mock.Interface)
fullPath := path.Join(os.Getenv("GOPATH"), "src", mockSrcPath)
if gcfg.SourcePath != "" {
fullPath = path.Join(gcfg.SourcePath, strings.ReplaceAll(mock.SrcPackage, baseDirectory, ""))
}
hash, err := interfaceHash(fullPath, mockSrcPath, mock.Interface)
if err != nil {
return "", "", errors.Wrapf(err, "fail to get interface hash of %v:%v", mock.SrcPackage, mock.Interface)
}
Expand Down
8 changes: 3 additions & 5 deletions gomock_generator/gomockgenerator/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@ import (
"go/parser"
"go/token"
"os"
"path"
"path/filepath"
"strings"

"github.com/pkg/errors"
)

func interfaceHash(pkg, iName string) (string, error) {
sig, err := interfaceSignature(pkg, iName)
func interfaceHash(fullPath, pkg, iName string) (string, error) {
sig, err := interfaceSignature(fullPath, pkg, iName)
if err != nil {
return "", errors.Wrapf(err, "fail to get interface signature for %s:%s", pkg, iName)
}
Expand All @@ -26,12 +25,11 @@ func interfaceHash(pkg, iName string) (string, error) {
return fmt.Sprintf("% x", hash), nil
}

func interfaceSignature(pkg, iName string) (string, error) {
func interfaceSignature(fullPath, pkg, iName string) (string, error) {
cwd, err := os.Getwd()
if err != nil {
return "", errors.Wrap(err, "fail to get current working directory")
}
fullPath := path.Join(os.Getenv("GOPATH"), "src", pkg)
vendoredPkg := filepath.Join(cwd, "vendor", pkg)
if _, err := os.Stat(vendoredPkg); err == nil {
fullPath = vendoredPkg
Expand Down
3 changes: 3 additions & 0 deletions gomock_generator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Reads the mymocks.json file from the current directory and generates the mocks,
app.cli.Flags = []cli.Flag{
cli.StringFlag{Name: "mocks-filepath", Value: "./mocks.json", Usage: "Path to the JSON file containing the MockConfiguration. Location of this file is the base package.", EnvVar: "MOCKS_FILEPATH"},
cli.StringFlag{Name: "signatures-filename", Value: "mocks_sig.json", Usage: "Filename of the signatures cache. Location of this file is the base package.", EnvVar: "SIGNATURES_FILENAME"},
cli.StringFlag{Name: "source-path", Value: "", Usage: "Path to the local source code", EnvVar: "SOURCE_PATH"},
cli.IntFlag{Name: "concurrent-goroutines", Value: 4, Usage: "Concurrent amount of goroutines to generate mock.", EnvVar: "CONCURRENT_GOROUTINES"},
cli.BoolFlag{Name: "debug", Usage: "Activate debug logs"},
}
Expand All @@ -66,6 +67,7 @@ VERSION:
{{end}}
`
app.cli.Before = func(c *cli.Context) error {
app.config.SourcePath = c.GlobalString("source-path")
app.config.MocksFilePath = c.GlobalString("mocks-filepath")
app.config.SignaturesFilename = c.GlobalString("signatures-filename")
app.config.ConcurrentGoroutines = c.GlobalInt("concurrent-goroutines")
Expand All @@ -87,6 +89,7 @@ VERSION:
"mocks_file_path": app.config.MocksFilePath,
"signatures_filename": app.config.SignaturesFilename,
"concurrent_goroutines": app.config.ConcurrentGoroutines,
"source-path": app.config.SourcePath,
}).Info("Configuration for this mocks generation")

rawFile, err := os.Open(app.config.MocksFilePath)
Expand Down