Skip to content

Commit

Permalink
Add unit tests for getTaskRunTimeout
Browse files Browse the repository at this point in the history
getTaskRunTimeout sets the timeout values for taskruns
created by pipelineruns. While adding the tests, I noticed
that if `pr.Spec.Timeout` is nil, we were setting the default
timeout to 60 seconds and not minutes. This is really an edge
case since we only hit this block if the webhook does not set
the defaults properly.

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
  • Loading branch information
dibyom authored and tekton-robot committed Jul 24, 2019
1 parent 4f6fcf0 commit 3409f09
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ func getTaskRunTimeout(pr *v1alpha1.PipelineRun) *metav1.Duration {

var timeout time.Duration
if pr.Spec.Timeout == nil {
timeout = config.DefaultTimeoutMinutes
timeout = config.DefaultTimeoutMinutes * time.Minute
} else {
timeout = pr.Spec.Timeout.Duration
}
Expand Down
48 changes: 48 additions & 0 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
duckv1beta1 "github.com/knative/pkg/apis/duck/v1beta1"
"github.com/knative/pkg/configmap"
rtesting "github.com/knative/pkg/reconciler/testing"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ktesting "k8s.io/client-go/testing"
Expand Down Expand Up @@ -1006,3 +1007,50 @@ func TestReconcilePropagateAnnotations(t *testing.T) {
t.Errorf("expected to see TaskRun %v created. Diff %s", expectedTaskRun, d)
}
}

func TestGetTaskRunTimeout(t *testing.T) {
prName := "pipelinerun-timeouts"
ns := "foo"
p := "pipeline"

tcs := []struct {
name string
pr *v1alpha1.PipelineRun
expected *metav1.Duration
}{{
name: "nil timeout duration",
pr: tb.PipelineRun(prName, ns,
tb.PipelineRunSpec(p, tb.PipelineRunNilTimeout),
tb.PipelineRunStatus(tb.PipelineRunStartTime(time.Now())),
),
expected: &metav1.Duration{Duration: 60 * time.Minute},
}, {
name: "timeout specified in pr",
pr: tb.PipelineRun(prName, ns,
tb.PipelineRunSpec(p, tb.PipelineRunTimeout(20*time.Minute)),
tb.PipelineRunStatus(tb.PipelineRunStartTime(time.Now())),
),
expected: &metav1.Duration{Duration: 20 * time.Minute},
}, {
name: "0 timeout duration",
pr: tb.PipelineRun(prName, ns,
tb.PipelineRunSpec(p, tb.PipelineRunTimeout(0*time.Minute)),
tb.PipelineRunStatus(tb.PipelineRunStartTime(time.Now())),
),
expected: &metav1.Duration{Duration: 0 * time.Minute},
}, {
name: "taskrun being created after timeout expired",
pr: tb.PipelineRun(prName, ns,
tb.PipelineRunSpec(p, tb.PipelineRunTimeout(1*time.Minute)),
tb.PipelineRunStatus(tb.PipelineRunStartTime(time.Now().Add(-2*time.Minute)))),
expected: &metav1.Duration{Duration: 1 * time.Second},
}}

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
if d := cmp.Diff(getTaskRunTimeout(tc.pr), tc.expected); d != "" {
t.Errorf("Unexpected task run timeout. Diff %s", d)
}
})
}
}

0 comments on commit 3409f09

Please sign in to comment.