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

Added InterfaceFile #773

Merged
merged 3 commits into from Apr 29, 2024
Merged

Added InterfaceFile #773

merged 3 commits into from Apr 29, 2024

Conversation

istrau2
Copy link
Contributor

@istrau2 istrau2 commented Apr 17, 2024

Description

Added InterfaceFile to available fields in .mockery.yaml template

I would like to name the mock file similarly to the interface file. Currently there is no way to know the interface file name in the template.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • [x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

@LandonTClipp
Copy link
Contributor

Hey, thanks for the PR! This parameter would be potentially misleading because you might have multiple interfaces within a single file. People have asked this before and I mention it only works in the case where you have one interface per file.

@istrau2
Copy link
Contributor Author

istrau2 commented Apr 18, 2024

@LandonTClipp Thanks for the quick response.

I understand the concern, however, as far as this package is concerned I don't see how this is misleading. It is in fact the name of the file that the interface is in. If the consumer has decided to name their mock files based on the original name, they should ensure that there is only one interface in each file. Otherwise, as I understand, it would be overwritten, that seems fairly clear (and frankly, would be immediately observed since the tests would break).

For us, we have been happily using mockery for a couple of years now. Recently we've been getting deprecation warnings and I've been trying to upgrade to the new configuration. We do in fact have one interface per file and use a file name like interface_file_mock.go alongside the original interface_file.go all throughout the codebase. I would like to upgrade in a backwards compatible manner (rather than having to rename/restructure the codebase).

Perhaps a warning is in order if two interfaces share the same file name (although I am not sure how to implement that)?

Thanks again

@LandonTClipp
Copy link
Contributor

LandonTClipp commented Apr 22, 2024

I've thought about this a bit more and in order to help you migrate to the new config, I think we can go ahead with it. My main concern is that it causes confusion without making an explicit note in the docs about what the behavior is when multiple interfaces are in a single file. I will just make a note in the docs to make this clear.

The other consideration I am making is that I will, at some point, implement supporting multiple interfaces per mock file, and the intention was to use this exact config parameter in this PR to support that. I can't conceive of any issues with introducing multi-interface support into projects that make use of this parameter. It might cause mocks to magically appear where they otherwise would not, but again, making this clear in the docs is the right way forward.

@LandonTClipp
Copy link
Contributor

@istrau2 could you please add a test for this to ensure that the rendered template matches what we expect? Once you get that, I can approve.

@istrau2
Copy link
Contributor Author

istrau2 commented Apr 23, 2024

@LandonTClipp Much appreciated, thanks a lot. I have added the test.

@istrau2
Copy link
Contributor Author

istrau2 commented Apr 27, 2024

@LandonTClipp thoughts?

@LandonTClipp
Copy link
Contributor

Looks great!

@LandonTClipp LandonTClipp merged commit 25d2eb0 into vektra:master Apr 29, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants