From 340713305e9f5cfa1e8213bf72fae0f307cd0945 Mon Sep 17 00:00:00 2001
From: Dibyo Mukherjee <dibyo@google.com>
Date: Tue, 23 Jul 2019 16:55:09 -0400
Subject: [PATCH] Add unit tests for getTaskRunTimeout

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>
---
 .../v1alpha1/pipelinerun/pipelinerun.go       |  2 +-
 .../v1alpha1/pipelinerun/pipelinerun_test.go  | 50 +++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
index b585d268fcd..4906b359120 100644
--- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
+++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
@@ -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
 	}
diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go
index ebe2adb17f2..5774cf1bd91 100644
--- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go
+++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go
@@ -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"
+	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	"github.com/tektoncd/pipeline/pkg/reconciler/v1alpha1/pipelinerun/resources"
@@ -1004,4 +1005,53 @@ func TestReconcilePropagateAnnotations(t *testing.T) {
 	if d := cmp.Diff(actual, expectedTaskRun); d != "" {
 		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: config.DefaultTimeoutMinutes * 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)
+			}
+		})
+	}
 }
\ No newline at end of file