-
Notifications
You must be signed in to change notification settings - Fork 88
tets: test mock generation works #1029
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
Conversation
|
example failing test without my fix |
| const ( | ||
| mongoCLI = "mongocli" | ||
| atlasCLI = "atlascli" | ||
| ) |
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.
drive by these were being imported from the version package but only used here
| type ReleaseInformation struct { | ||
| Version string | ||
| PublishedAt time.Time | ||
| } |
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.
same as above, imported from version but only ever used here
| const ( | ||
| MongoCLI = "mongocli" | ||
| AtlasCLI = "atlascli" | ||
| owner = "mongodb" | ||
| ) |
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.
moved closer to where needed
| ) | ||
|
|
||
| //go:generate mockgen -destination=../mocks/mock_release_version.go -package=mocks github.com/mongodb/mongocli/internal/version VersionFinder | ||
| //go:generate mockgen -destination=../mocks/mock_release_version.go -package=mocks github.com/mongodb/mongocli/internal/latestrelease VersionFinder |
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.
fix to mock generation
| type ReleaseInformation struct { | ||
| Version string | ||
| PublishedAt time.Time | ||
| } |
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.
moved closer to where needed
| @@ -0,0 +1,32 @@ | |||
| --- | |||
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.
inspired by the docs check
andreaangiolillo
left a comment
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.
LGTM 👍
fmenezes
left a comment
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.
LGTM good job
I had some good inspiration to base this work from :) |
Proposed changes
the
make gen-mockscommand has been broken for a while, fixing it and adding a test to make sure it worksChecklist
make fmtand formatted my code