-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
chore: Automate mock generation #11391
Conversation
Signed-off-by: Will Roden <will@roden.cc>
Signed-off-by: Will Roden <will@roden.cc>
Signed-off-by: Will Roden <will@roden.cc>
Signed-off-by: Will Roden <will@roden.cc>
Codecov ReportBase: 46.95% // Head: 46.95% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #11391 +/- ##
=======================================
Coverage 46.95% 46.95%
=======================================
Files 242 242
Lines 40266 40266
=======================================
Hits 18906 18906
Misses 19452 19452
Partials 1908 1908
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I need this. tried it on the branch I'm working on, generated just fine.
Can you fix conflicts with master?
# Conflicts: # reposerver/repository/repository_test.go
@alexef conflict is resolved |
@WillAbides looks like we have a small diff on the codegen |
Signed-off-by: Will Roden <will@roden.cc>
Sorry about that. This one should be better 🤞 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
♨️
I'm not familiar enough with the e2e tests to know what's going wrong here. Looking at the history, a similar failure seems to be happening in multiple branches. |
What's the status on getting this merged? Is anything needed from me? |
@WillAbides this is very good and very helpful, just a little swamped getting ready for 2.6 RC1 Monday. Will come back to this after. :-) |
@WillAbides would you mind rebasing? I did it locally real quick but don't have permissions to push to your branch. |
Superseded by #18371 |
This adds
//go:generate
statements to generate all the mocks I found in this repo.make gogen
will now generate mocks and ci will fail when mocks are out of date. I searched for mocks by grepping for// Code generated by mockery
and then// Code generated by
along with a visual scan for mockgen or other files that looked like they may contain generated mocks.I chose the newest version of mockery (v2.15.0). I see some of the mocks were previously generated with mockery v1. Mockery v1 doesn't not work with go 1.18+. It doesn't look like there are any breaking changes that affect argo-cd between v1 and v2.
The mocks under
pkg/apiclient
aren't used in this repo as far as I can tell, however they are part of the public API so I kept them with a note that they are unused here.Some mocks were outdated, so this PR updates them to correctly implement their interface. This broke expectations in one test, so I updated that test.
I updated the
gogen
make target to go generate for all directories.I removed a
//go:generate mockgen ...
comment that didn't have a corresponding mock.Closes #11390
Checklist: