Migrate PipelineRun Reconciler Tests to use YAMLParser #4610

jerop · 2022-02-22T22:50:55Z

YAML parsers take a YAML in a string and returns respective deserialized objects including Tasks, TaskRuns, Pipelines and PipelineRun - thank you @imjasonh 🎉

These parsers can help us simplify the many tests we have in pipelinerun_test.go, and make it easier to add more reconciler tests so that we can safely make changes to the reconciler, such as refactoring and supporting TEP-0090: Matrix.

Because the functions are long, we will use this issue to claim tests (to avoid duplicate work) and track progress. We'd appreciate any contributions to this migration 😸

cc @pritidesai

In addition to the YAML Parser migration, move these three tests to pipelinerunstate_test.go - these three tests are mainly testing func (state PipelineRunState) GetTaskRunsStatus:

TestUpdateTaskRunsState - cleanup - refactor taskRunStatus and runStatus unit tests #4632
TestUpdateRunsState - cleanup - refactor taskRunStatus and runStatus unit tests #4632
TestUpdateTaskRunStateWithConditionChecks - cleanup - refactor taskRunStatus and runStatus unit tests #4632

The text was updated successfully, but these errors were encountered:

lbernick · 2022-02-23T03:04:34Z

/label "help wanted"

pritidesai · 2022-02-23T04:46:26Z

/help
/good-first-issue

tekton-robot · 2022-02-23T04:46:27Z

@pritidesai:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/help
/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jbpratt · 2022-02-25T14:52:55Z

I don't think TestReconcile_InvalidPipelineRunNames has any updates required

pipeline/pkg/reconciler/pipelinerun/pipelinerun_test.go

