-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Put all pkg/apis/pipeline/v1beta1/*_test.go
files to v1beta1_test
package
#5111
Comments
The motivation here is based on our code standards for tests, which state that we should aim to only test exported functions. FYI @pritidesai |
Note that the main point on the "code standards" is that exported function are tested, the only, for me, is to take with a grain of salt. In an ideal world we would only test exported method ; which tends to make the code base be composed of smaller packages. But we are not in an ideal world 😅. Also, if there is, sometimes, cases where we would like to exported a function or a set of function but we are not really prepared to support go package users to use those, there is always the possibility to use the special |
/assign |
The test files in v1 would be moved to v1_test in a separated PR from #5219. |
Issues go stale after 90d of inactivity. /lifecycle stale Send feedback to tektoncd/plumbing. |
Stale issues rot after 30d of inactivity. /lifecycle rotten Send feedback to tektoncd/plumbing. |
Rotten issues close after 30d of inactivity. /close Send feedback to tektoncd/plumbing. |
@tekton-robot: Closing this issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
verified that this could be closed. |
Issues go stale after 90d of inactivity. /lifecycle stale Send feedback to tektoncd/plumbing. |
Stale issues rot after 30d of inactivity. /lifecycle rotten Send feedback to tektoncd/plumbing. |
Feature request
Under
pkg/apis/pipeline/v1beta1
folder, there are a number of*_test.go
files underv1beta1
package, whereas some are underv1beta1_test
package. Problem with this is that in some test files underv1beta1
package, if we want to use some test helper functions that are defined inv1beta1_test
package, we cannot import them i.e. getContextBasedOnFeatureFlag, enableAlphaAPIFields.So it might be great to move those
*_test.go
files underv1beta1_test
package. However, a couple of pain pointsv1beta1
and prefix all functions/structs defined under it.v1beta1
but used in these*_test.go
files i.e. examplevalidateParamResults
. If we change*_test.go
from packagev1beta1
tov1beta1_test
, we are not sure if it's appropriate to export those functions OR just remove testings for those unexported functions.Talked to @lbernick about this today. Loop in @JeromeJu for visibility.
So this issue is created to track the thoughts/comments/suggestions about this.
The text was updated successfully, but these errors were encountered: