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 committed Jul 24, 2019
1 parent 8218f4f commit 4bc47d3
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 4bc47d3

Please sign in to comment.