error when loading package, can only [...] in module-aware mode

[devnev]

Description

Getting an error about loading packages and module-aware mode when running mockery, see below

Mockery Version

$ grep mockery go.mod go.sum
go.mod:	github.com/vektra/mockery/v2 v2.33.3
go.sum:github.com/vektra/mockery/v2 v2.33.2 h1:znIUwQ3FxnA5jvPy8irYBoiIqMZhuOJhoPOJYNoTJqU=
go.sum:github.com/vektra/mockery/v2 v2.33.2/go.mod h1:9lREs4VEeQiUS3rizYQx1saxHu2JiIhThP0q9+fDegM=
go.sum:github.com/vektra/mockery/v2 v2.33.3 h1:Bahxn87cpbh10fsHhw9671ING/v4dc/53KuLpx0L7I8=
go.sum:github.com/vektra/mockery/v2 v2.33.3/go.mod h1:9lREs4VEeQiUS3rizYQx1saxHu2JiIhThP0q9+fDegM=

Golang Version

$ go version
go version go1.21.0 darwin/amd64

Installation Method

• Binary Distribution
• Docker
• brew
• go install
• Other: go get / go run

Steps to Reproduce

I'm just trying out mockery's config mode and created the following config in the root of my repository, next to the go.mod file

packages:
  github.com/....
    config:
      all: true
      inpackage: true
      include-auto-generated: false
      filename: "mock_{{.InterfaceName}}_test.go"
      recursive: true
      dir: "{{.PackagePath}}"

I've run go get -u github.com/vektra/mockery/v2, and when I run go run github.com/vektra/mockery/v2 I get the following error:

18 Sep 23 17:54 BST ERR encountered error when loading package error="-: can only use path@version syntax with 'go get' and 'go install' in module-aware mode" dry-run=false version=v2.33.3
18 Sep 23 17:54 BST ERR unable to parse packages error="error occurred when loading packages" dry-run=false version=v2.33.3

Unfortunately I don't even know which package it was trying to load, so I'm having difficulty narrowing down the reproduction steps

Expected Behavior

No error

Actual Behavior

See above

[LandonTClipp]

I was thinking that maybe the logging could be improved, but it will be hard to say which package failed to parse because we pass all the packages you specified in packages.Load at once, so it's not like we could attach a single package name to the log key/val.

It's hard to say what's wrong with your config without looking at it. I'm guessing github.com/.... is not what you've actually specified.

[devnev]

