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 b50c2dc commit e9fa865
Show file tree
Hide file tree
Showing 2 changed files with 52 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
51 changes: 51 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 @@ -1016,3 +1017,53 @@ func TestGetTaskRunServiceAccounts(t *testing.T) {
})
}
}

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 e9fa865

Please sign in to comment.