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

Fix: avoid package name collision with inPackage (#291) #422

Merged
merged 5 commits into from Apr 25, 2022
Merged

Fix: avoid package name collision with inPackage (#291) #422

merged 5 commits into from Apr 25, 2022

Conversation

i-sevostyanov
Copy link
Contributor

@i-sevostyanov i-sevostyanov commented Nov 17, 2021

Fixes #291.

Tired of waiting, so I copied everything from #402, sorry.

@fterrag
Copy link

fterrag commented Dec 16, 2021

Can we get this PR reviewed, please? Thanks!

@peppelan
Copy link

same here, would be great, thankyousomuchplease

@strideynet
Copy link

@LandonTClipp What's the status of this ? Any reason this isn't being moved forward with ?

@LandonTClipp
Copy link
Contributor

@i-sevostyanov @strideynet sorry for the delay, life gets in the way sometimes, I'm trying to work through a backlog of stuff. Can you write in the description more succinctly what the solution to the original problem is? This is just to aid me so I can know better what I'm looking at.

@i-sevostyanov
Copy link
Contributor Author

I think this issue describes the problem in detail: Invalid mock generated with the same package names 🤓

@serivkos
Copy link

@LandonTClipp I would appreciate if you could look over this request and merge it with master. I’ll be waiting for that. Thanks

@fterrag
Copy link

fterrag commented Apr 18, 2022

We're currently working around this with some sed magic - looking forward to a fix!

Copy link
Contributor

@LandonTClipp LandonTClipp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, just a few minor comments and we can get this merged.

pkg/generator.go Outdated Show resolved Hide resolved
pkg/generator.go Show resolved Hide resolved
pkg/fixtures/example_project/foo/collision.go Outdated Show resolved Hide resolved
Copy link
Contributor

@LandonTClipp LandonTClipp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@codecov-commenter
Copy link

codecov-commenter commented Apr 21, 2022

Codecov Report

Merging #422 (facf60b) into master (58a7f18) will increase coverage by 0.27%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #422      +/-   ##
==========================================
+ Coverage   70.72%   70.99%   +0.27%     
==========================================
  Files           7        7              
  Lines        1291     1293       +2     
==========================================
+ Hits          913      918       +5     
+ Misses        325      321       -4     
- Partials       53       54       +1     
Impacted Files Coverage Δ
pkg/generator.go 91.47% <100.00%> (+0.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58a7f18...facf60b. Read the comment docs.

@LandonTClipp
Copy link
Contributor

@i-sevostyanov sorry one more thing :) the coverage report is showing some missing coverage.

https://app.codecov.io/gh/vektra/mockery/compare/422/changes

Could you add tests for the cases that are missing? PRs generally should be increasing coverage, as a matter of principle.

@i-sevostyanov
Copy link
Contributor Author

Done 🤓

@LandonTClipp LandonTClipp merged commit c9dc740 into vektra:master Apr 25, 2022
@i-sevostyanov i-sevostyanov deleted the fix-package-collision branch April 25, 2022 18:22
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.

Invalid mock generated with the same package names
7 participants