From 434c47daaf623a595e2010ec966a7e6dbedb2df6 Mon Sep 17 00:00:00 2001 From: Andrea Frittoli Date: Fri, 25 Sep 2020 08:58:47 +0100 Subject: [PATCH] Use the test context in Reconcile tests The reconcile tests setup a test context along with other test assets using the test.controller module. The context however is not returned as part of the assets, so when we invoke `Reconcile` in reconciler tests we pass a fresh `context.Background()` which does not include the test logger. This causes log lines from taskrun.go and pipelinerun.go to be logged with a different format and with INFO log level instead of the DEBUG one required to troubleshoot test failures. This PR fixes that by adding the context to the test assets, and passing it to `Reconcile`. See https://github.com/tektoncd/pipeline/pull/3282#discussion_r494797820 for more context and results with a forced test failure. Follows up #3282, works towards #2992 Signed-off-by: Andrea Frittoli --- .../pipelinerun/pipelinerun_test.go | 7 ++--- pkg/reconciler/taskrun/taskrun_test.go | 27 ++++++++++--------- test/controller.go | 1 + 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index a1d2deaf39f..7a883e0f90c 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -152,6 +152,7 @@ func getPipelineRunController(t *testing.T, d test.Data) (test.Assets, func()) { Clients: c, Informers: informers, Recorder: controller.GetEventRecorder(ctx).(*record.FakeRecorder), + Ctx: ctx, }, cancel } @@ -747,7 +748,7 @@ func TestReconcile_InvalidPipelineRunNames(t *testing.T) { defer cancel() c := testAssets.Controller - err := c.Reconciler.Reconcile(context.Background(), tc.pipelineRun) + err := c.Reconciler.Reconcile(testAssets.Ctx, 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) @@ -1244,7 +1245,7 @@ func TestReconcileCancelledFailsTaskRunCancellation(t *testing.T) { return true, nil, fmt.Errorf("i'm sorry Dave, i'm afraid i can't do that") }) - err := c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-fails-to-cancel") + err := c.Reconciler.Reconcile(testAssets.Ctx, "foo/test-pipeline-fails-to-cancel") if err == nil { t.Errorf("Expected to see error returned from reconcile after failing to cancel TaskRun but saw none!") } @@ -3942,7 +3943,7 @@ func (prt PipelineRunTest) reconcileRun(namespace, pipelineRunName string, wantE c := prt.TestAssets.Controller clients := prt.TestAssets.Clients - reconcileError := c.Reconciler.Reconcile(context.Background(), namespace+"/"+pipelineRunName) + reconcileError := c.Reconciler.Reconcile(prt.TestAssets.Ctx, namespace+"/"+pipelineRunName) if permanentError { // When a PipelineRun is invalid and can't run, we expect a permanent error that will // tell the Reconciler to not keep trying to reconcile. diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 2aa36e46236..f14bcca7e38 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -300,6 +300,7 @@ func getTaskRunController(t *testing.T, d test.Data) (test.Assets, func()) { Clients: c, Informers: informers, Recorder: controller.GetEventRecorder(ctx).(*record.FakeRecorder), + Ctx: ctx, }, cancel } @@ -483,7 +484,7 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { c := testAssets.Controller clients := testAssets.Clients - if err := c.Reconciler.Reconcile(context.Background(), getRunName(tc.taskRun)); err != nil { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tc.taskRun)); err != nil { t.Errorf("expected no error. Got error %v", err) } if len(clients.Kube.Actions()) == 0 { @@ -668,7 +669,7 @@ func TestReconcile_FeatureFlags(t *testing.T) { }); err != nil { t.Fatal(err) } - if err := c.Reconciler.Reconcile(context.Background(), getRunName(tc.taskRun)); err != nil { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tc.taskRun)); err != nil { t.Errorf("expected no error. Got error %v", err) } if len(clients.Kube.Actions()) == 0 { @@ -753,7 +754,7 @@ func TestReconcile_CloudEvents(t *testing.T) { t.Fatal(err) } - if err := c.Reconciler.Reconcile(context.Background(), getRunName(taskRun)); err != nil { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err != nil { t.Errorf("expected no error. Got error %v", err) } if len(clients.Kube.Actions()) == 0 { @@ -1458,7 +1459,7 @@ func TestReconcile(t *testing.T) { t.Fatal(err) } - if err := c.Reconciler.Reconcile(context.Background(), getRunName(tc.taskRun)); err != nil { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tc.taskRun)); err != nil { t.Errorf("expected no error. Got error %v", err) } if len(clients.Kube.Actions()) == 0 { @@ -1616,7 +1617,7 @@ func TestReconcileInvalidTaskRuns(t *testing.T) { defer cancel() c := testAssets.Controller clients := testAssets.Clients - reconcileErr := c.Reconciler.Reconcile(context.Background(), getRunName(tc.taskRun)) + reconcileErr := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tc.taskRun)) // When a TaskRun is invalid and can't run, we return a permanent error because // a regular error will tell the Reconciler to keep trying to reconcile; instead we want to stop @@ -1676,7 +1677,7 @@ func TestReconcilePodFetchError(t *testing.T) { return true, nil, errors.New("induce failure fetching pods") }) - if err := c.Reconciler.Reconcile(context.Background(), getRunName(taskRun)); err == nil { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err == nil { t.Fatal("expected error when reconciling a Task for which we couldn't get the corresponding Pod but got nil") } } @@ -1734,7 +1735,7 @@ func TestReconcilePodUpdateStatus(t *testing.T) { c := testAssets.Controller clients := testAssets.Clients - if err := c.Reconciler.Reconcile(context.Background(), getRunName(taskRun)); err != nil { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err != nil { t.Fatalf("Unexpected error when Reconcile() : %v", err) } newTr, err := clients.Pipeline.TektonV1beta1().TaskRuns(taskRun.Namespace).Get(taskRun.Name, metav1.GetOptions{}) @@ -1770,7 +1771,7 @@ func TestReconcilePodUpdateStatus(t *testing.T) { // lister cache is update to reflect the result of the previous Reconcile. testAssets.Informers.TaskRun.Informer().GetIndexer().Add(newTr) - if err := c.Reconciler.Reconcile(context.Background(), getRunName(taskRun)); err != nil { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err != nil { t.Fatalf("Unexpected error when Reconcile(): %v", err) } @@ -1820,7 +1821,7 @@ func TestReconcileOnCompletedTaskRun(t *testing.T) { c := testAssets.Controller clients := testAssets.Clients - if err := c.Reconciler.Reconcile(context.Background(), getRunName(taskRun)); err != nil { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err != nil { t.Fatalf("Unexpected error when reconciling completed TaskRun : %v", err) } newTr, err := clients.Pipeline.TektonV1beta1().TaskRuns(taskRun.Namespace).Get(taskRun.Name, metav1.GetOptions{}) @@ -1852,7 +1853,7 @@ func TestReconcileOnCancelledTaskRun(t *testing.T) { c := testAssets.Controller clients := testAssets.Clients - if err := c.Reconciler.Reconcile(context.Background(), getRunName(taskRun)); err != nil { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err != nil { t.Fatalf("Unexpected error when reconciling completed TaskRun : %v", err) } newTr, err := clients.Pipeline.TektonV1beta1().TaskRuns(taskRun.Namespace).Get(taskRun.Name, metav1.GetOptions{}) @@ -1967,7 +1968,7 @@ func TestReconcileTimeouts(t *testing.T) { c := testAssets.Controller clients := testAssets.Clients - if err := c.Reconciler.Reconcile(context.Background(), getRunName(tc.taskRun)); err != nil { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tc.taskRun)); err != nil { t.Fatalf("Unexpected error when reconciling completed TaskRun : %v", err) } newTr, err := clients.Pipeline.TektonV1beta1().TaskRuns(tc.taskRun.Namespace).Get(tc.taskRun.Name, metav1.GetOptions{}) @@ -2224,7 +2225,7 @@ func TestReconcileCloudEvents(t *testing.T) { t.Fatal(err) } - if err := c.Reconciler.Reconcile(context.Background(), getRunName(tc.taskRun)); err != nil { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tc.taskRun)); err != nil { t.Errorf("expected no error. Got error %v", err) } @@ -2566,7 +2567,7 @@ func TestReconcileTaskResourceResolutionAndValidation(t *testing.T) { clients := testAssets.Clients c := testAssets.Controller - reconcileErr := c.Reconciler.Reconcile(context.Background(), getRunName(tt.d.TaskRuns[0])) + reconcileErr := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tt.d.TaskRuns[0])) // When a TaskRun is invalid and can't run, we return a permanent error because // a regular error will tell the Reconciler to keep trying to reconcile; instead we want to stop diff --git a/test/controller.go b/test/controller.go index 939b79b628f..2c4fdf12191 100644 --- a/test/controller.go +++ b/test/controller.go @@ -102,6 +102,7 @@ type Assets struct { Clients Clients Informers Informers Recorder *record.FakeRecorder + Ctx context.Context } func AddToInformer(t *testing.T, store cache.Store) func(ktesting.Action) (bool, runtime.Object, error) {