Lines 1471 to 1490 in 9c529d3

    
           func TestReconcile_InvalidPipelineRunNames(t *testing.T) { 
        
           	// TestReconcile_InvalidPipelineRunNames runs "Reconcile" on several PipelineRuns that have invalid names. 
        
           	// It verifies that reconcile fails, how it fails and which events are triggered. 
        
           	// Note that the code tested here is part of the genreconciler. 
        
           	invalidNames := []string{ 
        
           		"foo/test-pipeline-run-doesnot-exist", 
        
           		"test/invalidformat/t", 
        
           	} 
        
           	tcs := []struct { 
        
           		name        string 
        
           		pipelineRun string 
        
           	}{ 
        
           		{ 
        
           			name:        "invalid-pipeline-run-shd-stop-reconciling", 
        
           			pipelineRun: invalidNames[0], 
        
           		}, { 
        
           			name:        "invalid-pipeline-run-name-shd-stop-reconciling", 
        
           			pipelineRun: invalidNames[1], 
        
           		}, 
        
           	}

pritidesai · 2022-02-25T16:57:43Z

I don't think TestReconcile_InvalidPipelineRunNames has any updates required

Indeed, there are no changed needed in this test as part of this migration, thank you!

Refactor the pipelineRun and taskRun create helpers to parse yaml. Requires passing `*testing.T` through all calls. continued work on tektoncd#4610 Signed-off-by: jbpratt <[email protected]>

Refactor the pipelineRun and taskRun create helpers to parse yaml. Requires passing `*testing.T` through all calls. continued work on #4610 Signed-off-by: jbpratt <[email protected]>

This doesn't switch everything - the "global" tasks and other resources are left as is, for example. But it does move most of the explicit `Task` and `TaskRun` structs to parsed YAML instead. Tangentially related to tektoncd#4610. =) Signed-off-by: Andrew Bayer <[email protected]>

This doesn't switch everything - the "global" tasks and other resources are left as is, for example. But it does move most of the explicit `Task` and `TaskRun` structs to parsed YAML instead. Tangentially related to #4610. =) Signed-off-by: Andrew Bayer <[email protected]>

This doesn't switch everything - the "global" tasks and other resources are left as is, for example. But it does move most of the explicit `Task` and `TaskRun` structs to parsed YAML instead. Tangentially related to tektoncd#4610. =) Signed-off-by: Andrew Bayer <[email protected]>

tekton-robot · 2022-06-13T16:49:07Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

jerop · 2022-06-13T16:55:13Z

/lifecycle frozen

vsinghai · 2022-06-15T20:26:18Z

Wanted to bring to light that the YAML Parser cannot be used for tests in the v1beta1 package due to a circular dependency within the mustParse() function.

JeromeJu · 2022-06-16T13:32:34Z

Noticed that most of the tests have been refactored thanks to #4749.
I would love to help update the checkboxes if necessary.

jerop · 2022-06-16T13:35:44Z

Noticed that most of the tests have been refactored thanks to #4749. I would love to help update the checkboxes if necessary.

yes please, thanks @JeromeJu

JeromeJu · 2022-07-04T20:09:20Z

talife · 2022-12-02T08:43:09Z

Here's an updated list I compiled.
For TestUpdatePipelineRunStatusFromTaskRuns, mustParsePipelineRunTaskRunStatus is inside pipelinerun_updatestatus_test.go. Should the parser be extended instead ?

TestReconcile (@pritidesai) - cleaning up a unit test to use YAMLParser #4609
TestReconcile_CustomTask - [TEP-0100] Prepare for testing of minimal status implementation
TestReconcile_PipelineSpecTaskSpec - refactor TestReconcile_PipelineSpecTaskSpec objects #4622
TestReconcile_InvalidPipelineRuns - cleaning up a unit test InvalidPipelineRuns to use YAML parser #4616
~~TestReconcile_InvalidPipelineRunNames~~ - no changes needed
TestReconcileOnCompletedPipelineRun - Update a number of PipelineRun reconciler tests with parsed YAML
~~TestReconcileOnCancelledPipelineRunDeprecated~~ - outdated
~~TestReconcileOnCancelledPipelineRun~~ - no changes needed
TestReconcileForCustomTaskWithPipelineTaskTimedOut - @anu-baskar is working on this!
TestReconcileForCustomTaskWithPipelineRunTimedOut - Update a number of PipelineRun reconciler tests with parsed YAML
~~TestReconcileOnCancelledRunFinallyPipelineRun~~ - no changes needed
TestReconcileOnCancelledRunFinallyPipelineRunWithFinalTask - clean up reconcile unit test to use YAML parser
TestReconcileOnCancelledRunFinallyPipelineRunWithRunningFinalTask - Update a number of PipelineRun reconciler tests with parsed YAML
~~TestReconcileOnCancelledRunFinallyPipelineRunWithFinalTaskAndRetries~~ - no changes needed
~~TestReconcileCancelledRunFinallyFailsTaskRunCancellation~~ - outdated
TestReconcileTaskResolutionError - Migrate PipelineRun Reconciler__TestReconcileTaskResolutionError
~~TestReconcileOnStoppedRunFinallyPipelineRun~~ - outdated
~~TestReconcileOnStoppedRunFinallyPipelineRunWithRunningTask~~ - outdated
~~TestReconcileOnStoppedPipelineRunWithCompletedTask~~ - outdated
TestReconcileOnPendingPipelineRun - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
~~TestReconcileWithTimeout~~ - outdated
TestReconcileWithoutPVC - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
TestReconcileCancelledFailsTaskRunCancellation - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
~~TestReconcileCancelledPipelineRun~~ - outdated
~~TestReconcilePropagateLabels~~ - oudated
~~TestReconcilePropagateLabelsPending~~ - outdated
~~TestReconcilePropagateLabelsCancelled~~ - outdated
TestReconcileWithDifferentServiceAccounts - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
TestReconcileCustomTasksWithDifferentServiceAccounts - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
TestReconcileWithTimeoutAndRetry - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
~~TestReconcilePropagateAnnotations~~ - outdated
~~TestGetTaskRunTimeout~~ - outdated
~~TestGetFinallyTaskRunTimeout~~ - outdated
~~TestReconcileAndPropagateCustomPipelineTaskRunSpec~~ - outdated
TestReconcileCustomTasksWithTaskRunSpec - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
~~TestReconcileWithConditionChecks~~ - outdated
~~TestReconcileWithFailingConditionChecks~~ - outdated
~~TestReconcileWithWhenExpressionsWithParameters~~ - outdated
~~TestReconcileWithWhenExpressionsWithTaskResults~~ - outdated
~~TestReconcileWithWhenExpressionsScopedToTask~~ - outdated
~~TestReconcileWithWhenExpressionsScopedToTaskWitResultRefs~~ - outdated
TestReconcileWithAffinityAssistantStatefulSet - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
TestReconcileWithVolumeClaimTemplateWorkspace - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
TestReconcileWithVolumeClaimTemplateWorkspaceUsingSubPaths - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
TestReconcileWithTaskResults - Update a number of PipelineRun reconciler tests with parsed YAML
TestReconcileWithTaskResultsEmbeddedNoneStarted - Update a number of PipelineRun reconciler tests with parsed YAML
TestReconcileWithPipelineResults - Add support for finally task results in pipeline results
~~Test_storePipelineSpec~~ - outdated
TestReconcileOutOfSyncPipelineRun - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
TestUpdatePipelineRunStatusFromInformer - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
TestUpdatePipelineRunStatusFromTaskRuns
~~TestReconcilePipeline_FinalTasks~~ - no changes needed
TestReconcile_CloudEvents - clean up TestReconcile_CloudEvents YAML parsing #4619
TestReconcilePipeline_TaskSpecMetadata - refactor reconcile pipeline TaskSpecMetadata test
TestReconciler_ReconcileKind_PipelineTaskContext - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
TestReconcileWithTaskResultsInFinalTasks - Update a number of PipelineRun reconciler tests with parsed YAML
TestReconcile_RemotePipelineRef - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
TestReconcile_OptionalWorkspacesOmitted - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing
TestReconcile_DependencyValidationsImmediatelyFailPipelineRun - Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing

In addition to the YAML Parser migration, move these three tests to pipelinerunstate_test.go - these three tests are mainly testing func (state PipelineRunState) GetTaskRunsStatus:

TestUpdateTaskRunsState - cleanup - refactor taskRunStatus and runStatus unit tests #4632
TestUpdateRunsState - cleanup - refactor taskRunStatus and runStatus unit tests #4632
TestUpdateTaskRunStateWithConditionChecks - cleanup - refactor taskRunStatus and runStatus unit tests #4632

jerop · 2022-12-13T14:45:54Z

Migration was completed, thank you for the contributions!

jerop added area/epic Issues that should be considered as Epics (aka multiple sub-tasks, …) kind/misc Categorizes issue or PR as a miscellaneuous one. labels Feb 22, 2022

tekton-robot added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Feb 23, 2022

pritidesai mentioned this issue Feb 24, 2022

cleaning up a unit test InvalidPipelineRuns to use YAML parser #4616

Merged

5 tasks

This was referenced Feb 25, 2022

clean up TestReconcile_CloudEvents YAML parsing #4619

Merged

refactor TestReconcile_PipelineSpecTaskSpec objects #4622

Merged

This was referenced Feb 26, 2022

test reconcile on cancelled refactor #4628

Merged

clean up reconcile unit test to use YAML parser #4637

Merged

jbpratt mentioned this issue Mar 17, 2022

format pipelinerun_test.go #4681

Closed

5 tasks

jbpratt mentioned this issue Mar 17, 2022

refactor create helpers in reconcile pr tests #4682

Merged

5 tasks

jbpratt mentioned this issue Mar 18, 2022

refactor reconcile pipeline TaskSpecMetadata test #4693

Merged

5 tasks

lbernick mentioned this issue Apr 12, 2022

Update a number of PipelineRun reconciler tests with parsed YAML #4748

Merged

5 tasks

abayer mentioned this issue Apr 12, 2022

Switch (almost all of) the rest of pipelinerun_test.go to YAML parsing #4749

Merged

5 tasks

abayer mentioned this issue Apr 12, 2022

Switch to YAML parsing in much of taskrun_test.go #4751

Merged

4 tasks

tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 13, 2022

tekton-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 13, 2022

tekton-robot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Jun 13, 2022

xchapter7x added this to Tekton Community Roadmap Sep 20, 2022

xchapter7x moved this to Todo in Tekton Community Roadmap Sep 20, 2022

my-git9 mentioned this issue Oct 16, 2022

migrate PipelineRun to use YAMLParser Reconciler_TestReconcileTaskResolutionError #5644

Merged

7 tasks

jerop closed this as completed Dec 13, 2022

Repository owner moved this from Todo to Done in Tekton Community Roadmap Dec 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate PipelineRun Reconciler Tests to use YAMLParser #4610

Migrate PipelineRun Reconciler Tests to use YAMLParser #4610

jerop commented Feb 22, 2022 •

edited

Loading

lbernick commented Feb 23, 2022

pritidesai commented Feb 23, 2022

tekton-robot commented Feb 23, 2022

jbpratt commented Feb 25, 2022 •

edited

Loading

pritidesai commented Feb 25, 2022

tekton-robot commented Jun 13, 2022

jerop commented Jun 13, 2022

vsinghai commented Jun 15, 2022

JeromeJu commented Jun 16, 2022 •

edited

Loading

jerop commented Jun 16, 2022

JeromeJu commented Jul 4, 2022 •

edited

Loading

talife commented Dec 2, 2022 •

edited

Loading

jerop commented Dec 13, 2022

Migrate PipelineRun Reconciler Tests to use YAMLParser #4610

Migrate PipelineRun Reconciler Tests to use YAMLParser #4610

Comments

jerop commented Feb 22, 2022 • edited Loading

lbernick commented Feb 23, 2022

pritidesai commented Feb 23, 2022

tekton-robot commented Feb 23, 2022

jbpratt commented Feb 25, 2022 • edited Loading

pritidesai commented Feb 25, 2022

tekton-robot commented Jun 13, 2022

jerop commented Jun 13, 2022

vsinghai commented Jun 15, 2022

JeromeJu commented Jun 16, 2022 • edited Loading

jerop commented Jun 16, 2022

JeromeJu commented Jul 4, 2022 • edited Loading

talife commented Dec 2, 2022 • edited Loading

jerop commented Dec 13, 2022

jerop commented Feb 22, 2022 •

edited

Loading

jbpratt commented Feb 25, 2022 •

edited

Loading

JeromeJu commented Jun 16, 2022 •

edited

Loading

JeromeJu commented Jul 4, 2022 •

edited

Loading

talife commented Dec 2, 2022 •

edited

Loading