The package name in the mockery config exactly matches the package name in the go.mod file. I've tried using the exact same config in a barebones project with one interface but didn't get the same issue (but also it didn't actually do what I was expecting it to do, I somehow need to trim the module package prefix off the directory template). In my barebones project the config is

packages:
  github.com/devnev/test-mockery-conf:
    config:
      all: true
      inpackage: true
      include-auto-generated: false
      filename: "mock_{{.InterfaceName}}_test.go"
      recursive: true
      dir: "{{.PackagePath}}"

[devnev]

Changed dir to dir: "{{.InterfaceDir}}" - it does what I want in the barebones project, thanks for the docs - still getting the same issue in the real repository though.

[devnev]

Looking at the code,

	packages, err := packages.Load(&p.conf, packageNames...)
	if err != nil {
		return err
	}
	for _, pkg := range packages {
		for _, err := range pkg.Errors {
			log.Err(err).Msg("encountered error when loading package")
		}

It looks like the error could include pkg.PkgPath?

[devnev]

I've forked mockery to make above change to logging. The "package" causing the issue was actually a node_modules directory containing a directory starting with @, node_modules/@pulumi/... which also for some reason turned into package path node_modules@pulumi. I will need to exclude node_modules and presumably anything else in .gitignore.

[LandonTClipp]

@devnev very interesting, thanks for taking the time to dig into it. This is a strange edge case I've never seen before.

Perhaps the exclude parameter being kindly implemented in #709 would help solve your problems? Sorry for the trouble, packages is still kind of a new feature so weird things like this are bound to happen.

[devnev]

Weirdly, go list ./... doesn't include the node_modules directory, maybe there's some logic there that could be adopted?

[LandonTClipp]

So, the only way I can envision this would happen is if node_modules/@pulumi has any .go files somewhere beneath. Could you confirm that? The logic is to just recursively iterate through the root directory and find any .go files. Those paths that contain .go files are then passed to packages.Load. Maybe that's naive, I should look more into how go list ./... does it.

[devnev]

Confirmed, it contains a go module (go.mod/go.sum) and a main.go file. Maybe the presence of the go module is why go list ./... doesn't show it?

[devnev]

If I rename the go.mod and go.sum files, go list ./... also produces a (slightly different) error, pattern ./...: directory pulumi/node_modules/@pulumi/aws/node_modules/@pulumi/pulumi/tests/automation/data/testproj outside main module or its selected dependencies

[devnev]

The exclude parameter from #709 certainly seems like a sufficient and straightforward way of solving this particular problem, much simpler than trying to answer whether the recursive option should match its behaviour with go list ./....

It'll be interesting to see if it ends up growing into something very gitignore-like, or if there's interest in something like an exclude-file option such that .gitignore can be used directly.

[devnev]

Just trying go list ./... | xargs -n1 go run github.com/vektra/mockery/v2 --srcpkg and I'm instead getting FTL Cannot specify --filename or --structname with --all dry-run=false version=v2.33.3

[devnev]

How does (or should) mockery treat directories named testdata? That would be another case where Go's loading has additional logic.

[devnev]

I just realised the exclude from #709 is entirely prefix-based - not quite what I was expecting. So I can't just add node_modules and .venv to the exclude list.

[LandonTClipp]

Well, we already have an include-regex so maybe it is time to have an exclude-regex? Having exclude be prefix-based is a good enough solution for 90% of the cases I'm guessing.

I would have to think about the idea of parsing .gitignore. My reaction is that it's probably fine but I want to know why the currently existing options wouldn't suffice.

[devnev]

TBF, I'm still trying to work out how best to migrate to the config mode, so the approach might change. I can't even figure out if its better to have as much as possible in the central config and run mockery centrally, or have configs more distributed and close to the interfaces/packages/code and run mockery for each config/package.

[LandonTClipp]

I can't even figure out if its better to have as much as possible in the central config and run mockery centrally, or have configs more distributed and close to the interfaces/packages/code and run mockery for each config/package.

Well that's not a totally crazy way of doing it (one config for each package). It won't be as efficient as one config for the entire project because otherwise mockery would have to redundantly parse all of your deps multiple times. Calling out to packages.Load once (or in other words, only one mockery subprocess being run anywhere) will be the fastest way in general.

[devnev]

If a config file for the entire project is preferable, I definitely need a fix for this issue to continue

[devnev]

Well, we already have an include-regex so maybe it is time to have an exclude-regex? Having exclude be prefix-based is a good enough solution for 90% of the cases I'm guessing.

I would have to think about the idea of parsing .gitignore. My reaction is that it's probably fine but I want to know why the currently existing options wouldn't suffice.

regex and gitignores are both sources of complexity, matching the go list behaviour might allow for a more sensible default behaviour without extra config?

[LandonTClipp]

So just to recap, it seems like the source of your problem is that mockery is recursing into paths that contain go.mod files. I think we can solve your goal in two ways:

1. Add logic in the directory recursion code to ignore paths with go.mod
2. Additionally, ignore paths with .mockery.yaml in them. This would allow you to create a separate config file per service and mockery will by default understand that it should not recurse into paths that have separate config.

Both of these will be extremely easy to add, it's probably line 8 lines of code. Let me know if you think this would help solve your problem.

[devnev]

Does a .mockery.yaml in a subdirectory override behaviour when it is found via recursion? If not, then yes I think it has to stop when a .mockery.yaml is found. If the recursion takes these discovered .mockery.yamls into account, I think it should continue as per the resulting configuration.

The logic for stopping at directories with go.mod seems sound as it clearly has potential to break when one is present.

[LandonTClipp]

Does a .mockery.yaml in a subdirectory override behaviour when it is found via recursion? If not, then yes I think it has to stop when a .mockery.yaml is found. If the recursion takes these discovered .mockery.yamls into account, I think it should continue as per the resulting configuration.

Theoretically, mockery could be modified to override its config in-place (in fact, it already does this when discovering recursive packages. The discovered packages are injected into the config map just as if you had manually specified them). But, merging together multiple yaml files is really tricky and hasn't been implemented anyway. So the safest thing is to probably just stop the recursion.

The subdirectory exclusion logic would need to be added here: https://github.com/vektra/mockery/blob/v2.36.0/pkg/config/config.go#L548-L574 . It would be very easy. If you have the time for this, I'd be happy to accept a PR. Otherwise I'll leave this for someone else to implement (or for myself, if I find myself having spare time).

[devnev]

I'm giving it a go, but using pathlib.Walker is making it more complex as unlike stdlib walk it visits children before visiting the directory, so there's no easy way of preventing recursion. The pathlib walker just generally seems overcomplicated, would it be OK to drop in in favour of stdlib filepath.WalkDir?

[LandonTClipp]

Hmm, I understand why pathlib doesn't meet the requirements here. The Basic walker doesn't guarantee any kind of order but in this case we need an explicit BFS. For the sake of getting this fixed, yes feel free to swap with the filepath walker.

[devnev]

We don't even need BFS, just pre-order DFS that allows culling instead of unconditional post-order DFS.

[devnev]

Raised a PR for the submodules. A general exclude mechanism (that doesn't require the full prefix) would still be of interest, as a repository-wide mockery run will encounter directories it just isn't intended to recurse into (like node_modules).