[TEP-0100] Prepare for testing of minimal status implementation #4734

abayer · 2022-04-04T15:17:35Z

Changes

This change is to minimize the size of the actual implementation. We need to change a number of
tests in pkg/reconciler/pipelinerun/pipelinerun_test.go to be table-based, so that we can test
behavior under each possible value for the new embedded-status feature flag. Here, we just
modify the relevant tests to be table-based and use a parameterized helper function for the actual
execution, with a test case for each value.

It also adds helper functions for checking the relevant fields in the PipelineRun status, which
are hard-coded to always handle the current, "full" embedded status approach.

This also splits out TestUpdatePipelineRunStatusFromTaskRuns and TestUpdatePipelineRunStatusFromRuns
into a separate file, pipelinerun_updatestatus_test.go. When the TEP-0100 implementation lands,
this will also contain additional tests for updating via child references. Splitting like this helps
keep pipelinerun_test.go from getting even more bloated than it is currently.

Until the implementation, these table-based tests are purely duplicative - they're going to run the
same and check the same things for any value of embedded-status, but the implementation PR will be
cleaner, only adding the implementation and changing the helper functions to take all of the
possible "embedded-status" values into account. The changes which need to be made in the
implementation PR are all marked with // TODO(abayer): ... in pipelinerun_test.go and
pipelinerun_updatestatus_test.go.

/kind tep

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Docs included if any changes are user facing
Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Release notes block below has been filled in
(if there are no user facing changes, use release note "NONE")

Release Notes

NONE

abayer · 2022-04-04T15:17:45Z

/assign @lbernick

lbernick

Thanks Andrew! I'm still wondering if a pattern like the following could work?

func verifyPipelineRunStatus(t *testing.T, embeddedStatus string, prs *PipelineRunStatus, expectedChildStatuses map[string]ChildStatusReference) {
 //pseudocode
 taskRunStatuses := filter(expectedChildStatuses, status.value.kind == "TaskRun")
 runStatuses := filter(expectedChildStatuses, status.value.kind == "Run")
 if embeddedStatus == "full" {
   // verify all taskRuns in pipelineRunStatus have status, conditions, and when expressions matching those in taskRunStatuses, and likewise for runs
   // verify that childRefs has no entries
 }
 // do something similar for both/minimal
}

