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

Mockery panics when configuration contains a package with a null value and recursive is true #726

Closed
1 of 5 tasks
dlm opened this issue Oct 26, 2023 · 4 comments · Fixed by #730
Closed
1 of 5 tasks
Labels

Comments

@dlm
Copy link

dlm commented Oct 26, 2023

Description

Mockery panics when configuration contains a package with a null value and
recursive: True.

Mockery Version

$ mockery --version
25 Oct 23 18:13 MDT INF couldn't read any config file version=v2.36.0
25 Oct 23 18:13 MDT INF Starting mockery dry-run=false version=v2.36.0
25 Oct 23 18:13 MDT INF Using config:  dry-run=false version=v2.36.0
v2.36.0

I have also confirmed this with v2.32.1

Golang Version

$ go1.21.3 version
go version go1.21.3 linux/amd64

I have also confirmed this with v1.20.4

Installation Method

  • Binary Distribution
  • Docker
  • brew
  • go install
  • Other: [specify]

Steps to Reproduce

Create a project with the following structure:

$ tree
.
├── foo
│   ├── bar
│   │   └── bar.go
│   └── foo.go
├── go.mod
├── main.go
└── .mockery.yaml

Such that foo/bar/bar.go is:

package bar

type Barer interface {
    Bar()
}

And foo/foo.go is:

package foo

type Fooer interface {
    Foo()
}

And go.mod is:

module min-example

go 1.21

And main.go is (The contents of main.go does not matter, but it is helpful
for making sure that the code is correct.):

package main

import (
    "fmt"
    "min-example/foo"
    "min-example/foo/bar"
)

type AFooBar struct{}

func (a AFooBar) Foo() {
    fmt.Println("I am a fooer!")

}

func (a AFooBar) Bar() {
    fmt.Println("I am a barer!")
}

func main() {
    foobar := AFooBar{}
    var fooer foo.Fooer = foobar
    var barer bar.Barer = foobar
    fooer.Foo()
    barer.Bar()
}

And .mockery.yaml is:

all: True
packages:
  min-example/foo/:

From the root we can successfully run with recursive false:

$ mockery --recursive=false showconfig
25 Oct 23 18:20 MDT INF Using config: /home/dave/tmp/mockery-bug-example/.mockery.yaml version=v2.36.0
all: true
packages:
  min-example/foo/: null

If, however, we run with from the root with --recursive=true we get:

$ mockery --recursive=true showconfig
panic: interface conversion: interface {} is nil, not map[string]interface {}

goroutine 1 [running]:
github.com/vektra/mockery/v2/pkg/config.(*Config).mergeInConfig(0xc00012e6c0?, {0xa42830, 0xd8aa40})
        /home/dave/dev/go/pkg/mod/github.com/vektra/mockery/v2@v2.36.0/pkg/config/config.go:691 +0xb65
        github.com/vektra/mockery/v2/pkg/config.(*Config).Initialize(0xc000103d40?, {0xa42830, 0xd8aa40})
                /home/dave/dev/go/pkg/mod/github.com/vektra/mockery/v2@v2.36.0/pkg/config/config.go:124 +0xc5
...

Expected Behavior

From playing around with alternative configs, for example, changing
.mockery.yaml to:

all: True
packages:
  min-example/foo/:
    config:
      tags: ""

I get:

all: true
packages:
  min-example/foo/:
    config:
      all: true
      tags: ""
  min-example/foo/bar:
    config:
      all: true
      tags: ""

So I would guess that the config:

all: True
packages:
  min-example/foo/:

$ mockery --recursive=true showconfig

Should produce something like:

all: true
packages:
  min-example/foo/:
    config:
      all: true
  min-example/foo/bar:
    config:
      all: true

(...maybe?)

But either way, figuring that out seems like the starting point for working on
a fix and CONTRIBUTING.md suggests raising an issue before working on a PR.

Actual Behavior

Mockery crashes (as described above)

@dlm
Copy link
Author

dlm commented Oct 26, 2023

@LandonTClipp I wanted to add that I can to help work on this if you think it is an issue worth addressing. I have found a huge amount of value from this project and would be happy to contribute to it if it would add value.

@LandonTClipp
Copy link
Collaborator

LandonTClipp commented Oct 26, 2023

Damn was hoping no one would notice that 😂

I'm glad to hear you're enjoying the project so much. There is a lot more interesting stuff coming down the pipe. Yes please do send in PRs, although let's see if we can narrow down why this happens.

The config merging code is quite complex but I didn't have a good tool at my disposal that makes this easier. Let's see if we can find where this is happening.

The other thing... showconfig is kind of broken due to a bug noted in another issue (I'm on mobile so can't easily find it now). We should figure out how to fix that because it sometimes hangs indefinitely.

LandonTClipp added a commit to LandonTClipp/mockery that referenced this issue Nov 5, 2023
Fixes issue vektra#726.

We needed an additional bit of logic to ensure that if the `config` section
is nil that we default it to being an empty map.
@LandonTClipp
Copy link
Collaborator

FYI in the linked PR, I fixed the showconfig command and also the issue mentioned above with particular kinds of config causing mockery to crash. Please take a look @dlm when you get the chance, thanks for reporting!

@dlm
Copy link
Author

dlm commented Jan 21, 2024

@LandonTClipp sorry for the slow reply. (Hectic Nov & Dec). I just did the upgrade and it works perfectly!! Thank you very much for getting this resolved!!

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

Successfully merging a pull request may close this issue.

2 participants