Skip to content

Commit

Permalink
Unexport RPRT member functions used within resources pkg only
Browse files Browse the repository at this point in the history 
Some member functions of ResolvedPipelineRunTask are exported but they are used
within the resources package only: `IsDone`, `IsSuccessful`, `IsFailure`, `IsRunning`
`HasRemainingRetries`, `IsCancelled`, `IsConditionStatusFalse`, and `IsStarted`.

These functions, except `IsDone`, are not tested but the exported functions that
use them within the resources package are tested.

In this change, we unexport the above member functions that are used within resources
package only. We do not add tests for the now-unexported functions as recommended
in [guidelines][guidelines], but we can add them later if we change the guideline.

[guidelines]: https://github.com/tektoncd/community/blob/ac0ae1b304ef515e8099f772f42b91aac1b26e6b/standards.md#tests
  • Loading branch information
@jerop
jerop committed Jun 8, 2022
1 parent 566e2e3 commit b5971a6
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 45 deletions.
50 changes: 25 additions & 25 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ type ResolvedPipelineRunTask struct {
ResolvedTaskResources *resources.ResolvedTaskResources
}

// IsDone returns true only if the task is skipped, succeeded or failed
func (t ResolvedPipelineRunTask) IsDone(facts *PipelineRunFacts) bool {
return t.Skip(facts).IsSkipped || t.IsSuccessful() || t.IsFailure()
// isDone returns true only if the task is skipped, succeeded or failed
func (t ResolvedPipelineRunTask) isDone(facts *PipelineRunFacts) bool {
return t.Skip(facts).IsSkipped || t.isSuccessful() || t.isFailure()
}

// IsRunning returns true only if the task is neither succeeded, cancelled nor failed
func (t ResolvedPipelineRunTask) IsRunning() bool {
// isRunning returns true only if the task is neither succeeded, cancelled nor failed
func (t ResolvedPipelineRunTask) isRunning() bool {
if t.IsCustomTask() {
if t.Run == nil {
return false
Expand All @@ -84,7 +84,7 @@ func (t ResolvedPipelineRunTask) IsRunning() bool {
return false
}
}
return !t.IsSuccessful() && !t.IsFailure() && !t.IsCancelled()
return !t.isSuccessful() && !t.isFailure() && !t.isCancelled()
}

// IsCustomTask returns true if the PipelineTask references a Custom Task.
Expand All @@ -97,20 +97,20 @@ func (t ResolvedPipelineRunTask) IsMatrixed() bool {
return len(t.PipelineTask.Matrix) > 0
}

// IsSuccessful returns true only if the run has completed successfully
func (t ResolvedPipelineRunTask) IsSuccessful() bool {
// isSuccessful returns true only if the run has completed successfully
func (t ResolvedPipelineRunTask) isSuccessful() bool {
if t.IsCustomTask() {
return t.Run != nil && t.Run.IsSuccessful()
}
return t.TaskRun != nil && t.TaskRun.IsSuccessful()
}

// IsFailure returns true only if the run has failed and will not be retried.
func (t ResolvedPipelineRunTask) IsFailure() bool {
if t.IsCancelled() {
// isFailure returns true only if the run has failed and will not be retried.
func (t ResolvedPipelineRunTask) isFailure() bool {
if t.isCancelled() {
return true
}
if t.IsSuccessful() {
if t.isSuccessful() {
return false
}
var c *apis.Condition
Expand All @@ -128,12 +128,12 @@ func (t ResolvedPipelineRunTask) IsFailure() bool {
c = t.TaskRun.Status.GetCondition(apis.ConditionSucceeded)
isDone = t.TaskRun.IsDone()
}
return isDone && c.IsFalse() && !t.HasRemainingRetries()
return isDone && c.IsFalse() && !t.hasRemainingRetries()
}

// HasRemainingRetries returns true only when the number of retries already attempted
// hasRemainingRetries returns true only when the number of retries already attempted
// is less than the number of retries allowed.
func (t ResolvedPipelineRunTask) HasRemainingRetries() bool {
func (t ResolvedPipelineRunTask) hasRemainingRetries() bool {
var retriesDone int
if t.IsCustomTask() {
if t.Run == nil {
Expand All @@ -149,8 +149,8 @@ func (t ResolvedPipelineRunTask) HasRemainingRetries() bool {
return retriesDone < t.PipelineTask.Retries
}

// IsCancelled returns true only if the run is cancelled
func (t ResolvedPipelineRunTask) IsCancelled() bool {
// isCancelled returns true only if the run is cancelled
func (t ResolvedPipelineRunTask) isCancelled() bool {
if t.IsCustomTask() {
if t.Run == nil {
return false
Expand All @@ -165,20 +165,20 @@ func (t ResolvedPipelineRunTask) IsCancelled() bool {
return c != nil && c.IsFalse() && c.Reason == v1beta1.TaskRunReasonCancelled.String()
}

// IsStarted returns true only if the PipelineRunTask itself has a TaskRun or
// isStarted returns true only if the PipelineRunTask itself has a TaskRun or
// Run associated that has a Succeeded-type condition.
func (t ResolvedPipelineRunTask) IsStarted() bool {
func (t ResolvedPipelineRunTask) isStarted() bool {
if t.IsCustomTask() {
return t.Run != nil && t.Run.Status.GetCondition(apis.ConditionSucceeded) != nil

}
return t.TaskRun != nil && t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) != nil
}

// IsConditionStatusFalse returns true when a task has succeeded condition with status set to false
// isConditionStatusFalse returns true when a task has succeeded condition with status set to false
// it includes task failed after retries are exhausted, cancelled tasks, and time outs
func (t ResolvedPipelineRunTask) IsConditionStatusFalse() bool {
if t.IsStarted() {
func (t ResolvedPipelineRunTask) isConditionStatusFalse() bool {
if t.isStarted() {
if t.IsCustomTask() {
return t.Run.Status.GetCondition(apis.ConditionSucceeded).IsFalse()
}
Expand All @@ -194,7 +194,7 @@ func (t *ResolvedPipelineRunTask) checkParentsDone(facts *PipelineRunFacts) bool
stateMap := facts.State.ToMap()
node := facts.TasksGraph.Nodes[t.PipelineTask.Name]
for _, p := range node.Prev {
if !stateMap[p.Task.HashKey()].IsDone(facts) {
if !stateMap[p.Task.HashKey()].isDone(facts) {
return false
}
}
Expand All @@ -205,7 +205,7 @@ func (t *ResolvedPipelineRunTask) skip(facts *PipelineRunFacts) TaskSkipStatus {
var skippingReason v1beta1.SkippingReason

switch {
case facts.isFinalTask(t.PipelineTask.Name) || t.IsStarted():
case facts.isFinalTask(t.PipelineTask.Name) || t.isStarted():
skippingReason = v1beta1.None
case facts.IsStopping():
skippingReason = v1beta1.StoppingSkip
Expand Down Expand Up @@ -305,7 +305,7 @@ func (t *ResolvedPipelineRunTask) IsFinallySkipped(facts *PipelineRunFacts) Task
var skippingReason v1beta1.SkippingReason

switch {
case t.IsStarted():
case t.isStarted():
skippingReason = v1beta1.None
case facts.checkDAGTasksDone() && facts.isFinalTask(t.PipelineTask.Name):
switch {
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1066,8 +1066,8 @@ func TestIsFailure(t *testing.T) {
want: true,
}} {
t.Run(tc.name, func(t *testing.T) {
if got := tc.task.IsFailure(); got != tc.want {
t.Errorf("expected IsFailure: %t but got %t", tc.want, got)
if got := tc.task.isFailure(); got != tc.want {
t.Errorf("expected isFailure: %t but got %t", tc.want, got)
}

})
Expand Down
30 changes: 15 additions & 15 deletions pkg/reconciler/pipelinerun/resources/pipelinerunstate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (state PipelineRunState) GetTaskRunsResults() map[string][]v1beta1.TaskRunR
if rprt.IsCustomTask() {
continue
}
if !rprt.IsSuccessful() {
if !rprt.isSuccessful() {
continue
}
results[rprt.PipelineTask.Name] = rprt.TaskRun.Status.TaskRunResults
Expand Down Expand Up @@ -227,7 +227,7 @@ func (state PipelineRunState) GetRunsResults() map[string][]v1alpha1.RunResult {
if !rprt.IsCustomTask() {
continue
}
if !rprt.IsSuccessful() {
if !rprt.isSuccessful() {
continue
}
results[rprt.PipelineTask.Name] = rprt.Run.Status.Results
Expand Down Expand Up @@ -323,7 +323,7 @@ func (state PipelineRunState) getRetryableTasks(candidateTasks sets.String) []*R
}
if status.IsFalse() {
if !(isCancelled || status.Reason == ReasonConditionCheckFailed) {
if t.HasRemainingRetries() {
if t.hasRemainingRetries() {
tasks = append(tasks, t)
}
}
Expand All @@ -339,10 +339,10 @@ func (state PipelineRunState) getRetryableTasks(candidateTasks sets.String) []*R
func (facts *PipelineRunFacts) IsStopping() bool {
for _, t := range facts.State {
if facts.isDAGTask(t.PipelineTask.Name) {
if t.IsCancelled() {
if t.isCancelled() {
return true
}
if t.IsFailure() {
if t.isFailure() {
return true
}
}
Expand All @@ -354,7 +354,7 @@ func (facts *PipelineRunFacts) IsStopping() bool {
func (facts *PipelineRunFacts) IsRunning() bool {
for _, t := range facts.State {
if facts.isDAGTask(t.PipelineTask.Name) {
if t.IsRunning() {
if t.isRunning() {
return true
}
}
Expand Down Expand Up @@ -412,7 +412,7 @@ func (facts *PipelineRunFacts) GetFinalTasks() PipelineRunState {
if facts.checkDAGTasksDone() {
// return list of tasks with all final tasks
for _, t := range facts.State {
if facts.isFinalTask(t.PipelineTask.Name) && !t.IsSuccessful() {
if facts.isFinalTask(t.PipelineTask.Name) && !t.isSuccessful() {
finalCandidates.Insert(t.PipelineTask.Name)
}
}
Expand Down Expand Up @@ -546,10 +546,10 @@ func (facts *PipelineRunFacts) GetPipelineTaskStatus() map[string]string {
var s string
switch {
// execution status is Succeeded when a task has succeeded condition with status set to true
case t.IsSuccessful():
case t.isSuccessful():
s = v1beta1.TaskRunReasonSuccessful.String()
// execution status is Failed when a task has succeeded condition with status set to false
case t.IsConditionStatusFalse():
case t.isConditionStatusFalse():
s = v1beta1.TaskRunReasonFailed.String()
default:
// None includes skipped as well
Expand All @@ -567,7 +567,7 @@ func (facts *PipelineRunFacts) GetPipelineTaskStatus() map[string]string {
for _, t := range facts.State {
if facts.isDAGTask(t.PipelineTask.Name) {
// if any of the dag task failed, change the aggregate status to failed and return
if t.IsConditionStatusFalse() {
if t.isConditionStatusFalse() {
aggregateStatus = v1beta1.PipelineRunReasonFailed.String()
break
}
Expand All @@ -589,7 +589,7 @@ func (facts *PipelineRunFacts) completedOrSkippedDAGTasks() []string {
tasks := []string{}
for _, t := range facts.State {
if facts.isDAGTask(t.PipelineTask.Name) {
if t.IsDone(facts) {
if t.isDone(facts) {
tasks = append(tasks, t.PipelineTask.Name)
}
}
Expand All @@ -602,7 +602,7 @@ func (facts *PipelineRunFacts) completedOrSkippedDAGTasks() []string {
func (facts *PipelineRunFacts) checkTasksDone(d *dag.Graph) bool {
for _, t := range facts.State {
if isTaskInGraph(t.PipelineTask.Name, d) {
if !t.IsDone(facts) {
if !t.isDone(facts) {
return false
}
}
Expand Down Expand Up @@ -632,13 +632,13 @@ func (facts *PipelineRunFacts) getPipelineTasksCount() pipelineRunStatusCount {
for _, t := range facts.State {
switch {
// increment success counter since the task is successful
case t.IsSuccessful():
case t.isSuccessful():
s.Succeeded++
// increment cancelled counter since the task is cancelled
case t.IsCancelled():
case t.isCancelled():
s.Cancelled++
// increment failure counter since the task has failed
case t.IsFailure():
case t.isFailure():
s.Failed++
// increment skip counter since the task is skipped
case t.Skip(facts).IsSkipped:
Expand Down