lbernick · 2022-04-05T11:38:41Z

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -238,6 +238,36 @@ func getPipelineRunUpdates(t *testing.T, actions []ktesting.Action) []*v1beta1.P
 }

 func TestReconcile(t *testing.T) {
+	testCases := []struct {


I see a lot of tests with these same test cases. They could probably be combined into one, e.g.

testSuites := []func(*testing.T){testReconcileWithEmbeddedStatus, testReconcileWithCustomTask...} for _, testCase := range testCases { for _, testClass := range testSuites { t.Run(tc.Name, testClass(t, tc.embeddedStatus)) } }

Good question. I’d lean against this for clarity and ease of running individual test cases in an IDE.

at the very least maybe the test cases struct can be defined only once?

I considered that, but at least in Goland/IntelliJ, if you don't have the test case structs defined in the test function, you can't launch them individually.

lbernick · 2022-04-05T11:40:48Z

pkg/reconciler/pipelinerun/pipelinerun_test.go

+// with one Custom Task reference that has not been run yet, and each possible
+// value for the "embedded-status" feature flag. It verifies that the Run is
+// created, it checks the resulting API actions, status and events.
+func TestReconcile_CustomTaskWithEmbeddedStatus(t *testing.T) {


can this follow the same pattern as Test_Reconcile above?

Do you mean the separate function thingie? I didn’t bother splitting this one because it’s entirely new, so the diff would have been just as large either way. I guess there is value in consistency, though!

yeah I think absent a strong reason to prefer either approach, let's keep things consistent

lbernick · 2022-04-05T11:44:52Z

pkg/reconciler/pipelinerun/pipelinerun_test.go

+}
+
+func verifyTaskRunStatusesCount(t *testing.T, embeddedStatus string, prStatus v1beta1.PipelineRunStatus, taskCount int) {
+	if shouldHaveFullEmbeddedStatus(embeddedStatus) && len(prStatus.TaskRuns) != taskCount {


rather than creating these functions "shouldhavefullembeddedstatus" etc I think it might be cleaner to not have these branches at all, and just add them when the embedded status functionality is enabled.

I’m not sure what you mean? The point of this PR is to cut down on the size of the implementation PR, so if we don’t add the new test behavior for the embedded status functionality here, we have to do it in the implementation PR instead.

ok-- whatever you think is best here, although my intention isn't specifically to reduce number of lines of code but number of "things" that a given PR is doing

Fair enough. =) I guess I just feel like stubbing shouldHaveFullEmbeddedStatus and shouldHaveMinimalEmbeddedStatus here with the TODOs, and full implementations of verify... is clearer on the intent, particularly since there (hopefully!) will not be a long gap between this PR landing and the implementation PR landing.

abayer · 2022-04-05T12:38:06Z

@lbernick Re that pattern - I feel like that would end up bloating the tests more than it would shrink them, since we’d have to define the full expected statuses for each case, while the existing tests don’t actually check the full statuses currently, just certain aspects. But I’ll give it another shot and see if a smooth enough approach reveals itself.

lbernick · 2022-04-05T13:17:26Z

pkg/reconciler/pipelinerun/pipelinerun_test.go

+	// return embeddedVal == config.MinimalEmbeddedStatus || embeddedVal == config.BothEmbeddedStatus
+}
+
+func verifyTaskRunStatusesCount(t *testing.T, embeddedStatus string, prStatus v1beta1.PipelineRunStatus, taskCount int) {


just one more comment-- t.Helper() would be useful here

Good catch!

vdemeester

We should make sure we aim for readability in our tests, no matter the size of the PR. I'd rather have a very readable set of tests in 2000 lines than a not-that-readable set of tests in 1000 lines.

Few things:

Can we use the test/pars package and inline yaml definitions, it's usually easier to read ? Closer to what we "see" in real life.
If we don't have those for some types (status, …), we could "enhance" that library
"duplication is far cheaper than the wrong abstraction", especially on tests that we have to maintain over time.

abayer · 2022-04-05T17:46:07Z

pkg/reconciler/pipelinerun/pipelinerun_updatestatus_test.go

+	simple         map[string]*v1beta1.PipelineRunTaskRunStatus
+}
+
+func getUpdateStatusTaskRunsData() updateStatusTaskRunsData {


I should note that this as a separate function/struct makes more sense if you see the tests added in my WIP branch for the implementation - there, we want to be able to test the function that calls either updatePipelineRunStatusFromTaskRuns and updatePipelineRunStatusFromRuns or the added-in-the-impl updatePipelineRunStatusFromChildRefs (or all of 'em, for embedded-status=both). It didn't make sense to me to copy-paste the structs I end up using in both the existing test and the new one when I could just generate them in a function both the tests can call. I went with a struct as the return from that function rather than a map just because I thought it'd be more clear, but I'm not married to that choice. =)

abayer · 2022-04-05T17:47:44Z

@vdemeester I'm probably going to come back later and see what makes sense to switch to parsed YAML, but I don't want to make this PR any more sprawling/confusing than it already is. =)

lbernick · 2022-04-05T18:17:59Z

/test pull-tekton-pipeline-alpha-integration-tests

lbernick · 2022-04-05T18:20:31Z

pkg/reconciler/pipelinerun/pipelinerun_test.go

 		})
 	}
 }

