petergtz / pegomock

Pegomock is a powerful, yet simple mocking framework for the Go programming language
Apache License 2.0
254 stars 28 forks source link

pegomock generates code with syntax errors #21

Closed godolatry closed 8 years ago

godolatry commented 8 years ago

The option to generate mocks from a source file produces code with syntax errors.

I'm running under Ubuntu Linux. I had this problem with an earlier version of Go, but I'm now running the latest version of Go (1.7). I fetched and built the current version of pegomock this morning.

I have a file Request.go in directory bug defining an interface called Request:

$ cat Request.go package bug

type Request interface { // Foo takes a string and returns a string Foo(str string) string }

With the current directory set to bug, I can build a mock by specifying the package and interface, and that works:

$ pegomock generate bug Request

It produces mock_request_test.go containing correct go. In particular, it produces this code:

 54 func (c *Request_Foo_OngoingVerification) getAllCapturedArguments() (_param0 []string) {
 55         params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams("Foo")
 56         if len(params) > 0 {
 57                 _param0 = make([]string, len(params[0]))
 58                 for u, param := range params[0] {
 59                         _param0[u] = param.(string)
 60                 }
 61         }
 62         return
 63 }

However, if I run the command specying the source code file like so:

$ pegomock generate Request.go

It produces a different file mock_Request_test.go (upper case R) and the result does not compile:

./mock_Request_test.go:57: undefined: _param0 ./mock_Request_test.go:59: undefined: _param0 FAIL bug [build failed]

In this version, getAllCapturedArguments() is:

54  func (c *Request_Foo_OngoingVerification) getAllCapturedArguments() (str []string) {
55      params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams("Foo")
56      if len(params) > 0 {
57          _param0 = make([]string, len(params[0]))
58          for u, param := range params[0] {
59              _param0[u] = param.(string)
60          }

On line 54 the return value of the method is defined as str, but lines 57 and 59 still try to refer to it as _param0.

petergtz commented 8 years ago

Hi @godolatry,

thanks for your bug report. Something looks indeed wrong. I'll have a look at this as soon as possible.

Peter

godolatry commented 8 years ago

No problem. I can just work around the issue by specifying the package and interface name.

Simon Ritchie

petergtz commented 8 years ago

Hi @godolatry,

Can you try again with the latest version? It should work now.

Note that I did not change the casing for filenames (uppercase R in your example), since this seems to be a separate concern. I believe the recommendation is to use lowercase-only filenames in Go, although I can't find a reference where this is defined.

Thanks, Peter

godolatry commented 8 years ago

Happy to do that, but I'm having some difficulties.

I tried

go get github.com/petergtz/pegomock/pegomock

but nothing changed. I guessed that you wanted me to try it before you made that work(?)

I ran

git clone https://github.com/petergtz/pegomock

Now I have a copy of the latest from the master branch, but I can't figure out how to build it. I'm puzzled because there doesn't seem to be a src directory.

How do I build it?

Regards

Simon

On Thu, Sep 1, 2016 at 1:17 PM, Peter Götz notifications@github.com wrote:

Hi @godolatry https://github.com/godolatry,

Can you try again with the latest version? It should work now.

Note that I did not change the casing for filenames (uppercase R in your example), since this seems to be a separate concern. I believe the recommendation is to use lowercase-only filenames in Go, although I can't find a reference where this is defined.

Thanks, Peter

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/petergtz/pegomock/issues/21#issuecomment-244061029, or mute the thread https://github.com/notifications/unsubscribe-auth/AIfV77NXycMJI0s5B_Ej9ed4is4vhPdDks5qlsJOgaJpZM4Jxu2X .

petergtz commented 8 years ago

Try go get -u github.com/petergtz/pegomock/pegomock

That should update the package when it's already there. From the doc:

The -u flag instructs get to use the network to update the named packages and their dependencies. By default, get uses the network to check out missing packages but does not use it to look for updates to existing packages.

godolatry commented 8 years ago

I figured out a way to build it. Almost certainly not the best way, so I'd still appreciate some guidance on that.

The fix works. Thank you kindly for that.

Concerning the file name, that is a bit of a detail, but it's maybe slightly more important than it looks. If you do:

$ pegomock generate bug Request

and then

$ pegomock generate Request.go

under Linux, then you would expect the second command to overwrite the result of the first, but in fact you end up with two files with similar content. If you then run your tests, they will fail:

$ go test can't load package: package bug: case-insensitive file name collision: "mock_Request_test.go" and "mock_request_test.go"

Under Windoze, of course, this won't happen, because mock_request_test.go and mock_Request_test.go are just two alternative names for the same file, and the second command will overwrite the first.

You should probably fix that issue some time in the future, but I agree that it's not urgent.

Regards

Simon

On Thu, Sep 1, 2016 at 1:17 PM, Peter Götz notifications@github.com wrote:

Hi @godolatry https://github.com/godolatry,

Can you try again with the latest version? It should work now.

Note that I did not change the casing for filenames (uppercase R in your example), since this seems to be a separate concern. I believe the recommendation is to use lowercase-only filenames in Go, although I can't find a reference where this is defined.

Thanks, Peter

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/petergtz/pegomock/issues/21#issuecomment-244061029, or mute the thread https://github.com/notifications/unsubscribe-auth/AIfV77NXycMJI0s5B_Ej9ed4is4vhPdDks5qlsJOgaJpZM4Jxu2X .

godolatry commented 8 years ago

That seems to work. Thanks for that.

On a completely different subject, I see that you have specific matchers for strings and other basic objects, but I can't see an "any" matcher. Have I missed something?

Also, when I create my own structures, is it possible to create my own matchers for them?

Regards

Simon

petergtz commented 8 years ago

The problem with your request to lowercase filenames automatically is that this would be unexpected behavior: the simple rule currently is to prefix the source file with mock_ and suffix it with _test. Doing more logic here, would break this simple rule.

Additionally lowercasing it would also break things: imagine you do actually have Request.go and request.go. Generating mocks for them would override files unexpectedly.

My recommendation is to stay out of trouble and follow Go conventions by having all filenames in lowercase.

petergtz commented 8 years ago

Could you please move the discussion about matchers into a separate issue? Thanks.

godolatry commented 8 years ago

Thanks for all that. You can close the issue.