use status helper functions in pod status unit tests #3141
Conversation
tekton-robot
added
release-note-none
|
Hi @adshmh. Thanks for your PR. I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
tekton-robot
added
the
needs-ok-to-test
|
/cc @pritidesai |
|
/ok-to-test |
tekton-robot
added
kind/cleanup
adshmh
force-pushed
the
use-helper-function-in-pkg-pod-status-tests
branch
2 times, most recently
from
fdbd822
to
4406fe6
Compare
pkg/pod/status_test.go
Outdated
| func statusRunning(t *testing.T) duckv1beta1.Status { | ||
| t.Helper() | ||
| var trs v1beta1.TaskRunStatus | ||
| MarkStatusRunning(&trs, v1beta1.TaskRunReasonRunning.String(), "Not all Steps in the Task have finished executing") | ||
| return trs.Status | ||
| } | ||
|
|
||
| func statusFailure(t *testing.T, message string) duckv1beta1.Status { | ||
| t.Helper() | ||
| var trs v1beta1.TaskRunStatus | ||
| MarkStatusFailure(&trs, message) | ||
| return trs.Status | ||
| } | ||
|
|
||
| func statusSuccess(t *testing.T) duckv1beta1.Status { | ||
| t.Helper() | ||
| var trs v1beta1.TaskRunStatus | ||
| MarkStatusSuccess(&trs) | ||
| return trs.Status | ||
| } | ||
|
|
||
| func statusPending(t *testing.T, reason, message string) duckv1beta1.Status { | ||
| t.Helper() | ||
| return duckv1beta1.Status{ | ||
| Conditions: []apis.Condition{{ | ||
| Type: apis.ConditionSucceeded, | ||
| Status: corev1.ConditionUnknown, | ||
| Reason: reason, | ||
| Message: message, | ||
| }}, | ||
| } | ||
| } |
There was a problem hiding this comment.
All those methods do not need t *testing.T 👼
There was a problem hiding this comment.
yes t.Helper() needs access to *t testing.T variable
There was a problem hiding this comment.
@pritidesai no need to call t.Helper if the function itself doesn't use *testing.T. t.Helper() is only useful (and needed) if you want the report (t.Log, t.Error, …) done in a function to be reported as the caller of this function. Here, we don't use t *testing.T at all in these functions, so we don't need to call t.Helper(), and so we don't need to pass *testing.T
tekton-robot
added
the
needs-rebase
|
@adshmh just checking in, do you want to carry on with this PR? Looks like there is still some open feedback to address. Cheers! |
Signed-off-by: Arash Deshmeh <[email protected]>
adshmh
force-pushed
the
use-helper-function-in-pkg-pod-status-tests
branch
from
4406fe6
to
3f05158
Compare
tekton-robot
removed
the
needs-rebase
Thank you for the reminder. I have updated the PR to address all the reviews. |
|
/retest |
|
Thanks for refactoring! /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbwsg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
Changes
use status helper functions in pod status unit tests. Addresses #2804 (comment)
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmddir, please updatethe release Task to build and release this image.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes