-
Notifications
You must be signed in to change notification settings - Fork 28
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
Explicitly validate the len of params to ensure we don't get index ou… #126
Explicitly validate the len of params to ensure we don't get index ou… #126
Conversation
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 think this looks good. Can you change the PR from draft to "proper" PR? I hope this will trigger the CI.
Ah, never mind my last comment, I was also able to remove the draft status. |
d45f891
to
b9ff469
Compare
Okay, tests show this doesn't work for variadic arguments. I recommend to stick with changing from comparison |
Sadly it doesn't look like changing the check to Instead, I just added a check for non variadic arguments. This change generates verification methods like: func (c *MockDisplay_ChanParams_OngoingVerification) GetAllCapturedArguments() (_param0 []<-chan string, _param1 []chan<- error) {
_params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations)
if len(_params) > 0 {
if 0 < len(_params) {
_param0 = make([]<-chan string, len(c.methodInvocations))
for u, param := range _params[0] {
_param0[u] = param.(<-chan string)
}
}
if 1 < len(_params) {
_param1 = make([]chan<- error, len(c.methodInvocations))
for u, param := range _params[1] {
_param1[u] = param.(chan<- error)
}
}
}
return
} |
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.
Okay, looks good and tests are green. Just one minor nit :-), then it's ready for merge.
Merged and cut a new release 4.1.0 with your changes. Thank you very much for your contribution @AndrewRPorter. |
v4 commits do not exist in
develop
branch, so PR is created againstmain
.Pull Request Changes
Closes
#125