From 723fca9cc061adc0204c72b24f39737affdb49f3 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Mon, 14 Sep 2020 21:27:14 -0400 Subject: [PATCH] Clean up dag_test.go - don't use test builders - use script mode instead of command+args - sort start times using sort.Slice - compare parallel task start times using math.Abs in case s1 starts >5s before s2 -- only the other case was checked before - then copy test/dag_test.go -> test/v1alpha1/dag_test.go and fix incompatibilities --- test/dag_test.go | 120 ++++++++---------- test/v1alpha1/dag_test.go | 259 +++++++++++++++++++++++--------------- 2 files changed, 208 insertions(+), 171 deletions(-) diff --git a/test/dag_test.go b/test/dag_test.go index 2b8593f8d33..e1f45032fd6 100644 --- a/test/dag_test.go +++ b/test/dag_test.go @@ -19,15 +19,14 @@ limitations under the License. package test import ( + "math" "sort" "strings" "testing" "time" - tb "github.com/tektoncd/pipeline/internal/builder/v1beta1" - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" - resources "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" + resource "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/typed/pipeline/v1beta1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -53,7 +52,7 @@ func TestDAGPipelineRun(t *testing.T) { // Create the Task that echoes text repoTaskResource := v1beta1.TaskResource{ResourceDeclaration: v1beta1.ResourceDeclaration{ - Name: "repo", Type: resources.PipelineResourceTypeGit, + Name: "repo", Type: resource.PipelineResourceTypeGit, }} echoTask := &v1beta1.Task{ ObjectMeta: metav1.ObjectMeta{Name: "echo-task", Namespace: namespace}, @@ -66,15 +65,13 @@ func TestDAGPipelineRun(t *testing.T) { Name: "text", Type: v1beta1.ParamTypeString, Description: "The text that should be echoed", }}, - Steps: []v1beta1.Step{{Container: corev1.Container{ - Image: "busybox", - Command: []string{"echo"}, - Args: []string{"$(params.text)"}, - }}, {Container: corev1.Container{ - Image: "busybox", - Command: []string{"ln"}, - Args: []string{"-s", "$(resources.inputs.repo.path)", "$(resources.outputs.repo.path)"}, - }}}, + Steps: []v1beta1.Step{{ + Container: corev1.Container{Image: "busybox"}, + Script: "echo $(params.text)", + }, { + Container: corev1.Container{Image: "busybox"}, + Script: "ln -s $(resources.inputs.repo.path) $(resources.outputs.repo.path)", + }}, }, } if _, err := c.TaskClient.Create(echoTask); err != nil { @@ -82,10 +79,16 @@ func TestDAGPipelineRun(t *testing.T) { } // Create the repo PipelineResource (doesn't really matter which repo we use) - repoResource := tb.PipelineResource("repo", tb.PipelineResourceSpec( - v1alpha1.PipelineResourceTypeGit, - tb.PipelineResourceSpecParam("Url", "https://github.com/githubtraining/example-basic"), - )) + repoResource := &resource.PipelineResource{ + ObjectMeta: metav1.ObjectMeta{Name: "repo"}, + Spec: resource.PipelineResourceSpec{ + Type: resource.PipelineResourceTypeGit, + Params: []resource.ResourceParam{{ + Name: "Url", + Value: "https://github.com/githubtraining/example-basic", + }}, + }, + } if _, err := c.PipelineResourceClient.Create(repoResource); err != nil { t.Fatalf("Failed to create simple repo PipelineResource: %s", err) } @@ -96,7 +99,7 @@ func TestDAGPipelineRun(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "dag-pipeline", Namespace: namespace}, Spec: v1beta1.PipelineSpec{ Resources: []v1beta1.PipelineDeclaredResource{{ - Name: "repo", Type: resources.PipelineResourceTypeGit, + Name: "repo", Type: resource.PipelineResourceTypeGit, }}, Tasks: []v1beta1.PipelineTask{{ Name: "pipeline-task-3", @@ -196,71 +199,46 @@ func TestDAGPipelineRun(t *testing.T) { t.Fatalf("Error waiting for PipelineRun to finish: %s", err) } - t.Logf("Verifying order of execution") - times := getTaskStartTimes(t, c.TaskRunClient) - vefifyExpectedOrder(t, times) -} - -type runTime struct { - name string - t time.Time -} - -type runTimes []runTime - -func (f runTimes) Len() int { - return len(f) + verifyExpectedOrder(t, c.TaskRunClient) } -func (f runTimes) Less(i, j int) bool { - return f[i].t.Before(f[j].t) -} - -func (f runTimes) Swap(i, j int) { - f[i], f[j] = f[j], f[i] -} +func verifyExpectedOrder(t *testing.T, c clientset.TaskRunInterface) { + t.Logf("Verifying order of execution") -func getTaskStartTimes(t *testing.T, c clientset.TaskRunInterface) runTimes { - taskRuns, err := c.List(metav1.ListOptions{}) + taskRunsResp, err := c.List(metav1.ListOptions{}) if err != nil { t.Fatalf("Couldn't get TaskRuns (so that we could check when they executed): %v", err) } - times := runTimes{} - for _, t := range taskRuns.Items { - times = append(times, runTime{ - name: t.Name, - t: t.Status.StartTime.Time, - }) - } - return times -} - -func vefifyExpectedOrder(t *testing.T, times runTimes) { - if len(times) != 5 { - t.Fatalf("Expected 5 Taskruns to have executed but only got start times for %d Taskruns", len(times)) + taskRuns := taskRunsResp.Items + if len(taskRuns) != 5 { + t.Fatalf("Expected 5 TaskRuns to have executed but got start times for %d TaskRuns", len(taskRuns)) } - sort.Sort(times) + sort.Slice(taskRuns, func(i, j int) bool { + it := taskRuns[i].Status.StartTime.Time + jt := taskRuns[j].Status.StartTime.Time + return it.Before(jt) + }) - if !strings.HasPrefix(times[0].name, "dag-pipeline-run-pipeline-task-1") { - t.Errorf("Expected first task to execute first, but %q was first", times[0].name) - } - if !strings.HasPrefix(times[1].name, "dag-pipeline-run-pipeline-task-2") { - t.Errorf("Expected parallel tasks to run second & third, but %q was second", times[1].name) - } - if !strings.HasPrefix(times[2].name, "dag-pipeline-run-pipeline-task-2") { - t.Errorf("Expected parallel tasks to run second & third, but %q was third", times[2].name) - } - if !strings.HasPrefix(times[3].name, "dag-pipeline-run-pipeline-task-3") { - t.Errorf("Expected third task to execute third, but %q was third", times[3].name) + wantPrefixes := []string{ + "dag-pipeline-run-pipeline-task-1", + // Could be task-2-parallel-1 or task-2-parallel-2 + "dag-pipeline-run-pipeline-task-2-parallel", + "dag-pipeline-run-pipeline-task-2-parallel", + "dag-pipeline-run-pipeline-task-3", + "dag-pipeline-run-pipeline-task-4", } - if !strings.HasPrefix(times[4].name, "dag-pipeline-run-pipeline-task-4") { - t.Errorf("Expected fourth task to execute fourth, but %q was fourth", times[4].name) + for i, wp := range wantPrefixes { + if !strings.HasPrefix(taskRuns[i].Name, wp) { + t.Errorf("Expected task %q to execute first, but %q was first", wp, taskRuns[0].Name) + } } // Check that the two tasks that can run in parallel did - parallelDiff := times[2].t.Sub(times[1].t) - if parallelDiff > (time.Second * 5) { - t.Errorf("Expected parallel tasks to execute more or less at the same time, but they were %v apart", parallelDiff) + s1 := taskRuns[1].Status.StartTime.Time + s2 := taskRuns[2].Status.StartTime.Time + absDiff := time.Duration(math.Abs(float64(s2.Sub(s1)))) + if absDiff > (time.Second * 5) { + t.Errorf("Expected parallel tasks to execute more or less at the same time, but they were %v apart", absDiff) } } diff --git a/test/v1alpha1/dag_test.go b/test/v1alpha1/dag_test.go index cf80c296037..1cdd6a51a67 100644 --- a/test/v1alpha1/dag_test.go +++ b/test/v1alpha1/dag_test.go @@ -19,14 +19,17 @@ limitations under the License. package test import ( + "math" "sort" "strings" "testing" "time" - tb "github.com/tektoncd/pipeline/internal/builder/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + resource "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/typed/pipeline/v1alpha1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" knativetest "knative.dev/pkg/test" ) @@ -49,65 +52,146 @@ func TestDAGPipelineRun(t *testing.T) { defer tearDown(t, c, namespace) // Create the Task that echoes text - echoTask := tb.Task("echo-task", tb.TaskSpec( - tb.TaskInputs( - tb.InputsResource("repo", v1alpha1.PipelineResourceTypeGit), - tb.InputsParamSpec("text", v1alpha1.ParamTypeString, tb.ParamSpecDescription("The text that should be echoed")), - ), - tb.TaskOutputs(tb.OutputsResource("repo", v1alpha1.PipelineResourceTypeGit)), - tb.Step("busybox", tb.StepCommand("echo"), tb.StepArgs("$(inputs.params.text)")), - tb.Step("busybox", tb.StepCommand("ln"), tb.StepArgs("-s", "$(inputs.resources.repo.path)", "$(outputs.resources.repo.path)")), - )) + repoTaskResource := v1alpha1.TaskResource{ResourceDeclaration: v1alpha1.ResourceDeclaration{ + Name: "repo", Type: resource.PipelineResourceTypeGit, + }} + echoTask := &v1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{Name: "echo-task", Namespace: namespace}, + Spec: v1alpha1.TaskSpec{TaskSpec: v1beta1.TaskSpec{ + Resources: &v1beta1.TaskResources{ + Inputs: []v1alpha1.TaskResource{repoTaskResource}, + Outputs: []v1alpha1.TaskResource{repoTaskResource}, + }, + Params: []v1alpha1.ParamSpec{{ + Name: "text", Type: v1alpha1.ParamTypeString, + Description: "The text that should be echoed", + }}, + Steps: []v1alpha1.Step{{ + Container: corev1.Container{Image: "busybox"}, + Script: "echo $(params.text)", + }, { + Container: corev1.Container{Image: "busybox"}, + Script: "ln -s $(resources.inputs.repo.path) $(resources.outputs.repo.path)", + }}, + }}, + } if _, err := c.TaskClient.Create(echoTask); err != nil { t.Fatalf("Failed to create echo Task: %s", err) } // Create the repo PipelineResource (doesn't really matter which repo we use) - repoResource := tb.PipelineResource("repo", tb.PipelineResourceSpec( - v1alpha1.PipelineResourceTypeGit, - tb.PipelineResourceSpecParam("Url", "https://github.com/githubtraining/example-basic"), - )) + repoResource := &resource.PipelineResource{ + ObjectMeta: metav1.ObjectMeta{Name: "repo"}, + Spec: resource.PipelineResourceSpec{ + Type: resource.PipelineResourceTypeGit, + Params: []resource.ResourceParam{{ + Name: "Url", + Value: "https://github.com/githubtraining/example-basic", + }}, + }, + } if _, err := c.PipelineResourceClient.Create(repoResource); err != nil { t.Fatalf("Failed to create simple repo PipelineResource: %s", err) } // Intentionally declaring Tasks in a mixed up order to ensure the order // of execution isn't at all dependent on the order they are declared in - pipeline := tb.Pipeline("dag-pipeline", tb.PipelineSpec( - tb.PipelineDeclaredResource("repo", "git"), - tb.PipelineTask("pipeline-task-3", "echo-task", - tb.PipelineTaskInputResource("repo", "repo", tb.From("pipeline-task-2-parallel-1", "pipeline-task-2-parallel-2")), - tb.PipelineTaskOutputResource("repo", "repo"), - tb.PipelineTaskParam("text", "wow"), - ), - tb.PipelineTask("pipeline-task-2-parallel-2", "echo-task", - tb.PipelineTaskInputResource("repo", "repo", tb.From("pipeline-task-1")), tb.PipelineTaskOutputResource("repo", "repo"), - tb.PipelineTaskOutputResource("repo", "repo"), - tb.PipelineTaskParam("text", "such parallel"), - ), - tb.PipelineTask("pipeline-task-4", "echo-task", - tb.RunAfter("pipeline-task-3"), - tb.PipelineTaskInputResource("repo", "repo"), - tb.PipelineTaskOutputResource("repo", "repo"), - tb.PipelineTaskParam("text", "very cloud native"), - ), - tb.PipelineTask("pipeline-task-2-parallel-1", "echo-task", - tb.PipelineTaskInputResource("repo", "repo", tb.From("pipeline-task-1")), - tb.PipelineTaskOutputResource("repo", "repo"), - tb.PipelineTaskParam("text", "much graph"), - ), - tb.PipelineTask("pipeline-task-1", "echo-task", - tb.PipelineTaskInputResource("repo", "repo"), - tb.PipelineTaskOutputResource("repo", "repo"), - tb.PipelineTaskParam("text", "how to ci/cd?"), - ), - )) + pipeline := &v1alpha1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "dag-pipeline", Namespace: namespace}, + Spec: v1alpha1.PipelineSpec{ + Resources: []v1alpha1.PipelineDeclaredResource{{ + Name: "repo", Type: resource.PipelineResourceTypeGit, + }}, + Tasks: []v1alpha1.PipelineTask{{ + Name: "pipeline-task-3", + TaskRef: &v1alpha1.TaskRef{Name: "echo-task"}, + Params: []v1alpha1.Param{{ + Name: "text", Value: *v1beta1.NewArrayOrString("wow"), + }}, + Resources: &v1alpha1.PipelineTaskResources{ + Inputs: []v1alpha1.PipelineTaskInputResource{{ + Name: "repo", Resource: "repo", + From: []string{"pipeline-task-2-parallel-1", "pipeline-task-2-parallel-2"}, + }}, + Outputs: []v1alpha1.PipelineTaskOutputResource{{ + Name: "repo", Resource: "repo", + }}, + }, + }, { + Name: "pipeline-task-2-parallel-2", + TaskRef: &v1alpha1.TaskRef{Name: "echo-task"}, + Params: []v1alpha1.Param{{ + Name: "text", Value: *v1beta1.NewArrayOrString("such parallel"), + }}, + Resources: &v1alpha1.PipelineTaskResources{ + Inputs: []v1alpha1.PipelineTaskInputResource{{ + Name: "repo", Resource: "repo", + From: []string{"pipeline-task-1"}, + }}, + Outputs: []v1alpha1.PipelineTaskOutputResource{{ + Name: "repo", Resource: "repo", + }}, + }, + }, { + Name: "pipeline-task-4", + TaskRef: &v1alpha1.TaskRef{Name: "echo-task"}, + Params: []v1alpha1.Param{{ + Name: "text", Value: *v1beta1.NewArrayOrString("very cloud native"), + }}, + Resources: &v1alpha1.PipelineTaskResources{ + Inputs: []v1alpha1.PipelineTaskInputResource{{ + Name: "repo", Resource: "repo", + }}, + Outputs: []v1alpha1.PipelineTaskOutputResource{{ + Name: "repo", Resource: "repo", + }}, + }, + RunAfter: []string{"pipeline-task-3"}, + }, { + Name: "pipeline-task-2-parallel-1", + TaskRef: &v1alpha1.TaskRef{Name: "echo-task"}, + Params: []v1alpha1.Param{{ + Name: "text", Value: *v1beta1.NewArrayOrString("much graph"), + }}, + Resources: &v1alpha1.PipelineTaskResources{ + Inputs: []v1alpha1.PipelineTaskInputResource{{ + Name: "repo", Resource: "repo", + From: []string{"pipeline-task-1"}, + }}, + Outputs: []v1alpha1.PipelineTaskOutputResource{{ + Name: "repo", Resource: "repo", + }}, + }, + }, { + Name: "pipeline-task-1", + TaskRef: &v1alpha1.TaskRef{Name: "echo-task"}, + Params: []v1alpha1.Param{{ + Name: "text", Value: *v1beta1.NewArrayOrString("how to ci/cd?"), + }}, + Resources: &v1alpha1.PipelineTaskResources{ + Inputs: []v1alpha1.PipelineTaskInputResource{{ + Name: "repo", Resource: "repo", + }}, + Outputs: []v1alpha1.PipelineTaskOutputResource{{ + Name: "repo", Resource: "repo", + }}, + }, + }}, + }, + } if _, err := c.PipelineClient.Create(pipeline); err != nil { t.Fatalf("Failed to create dag-pipeline: %s", err) } - pipelineRun := tb.PipelineRun("dag-pipeline-run", tb.PipelineRunSpec("dag-pipeline", - tb.PipelineRunResourceBinding("repo", tb.PipelineResourceBindingRef("repo")), - )) + pipelineRun := &v1alpha1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{Name: "dag-pipeline-run", Namespace: namespace}, + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: &v1alpha1.PipelineRef{Name: "dag-pipeline"}, + Resources: []v1alpha1.PipelineResourceBinding{{ + Name: "repo", + ResourceRef: &v1alpha1.PipelineResourceRef{Name: "repo"}, + }}, + }, + } if _, err := c.PipelineRunClient.Create(pipelineRun); err != nil { t.Fatalf("Failed to create dag-pipeline-run PipelineRun: %s", err) } @@ -116,71 +200,46 @@ func TestDAGPipelineRun(t *testing.T) { t.Fatalf("Error waiting for PipelineRun to finish: %s", err) } - t.Logf("Verifying order of execution") - times := getTaskStartTimes(t, c.TaskRunClient) - vefifyExpectedOrder(t, times) + verifyExpectedOrder(t, c.TaskRunClient) } -type runTime struct { - name string - t time.Time -} - -type runTimes []runTime - -func (f runTimes) Len() int { - return len(f) -} - -func (f runTimes) Less(i, j int) bool { - return f[i].t.Before(f[j].t) -} - -func (f runTimes) Swap(i, j int) { - f[i], f[j] = f[j], f[i] -} +func verifyExpectedOrder(t *testing.T, c clientset.TaskRunInterface) { + t.Logf("Verifying order of execution") -func getTaskStartTimes(t *testing.T, c clientset.TaskRunInterface) runTimes { - taskRuns, err := c.List(metav1.ListOptions{}) + taskRunsResp, err := c.List(metav1.ListOptions{}) if err != nil { t.Fatalf("Couldn't get TaskRuns (so that we could check when they executed): %v", err) } - times := runTimes{} - for _, t := range taskRuns.Items { - times = append(times, runTime{ - name: t.Name, - t: t.Status.StartTime.Time, - }) - } - return times -} - -func vefifyExpectedOrder(t *testing.T, times runTimes) { - if len(times) != 5 { - t.Fatalf("Expected 5 Taskruns to have executed but only got start times for %d Taskruns", len(times)) + taskRuns := taskRunsResp.Items + if len(taskRuns) != 5 { + t.Fatalf("Expected 5 TaskRuns to have executed but only got start times for %d TaskRuns", len(taskRuns)) } - sort.Sort(times) - - if !strings.HasPrefix(times[0].name, "dag-pipeline-run-pipeline-task-1") { - t.Errorf("Expected first task to execute first, but %q was first", times[0].name) - } - if !strings.HasPrefix(times[1].name, "dag-pipeline-run-pipeline-task-2") { - t.Errorf("Expected parallel tasks to run second & third, but %q was second", times[1].name) - } - if !strings.HasPrefix(times[2].name, "dag-pipeline-run-pipeline-task-2") { - t.Errorf("Expected parallel tasks to run second & third, but %q was third", times[2].name) - } - if !strings.HasPrefix(times[3].name, "dag-pipeline-run-pipeline-task-3") { - t.Errorf("Expected third task to execute third, but %q was third", times[3].name) + sort.Slice(taskRuns, func(i, j int) bool { + it := taskRuns[i].Status.StartTime.Time + jt := taskRuns[j].Status.StartTime.Time + return it.Before(jt) + }) + + wantPrefixes := []string{ + "dag-pipeline-run-pipeline-task-1", + // Could be task-2-parallel-1 or task-2-parallel-2 + "dag-pipeline-run-pipeline-task-2-parallel", + "dag-pipeline-run-pipeline-task-2-parallel", + "dag-pipeline-run-pipeline-task-3", + "dag-pipeline-run-pipeline-task-4", } - if !strings.HasPrefix(times[4].name, "dag-pipeline-run-pipeline-task-4") { - t.Errorf("Expected fourth task to execute fourth, but %q was fourth", times[4].name) + for i, wp := range wantPrefixes { + if !strings.HasPrefix(taskRuns[i].Name, wp) { + t.Errorf("Expected task %q to execute first, but %q was first", wp, taskRuns[0].Name) + } } // Check that the two tasks that can run in parallel did - parallelDiff := times[2].t.Sub(times[1].t) - if parallelDiff > (time.Second * 5) { - t.Errorf("Expected parallel tasks to execute more or less at the same time, but they were %v apart", parallelDiff) + s1 := taskRuns[1].Status.StartTime.Time + s2 := taskRuns[2].Status.StartTime.Time + absDiff := time.Duration(math.Abs(float64(s2.Sub(s1)))) + if absDiff > (time.Second * 5) { + t.Errorf("Expected parallel tasks to execute more or less at the same time, but they were %v apart", absDiff) } }