Skip to content

Commit

Permalink
Re-enable TestReconcile_InvalidPipelineRunNames…
Browse files Browse the repository at this point in the history 
… and remove fakeRecorder from tests as it is no more used

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
  • Loading branch information
@vdemeester @tekton-robot
vdemeester authored and tekton-robot committed Jul 10, 2019
1 parent e8c6ad1 commit 4f15cb8
Showing 1 changed file with 13 additions and 82 deletions.
95 changes: 13 additions & 82 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,29 +37,15 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ktesting "k8s.io/client-go/testing"
"k8s.io/client-go/tools/record"
)

func getRunName(pr *v1alpha1.PipelineRun) string {
return strings.Join([]string{pr.Namespace, pr.Name}, "/")
}

func validateNoEvents(t *testing.T, r *record.FakeRecorder) {
t.Helper()
timer := time.NewTimer(1 * time.Second)

select {
case event := <-r.Events:
t.Errorf("Expected no event but got %s", event)
case <-timer.C:
return
}
}

// getPipelineRunController returns an instance of the PipelineRun controller/reconciler that has been seeded with
// d, where d represents the state of the system (existing resources) needed for the test.
// recorder can be a fake recorder that is used for testing
func getPipelineRunController(t *testing.T, d test.Data, recorder record.EventRecorder) (test.TestAssets, func()) {
func getPipelineRunController(t *testing.T, d test.Data) (test.TestAssets, func()) {
ctx, _ := rtesting.SetupFakeContext(t)
c, _ := test.SeedTestData(t, ctx, d)
configMapWatcher := configmap.NewInformedWatcher(c.Kube, system.GetNamespace())
Expand Down Expand Up @@ -174,10 +160,7 @@ func TestReconcile(t *testing.T) {
PipelineResources: rs,
}

// create fake recorder for testing
fr := record.NewFakeRecorder(1)

testAssets, cancel := getPipelineRunController(t, d, fr)
testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients
Expand All @@ -186,9 +169,6 @@ func TestReconcile(t *testing.T) {
t.Fatalf("Error reconciling: %s", err)
}

// make sure there is no failed events
validateNoEvents(t, fr)

if len(clients.Pipeline.Actions()) == 0 {
t.Fatalf("Expected client to have been used to create a TaskRun but it wasn't")
}
Expand Down Expand Up @@ -326,12 +306,9 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) {
},
}

// Create fake recorder for testing
fr := record.NewFakeRecorder(1)

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
testAssets, cancel := getPipelineRunController(t, d, fr)
testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller

Expand All @@ -342,9 +319,6 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) {
// an error will tell the Reconciler to keep trying to reconcile; instead we want to stop
// and forget about the Run.

// make sure there is no failed events
validateNoEvents(t, fr)

if tc.pipelineRun.Status.CompletionTime == nil {
t.Errorf("Expected a CompletionTime on invalid PipelineRun but was nil")
}
Expand All @@ -361,7 +335,6 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) {
}
}

/*
func TestReconcile_InvalidPipelineRunNames(t *testing.T) {
invalidNames := []string{
"foo/test-pipeline-run-doesnot-exist",
Expand All @@ -370,43 +343,30 @@ func TestReconcile_InvalidPipelineRunNames(t *testing.T) {
tcs := []struct {
name string
pipelineRun string
log string
}{
{
name: "invalid-pipeline-run-shd-stop-reconciling",
pipelineRun: invalidNames[0],
log: "pipeline run \"foo/test-pipeline-run-doesnot-exist\" in work queue no longer exists",
}, {
name: "invalid-pipeline-run-name-shd-stop-reconciling",
pipelineRun: invalidNames[1],
log: "invalid resource key: test/invalidformat/t",
},
}

// create fake recorder for testing
fr := record.NewFakeRecorder(1)
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
testAssets, cancel := getPipelineRunController(t, test.Data{}, fr)
testAssets, cancel := getPipelineRunController(t, test.Data{})
defer cancel()
c := testAssets.Controller
logs := testAssets.Logs

err := c.Reconciler.Reconcile(context.Background(), tc.pipelineRun)
// No reason to keep reconciling something that doesnt or can't exist
if err != nil {
t.Errorf("Did not expect to see error when reconciling invalid PipelineRun but saw %q", err)
}
if logs.FilterMessage(tc.log).Len() == 0 {
m := test.GetLogMessages(logs)
t.Errorf("Log lines diff %s", cmp.Diff(tc.log, m))
}
})
}
}
*/

