petergtz / pegomock

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

Fix a bug when multiple interfaces were parsed #104

Closed anthonyraymond closed 4 years ago

anthonyraymond commented 4 years ago

Fix #103

The loop was writing to a variable out of the scope and the second time it goes through if structName == "" { the condition was never false because the out of scope variable was updated with the first

It resulted in all generated interfaces being named the same (even if the implementation was distinct for each)

petergtz commented 4 years ago

Hey @anthonyraymond,

super cool that you've already fixed the issue you opened. I'm happy to merge it. Before I do that, what do you think, would you be able to also add a test for it? It's probably not super trivial, because all the tests so far simply use the one interface "Display" to do their testing. Adding another one might require a few more changes.

It looks to me like a new test could either go into mockgen_test.go or into main_test.go. The latter might be the simpler one and maybe for that one the changes are even limited to that file.

Curious to get your feedback.

Thanks, Peter

anthonyraymond commented 4 years ago

HI @petergtz, thanks for your answer.

I was going to add a test in the first place, but i don't understood how to compile the project in the first place, so i had to break it appart to kind of adapt it the "module way", that was obviously not the good way to make this project module compliant ^^ Now i've deleted the "hacky" project and i'm kinda lazy to redo all of that.

But if you plan to officially migrate to module i'd be please to add a unit test for this

petergtz commented 4 years ago

Hey @anthonyraymond, sounds reasonable. I'll see if I can make some time to migrate it to modules sometime soon. For now, I'll merge it as is, but could you please change the pull request to go against develop instead of master (sorry, an issue template is also due).

anthonyraymond commented 4 years ago

hi @petergtz , i've just changed the base branch for the PR 😉

petergtz commented 4 years ago

I'll merge it to mainline and make a new release soon (once I have access to a computer again).