+func runTestReconcileCustomTaskWithEmbeddedStatus(t *testing.T, embeddedStatus string) {


it looks like this function may accidentally have been duplicated with the one above?

Ah, yeah, I think I added this one while initially prototyping, before the table-based changes. I'm gonna remove this.

Check that - I've got TestReconcile_CustomTaskWithEmbeddedStatus and TestReconcile_CustomTask because TestReconcile_CustomTask is already a table-based test and I really didn't want to nest it even further. Lemme think on what's the best way to rearrange these two tests to get the right coverage but not be so duplicative.

Ok, how does https://github.com/tektoncd/pipeline/pull/4734/files#diff-a64a19b939f791920d235cbf5bf2e4905d4814af06f21401b27af11351afbd0bR730 look? I've got it in a separate commit right now so that I can just nuke it easily if it doesn't feel right. =) I decided to skip testing both here because...well, it felt a little redundant.

I can't tell where that links to, but I think the pattern you have going of func TestReconcile(t *testing.T) + func runTestReconcileWithEmbeddedStatus(t *testing.T, embeddedStatus string) works well, and it would also be nice to use it here for consistency. can you explain a bit more about the concern with doing it that way?

Ah, yeah, you have to expand the diff to see it.

So TestReconcile_CustomTask is already a table-based test - I didn't want to do the matrix-y thing of running each of its testcases once for each embedded-status value, so instead I added two new test cases to TestReconcile_CustomTask, using the same input PipelineRun/expected Run as the existing "custom task with taskref" test case, but with embedded-status=full and embedded-status=minimal. So it doesn't fit exactly the same pattern as the runTest... ones, but I think it's a better fit for this particular test.

Honestly, in practice, we could probably skip having test cases in TestReconcile_CustomTask for different embedded-status values, since we test reconciliation of a Run with the different embedded-status values in runTestUpdatePipelineRunStatusFromInformer now anyway.

lbernick · 2022-04-05T18:22:47Z

pkg/reconciler/pipelinerun/pipelinerun_test.go

+	return newCM
+}
+
+func TestReconcileOnCancelledPipelineRunFullEmbeddedStatus(t *testing.T) {


This test case could likely also use the pattern you're establishing (parameterizing the embedded status), because it doesn't really check any parts of the child statuses

Yeah, good catch.

abayer · 2022-04-05T19:00:48Z

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -702,67 +730,68 @@ func TestReconcile_CustomTask(t *testing.T) {
 	const pipelineRunName = "test-pipelinerun"
 	const pipelineTaskName = "custom-task"
 	const namespace = "namespace"
+
+	simpleCustomTaskPRYAML := `metadata:


This and simpleCustomTaskWantRunYAML are here so that we can use the same input PipelineRun and expected Run for test cases with different embedded-status values without duplicating everything.

lbernick · 2022-04-06T13:38:29Z

/lgtm

don't forget to squash the commits

This change is to minimize the size of the actual implementation. We need to change a number of tests in `pkg/reconciler/pipelinerun/pipelinerun_test.go` to be table-based, so that we can test behavior under each possible value for the new `embedded-status` feature flag. Here, we just modify the relevant tests to be table-based and use a parameterized helper function for the actual execution, with a test case for each value. It also adds helper functions for checking the relevant fields in the `PipelineRun` status, which are hard-coded to always handle the current, "full" embedded status approach. This also splits out `TestUpdatePipelineRunStatusFromTaskRuns` and `TestUpdatePipelineRunStatusFromRuns` into a separate file, `pipelinerun_updatestatus_test.go`. When the TEP-0100 implementation lands, this will also contain additional tests for updating via child references. Splitting like this helps keep `pipelinerun_test.go` from getting even more bloated than it is currently. Until the implementation, these table-based tests are purely duplicative - they're going to run the same and check the same things for any value of `embedded-status`, but the implementation PR will be cleaner, only adding the implementation and changing the helper functions to take all of the possible "embedded-status" values into account. The changes which need to be made in the implementation PR are all marked with `// TODO(abayer): ...` in `pipelinerun_test.go` and `pipelinerun_updatestatus_test.go`. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

abayer · 2022-04-06T13:46:23Z

Thanks, @lbernick! Squashed! =)

lbernick · 2022-04-06T13:50:54Z

/lgtm

…pelineRuns See: * https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md * tektoncd#4705 * tektoncd#4734 * tektoncd#3140 This implements TEP-0100, building on top of the flags/fields/docs changes in tektoncd#4705 and the test changes in tektoncd#4734. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

vdemeester · 2022-04-06T17:27:03Z

/approve

tekton-robot · 2022-04-06T17:27:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [vdemeester]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…pelineRuns See: * https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md * tektoncd#4705 * tektoncd#4734 * tektoncd#3140 This implements TEP-0100, building on top of the flags/fields/docs changes in tektoncd#4705 and the test changes in tektoncd#4734. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

…pelineRuns See: * https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md * tektoncd#4705 * tektoncd#4734 * tektoncd#3140 This implements TEP-0100, allowing for choosing between the original full embedded `TaskRun` and `Run` statuses in `PipelineRun` statuses, minimal child references to the underlying `TaskRun` and `Run`s, or both, building on top of the flags/fields/docs changes in tektoncd#4705 and the test changes in Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

…d status This builds on tektoncd#4694, tektoncd#4734, and tektoncd#4753. It will feed into a revamped tektoncd#4739, all as part of https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md and tektoncd#3140. Specifically, this adds functionality to `pkg/reconciler/pipelinerun/resources` in `pipelinerunresolution.go` and `pipelinerunstate.go` which will be needed for the full implementation. These changes won't have any effects in the current situation, because `pr.Status.ChildReferences` is never populated, so can be made independently of the rest of the implementation, thus also shrinking the size of the rest of the implementation PR(s) for easier review. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

…pelineRuns See: * https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md * tektoncd#4705 * tektoncd#4734 * tektoncd#3140 This implements TEP-0100, allowing for choosing between the original full embedded `TaskRun` and `Run` statuses in `PipelineRun` statuses, minimal child references to the underlying `TaskRun` and `Run`s, or both, building on top of the flags/fields/docs changes in tektoncd#4705 and the test changes in Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

…d status This builds on #4694, #4734, and #4753. It will feed into a revamped #4739, all as part of https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md and #3140. Specifically, this adds functionality to `pkg/reconciler/pipelinerun/resources` in `pipelinerunresolution.go` and `pipelinerunstate.go` which will be needed for the full implementation. These changes won't have any effects in the current situation, because `pr.Status.ChildReferences` is never populated, so can be made independently of the rest of the implementation, thus also shrinking the size of the rest of the implementation PR(s) for easier review. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

…pelineRuns See: * https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md * tektoncd#4705 * tektoncd#4734 * tektoncd#3140 This implements TEP-0100, allowing for choosing between the original full embedded `TaskRun` and `Run` statuses in `PipelineRun` statuses, minimal child references to the underlying `TaskRun` and `Run`s, or both, building on top of the flags/fields/docs changes in tektoncd#4705 and the test changes in Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

And of course, this is part of https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md, building on a pile of other PRs (tektoncd#4705, tektoncd#4734, tektoncd#4753, tektoncd#4757). This adds a new function to `pkg/reconciler/pipelinerun/pipelinerun.go`, specifically for updating `pr.Status.ChildReferences` during reconciliation. It's analogous to the existing `updatePipelineRunStatusFromTaskRuns` and `updatePipelineRunStatusFromRuns` functions. This PR doesn't actually call the new function - behavior is exactly the same. But it adds the new function, along with other functions it depends on. In the final step of the implementation, these other functions will also be used in `...FromTaskRuns` and/or `...FromRuns`. I also reworked `pkg/reconciler/pipelinerun/pipelinerun_updatestatus_test.go` to improve its test fixtures, so that they're easier to reuse and instantiated via YAML parsing as much as possible. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

And of course, this is part of https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md, building on a pile of other PRs (#4705, #4734, #4753, #4757). This adds a new function to `pkg/reconciler/pipelinerun/pipelinerun.go`, specifically for updating `pr.Status.ChildReferences` during reconciliation. It's analogous to the existing `updatePipelineRunStatusFromTaskRuns` and `updatePipelineRunStatusFromRuns` functions. This PR doesn't actually call the new function - behavior is exactly the same. But it adds the new function, along with other functions it depends on. In the final step of the implementation, these other functions will also be used in `...FromTaskRuns` and/or `...FromRuns`. I also reworked `pkg/reconciler/pipelinerun/pipelinerun_updatestatus_test.go` to improve its test fixtures, so that they're easier to reuse and instantiated via YAML parsing as much as possible. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

…pelineRuns See: * https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md * tektoncd#4705 * tektoncd#4734 * tektoncd#4753 * tektoncd#4757 * tektoncd#4760 * tektoncd#3140 This implements TEP-0100, allowing for choosing between the original full embedded `TaskRun` and `Run` statuses in `PipelineRun` statuses, minimal child references to the underlying `TaskRun` and `Run`s, or both, building on top of all the other PRs referenced above. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

…pelineRuns See: * https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md * #4705 * #4734 * #4753 * #4757 * #4760 * #3140 This implements TEP-0100, allowing for choosing between the original full embedded `TaskRun` and `Run` statuses in `PipelineRun` statuses, minimal child references to the underlying `TaskRun` and `Run`s, or both, building on top of all the other PRs referenced above. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

…d status This builds on tektoncd#4694, tektoncd#4734, and tektoncd#4753. It will feed into a revamped tektoncd#4739, all as part of https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md and tektoncd#3140. Specifically, this adds functionality to `pkg/reconciler/pipelinerun/resources` in `pipelinerunresolution.go` and `pipelinerunstate.go` which will be needed for the full implementation. These changes won't have any effects in the current situation, because `pr.Status.ChildReferences` is never populated, so can be made independently of the rest of the implementation, thus also shrinking the size of the rest of the implementation PR(s) for easier review. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

And of course, this is part of https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md, building on a pile of other PRs (tektoncd#4705, tektoncd#4734, tektoncd#4753, tektoncd#4757). This adds a new function to `pkg/reconciler/pipelinerun/pipelinerun.go`, specifically for updating `pr.Status.ChildReferences` during reconciliation. It's analogous to the existing `updatePipelineRunStatusFromTaskRuns` and `updatePipelineRunStatusFromRuns` functions. This PR doesn't actually call the new function - behavior is exactly the same. But it adds the new function, along with other functions it depends on. In the final step of the implementation, these other functions will also be used in `...FromTaskRuns` and/or `...FromRuns`. I also reworked `pkg/reconciler/pipelinerun/pipelinerun_updatestatus_test.go` to improve its test fixtures, so that they're easier to reuse and instantiated via YAML parsing as much as possible. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

…pelineRuns See: * https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md * tektoncd#4705 * tektoncd#4734 * tektoncd#4753 * tektoncd#4757 * tektoncd#4760 * tektoncd#3140 This implements TEP-0100, allowing for choosing between the original full embedded `TaskRun` and `Run` statuses in `PipelineRun` statuses, minimal child references to the underlying `TaskRun` and `Run`s, or both, building on top of all the other PRs referenced above. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

tekton-robot added release-note-none Denotes a PR that doesnt merit a release note. kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). labels Apr 4, 2022

tekton-robot requested review from afrittoli and vdemeester April 4, 2022 15:17

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 4, 2022

tekton-robot assigned lbernick Apr 4, 2022

abayer force-pushed the esi-test-changes branch from 1801630 to 6b66506 Compare April 4, 2022 16:01

lbernick reviewed Apr 5, 2022

View reviewed changes

lbernick mentioned this pull request Apr 5, 2022

Clean up usages of config maps in pipelinerun_test #4736

Merged

4 tasks

lbernick reviewed Apr 5, 2022

View reviewed changes

vdemeester reviewed Apr 5, 2022

View reviewed changes

jerop added this to the Pipelines v0.35 milestone Apr 5, 2022

abayer commented Apr 5, 2022

View reviewed changes

abayer force-pushed the esi-test-changes branch from 6b66506 to 8a4b527 Compare April 5, 2022 17:46

lbernick reviewed Apr 5, 2022

View reviewed changes

abayer force-pushed the esi-test-changes branch from 8a4b527 to 7918f6d Compare April 5, 2022 18:59

abayer commented Apr 5, 2022

View reviewed changes

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 6, 2022

abayer force-pushed the esi-test-changes branch from 7918f6d to 7f043a0 Compare April 6, 2022 13:45

tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 6, 2022

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 6, 2022

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 6, 2022

tekton-robot merged commit e46259e into tektoncd:main Apr 6, 2022

abayer mentioned this pull request Apr 6, 2022

[TEP-0100] Implementation for embedded TaskRun and Run statuses in PipelineRuns #4739

Merged

5 tasks

abayer mentioned this pull request Apr 13, 2022

[TEP-0100] Add functionality to be used in supporting minimal embedded status #4757

Merged

5 tasks

abayer mentioned this pull request Apr 14, 2022

[TEP-0100] Add new updatePipelineRunStatusFromChildRefs function #4760

Merged

5 tasks

talife mentioned this pull request Dec 2, 2022

Migrate PipelineRun Reconciler Tests to use YAMLParser #4610

Closed

62 tasks

[TEP-0100] Prepare for testing of minimal status implementation #4734

[TEP-0100] Prepare for testing of minimal status implementation #4734

Conversation

abayer commented Apr 4, 2022

Changes

Submitter Checklist

Release Notes

abayer commented Apr 4, 2022

lbernick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abayer commented Apr 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vdemeester left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abayer commented Apr 5, 2022

lbernick commented Apr 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lbernick commented Apr 6, 2022

abayer commented Apr 6, 2022

lbernick commented Apr 6, 2022

vdemeester commented Apr 6, 2022

tekton-robot commented Apr 6, 2022