-
Notifications
You must be signed in to change notification settings - Fork 3
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
add unit tests to pipelinerun_helper #8
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.
How does it look when one function is failing?
I was expecting the unit tests to be separated by the units (functions) being tested.
I do see some sense now with grouping them by context, which can represent a single scenario in which the different functions are called. having said that, I'd like to see that when we have a regression in one function, the results will point to that function, rather than to a case with many functions.
It is well organized when using the |
ok. I'm good with that. In that case, I think the |
5c0f4dc
to
e2d7457
Compare
result := []tektonv1.PipelineRunResult{ | ||
{ | ||
Name: "PIPELINERUN_NAME", | ||
Value: *tektonv1.NewStructuredValues(testTruePipelineRun.Name), | ||
}, | ||
{ | ||
Name: "IMAGE_DIGEST", | ||
Value: *tektonv1.NewStructuredValues("image_digest_value"), | ||
}, | ||
{ | ||
Name: "IMAGE_URL", | ||
Value: *tektonv1.NewStructuredValues("image"), | ||
}, | ||
{ | ||
Name: "CHAINS-GIT_URL", | ||
Value: *tektonv1.NewStructuredValues("git_url_value"), | ||
}, | ||
{ | ||
Name: "CHAINS-GIT_COMMIT", | ||
Value: *tektonv1.NewStructuredValues("git_commit_value"), | ||
}, | ||
} |
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.
Is the eventual comparison sensitive to the order of the results? If we see flakiness, we'll need to look into this again.
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.
Left some minor comments
Add unit tests to pipelinerun helper Signed-off-by: Avi Biton <[email protected]>
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
Add unit tests to pipelinerun_helper