Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: Creates mock generation infrastructure #1763
chore: Creates mock generation infrastructure #1763
Changes from all commits
add48d9
b41ba5e
cc90022
0cbacbc
630cd1b
bb449fb
fee2a7e
c76d184
d171e9b
a3dd559
194a843
05ed39b
481fd10
9520446
92abb8f
19e3159
8907cd8
b761422
a125d08
5b7f3ad
7d1595d
4b4066b
3b478f7
5ef6390
233ce19
7191cd1
650b772
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
taken from Atlas CLI.
We don't run mockery in each make build so it doesn't get slow, but we detect if it was not run when pushing the commits.
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.
why are we weakening the permissions?
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.
@gssbzn the intention was to simplify the action, what problems or risks do you see removing this? (I'm not sure why we're defining permissions in this job but not in others)
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.
at some point we'll have to do some audit of gh actions permissions and be sure GH actions only use what they need, I'm not sure when was this permission added or why, a blame to understand the context would be great before trying to change it
I started some of this auditing on the CLI using https://github.com/ossf/scorecard but it's quite consuming and I haven't had the time to dig deeper on it
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 see, thanks for the info, it looks like we'll need to a focused effort on this and change all workflows.
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.
timeout is in config file now
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.
go generate syntax is deprecated in favor of mockery config file
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.
[nit] since your interfaces are not centralized don't centralize the mocks, it leads to a lot of hooks if an interface happens to share a name and also to a lot of issues later on when you refactor your interfaces
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.
thanks. I wanted to start simple, it's very easy to change at any moment to have mocks in their resource folders.
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.
just be vigilant this doesn't become the permanent state, I'm currently trying to de-tangle some mock mess in the CLI and collocation would've made this easier
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.
won't this pollute the go.mod file?
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.
uhm, it didn' change anything in go.mod
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.
cool, double checking sometimes when doing go install and not using latest things get added to the go.mod
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.
mock programming
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.
we call the SUT (System Under Test) passing the mock collaborator
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.
check mock expectations
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.
Just want to point out that "AssertExpectations asserts that everything specified with On and Return was in fact called as expected. Calls may have occurred in any order." (link) In some tests we might want to avoid this call and instead use other methods such as
AssertNumberOfCalls
,AssertNotCalled
, etc.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.
thanks for the clarification
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.
the mock is auto-generated now