-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Description
Summary
While working on the unit tests for Pipelines-in-Pipelines (short PinP, TEP-0056 tracked in #8760) I noticed that refactoring the PipelineRun
reconciler test helpers and some of the tests themself in pipelinerun_test.go
will greatly benefit implementing future unit tests. Every refactoring which reduces the 19k lines of code is of great value.
I put the PinP tests which are not part of a table driven test cases in existing test functions in their own file here. This file will grow when more functionality (Params, Timeouts, Results, ...) is implemented.
I propose the following pipelinerun_test.go
refactoring:
- Refactor helpers to a testing framework.
- Break apart
pipelinerun_test.go
. - Rely on testing framework in
PipelineRun
reconciler unit tests. - Introduce Arrange-Act-Assert [1, 2] / Given-When-Then [1] test structure where reasonable.
- Update documentation.
Technical Design
1. Testing Framework
I see two main ways of structuring the framework. Order helper files, functions and data structures by:
- responsibility i.e.
factory.go
forPipeline/Task/CustomTask
mock data creation,envsetup.go
for setup preparation,execution.go
for runningReconcileKind
with and without errors,status.go
for status assertions/creations/transformations, etc. - Tekton resource i.e. child
PipelineRun, TaskRun and CustomRun
wheretesting/pinp/*
contains all PinP relevant helpers,testing/task/*
all task relevant helpers,testing/common/*
all shared helpers, etc.
I started using option 1 and here is its high level technical design:
Move helper functions from pipelinerun_test.go
to a structured testing framework in pkg/reconciler/testing
:
configmap.go
:ConfigMap
related helpers like creation, transformation, etc.factory.go
:Pipeline/Task/CustomTask, PipelineRun/TaskRun/CustomRun
mock data creation and transformation.status.go
:Status
creation, transformation and assertion.envsetup.go
: Test environment setup, everything you need to be able to execute a reconciliation; everything here should only be needed byexecution.go
.execution.go
: Reconciliation execution withreconcile
,reconcileWithError
, functions and their variants.assertion.go
: Collection of helpers which combine assertions/validations into one validation function, i.e.validatePinP
. Create such validation helpers for existing unit tests inpipelinerun_test.go
=> it will remove a lot of duplication.selectors.go
: Helpers which fetch the target resource from the test environment or select it from amap
orslice
, i.e.getTaskRunByName
,getTaskRunsForPipelineRun
,getChildPipelineRunByName
, etc.logger.go
: Test logger setup.common.go
:TaskRun/PipelineRun/CustomRun
count validators and remaining helpers which are not worthy of a dedicated file.- ... maybe a few more.
I recommend to move the easy and small helpers first and in the last step take care of the more difficult ones like assertion.go
and execution.go
.
2. Break Apart
Break down pipelinerun_test.go
in three or maybe four test files:
pipelinerun_pinp_test.go
,pipelinerun_task_test.go
,pipelinerun_customtask_test.go
,- maybe a
pipelinerun_test.go
orpipelinerun_common_test.go
with tests which do not belong to any of the three or apply to all of them => use your judgement.
3. Refactor Unit Tests
If not already partly done during the creation of the test framework start switching to the test framework in pipelinerun_*_test.go
.
4. Arrange Act Assert
With the new test framework in place a Arrange Act Assert structure of the tests should come quite naturally as in TestReconcile_ChildPipelineRunPipelineSpec
, TestReconcile_NestedChildPipelineRuns
, etc. Introduce this structure in existing unit tests where reasonable.
5. Documentation
The testing documentation should be updated with an introduction to the testing framework, how to best use it with links to example tests. Writing tests should become easy.
Parting Words
Write me in Slack if you need support or want to spar on technical details and implementation.
Originally posted by @twoGiants in #8850 (comment)
Metadata
Metadata
Assignees
Labels
Type
Projects
Status