func TestUpdateTaskRunsState(t *testing.T) {
pr := tb.PipelineRun("test-pipeline-run", "foo", tb.PipelineRunSpec("test-pipeline"))
Expand Down Expand Up @@ -500,10 +460,7 @@ func TestReconcileOnCompletedPipelineRun(t *testing.T) {
TaskRuns: trs,
}

// create fake recorder for testing
fr := record.NewFakeRecorder(1)

testAssets, cancel := getPipelineRunController(t, d, fr)
testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients
Expand All @@ -512,9 +469,6 @@ func TestReconcileOnCompletedPipelineRun(t *testing.T) {
t.Fatalf("Error reconciling: %s", err)
}

// make sure there is no failed events
validateNoEvents(t, fr)

if len(clients.Pipeline.Actions()) != 1 {
t.Fatalf("Expected client to have updated the TaskRun status for a completed PipelineRun, but it did not")
}
Expand Down Expand Up @@ -587,10 +541,7 @@ func TestReconcileOnCancelledPipelineRun(t *testing.T) {
TaskRuns: trs,
}

// create fake recorder for testing
fr := record.NewFakeRecorder(1)

testAssets, cancel := getPipelineRunController(t, d, fr)
testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients
Expand Down Expand Up @@ -636,10 +587,7 @@ func TestReconcileWithTimeout(t *testing.T) {
Tasks: ts,
}

// create fake recorder for testing
fr := record.NewFakeRecorder(2)

testAssets, cancel := getPipelineRunController(t, d, fr)
testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients
Expand Down Expand Up @@ -693,10 +641,7 @@ func TestReconcileWithoutPVC(t *testing.T) {
Tasks: ts,
}

// create fake recorder for testing
fr := record.NewFakeRecorder(2)

testAssets, cancel := getPipelineRunController(t, d, fr)
testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients
Expand Down Expand Up @@ -744,10 +689,7 @@ func TestReconcileCancelledPipelineRun(t *testing.T) {
Tasks: ts,
}

// create fake recorder for testing
fr := record.NewFakeRecorder(2)

testAssets, cancel := getPipelineRunController(t, d, fr)
testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients
Expand Down Expand Up @@ -841,10 +783,7 @@ func TestReconcilePropagateLabels(t *testing.T) {
Tasks: ts,
}

// create fake recorder for testing
fr := record.NewFakeRecorder(2)

testAssets, cancel := getPipelineRunController(t, d, fr)
testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients
Expand Down Expand Up @@ -896,10 +835,7 @@ func TestReconcileWithDifferentServiceAccounts(t *testing.T) {
Tasks: ts,
}

// create fake recorder for testing
fr := record.NewFakeRecorder(2)

testAssets, cancel := getPipelineRunController(t, d, fr)
testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients
Expand Down Expand Up @@ -1029,9 +965,7 @@ func TestReconcileWithTimeoutAndRetry(t *testing.T) {
TaskRuns: trs,
}

fr := record.NewFakeRecorder(2)

testAssets, cancel := getPipelineRunController(t, d, fr)
testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients
Expand Down Expand Up @@ -1079,10 +1013,7 @@ func TestReconcilePropagateAnnotations(t *testing.T) {
Tasks: ts,
}

// create fake recorder for testing
fr := record.NewFakeRecorder(2)

testAssets, cancel := getPipelineRunController(t, d, fr)
testAssets, cancel := getPipelineRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients
Expand Down

0 comments on commit 4f15cb8

Please sign in to comment.