Add Unit Tests for TestMissingResultWhenStepErrorIsIgnored and Update e2e test: TestFailingStepOnContinue #6771

EmmaMunley · 2023-06-05T15:52:38Z

Changes

This PR adds test coverage for a pipelinerun that has 2 tasks where the first task produces 2 results that are consumed in the 2nd task. However, the first task contains a failing step so only 1 of the 2 results are produced which results in a failing pipeline run.

Prior to this PR, this was an E2E integration test, but missing unit test coverage. This increases the test coverage and also modifies the e2e test to only test the behavior of the TaskRun, since the PipelineRun behavior is now being tested in unit tests to reduce redundancies, improve speed, and reduce flakes.

/kind bug

Submitter Checklist

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

Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

tekton-robot · 2023-06-05T15:52:43Z

@EmmaMunley: The label(s) kind/ cannot be applied, because the repository doesn't have them.

In response to this:

Changes

This PR adds test coverage for a PipelineRun that has 2 tasks where the first task produces 2 results that are consumed in the 2nd task. However, the first task contains a failing step so only 1 of the 2 results are produced which results in a failing pipeline run. This is currently an Integration Test, but found missing code coverage in the unit tests.

Submitter Checklist

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

Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps

Has Tests included if any functionality added or changed

Follows the commit message standard

Meets the Tekton contributor standards (including functionality, content, code)

Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep

Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.

Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes
NONE
/kind bug

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.

tekton-robot · 2023-06-05T15:57:33Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.4%	90.9%	0.5

tekton-robot · 2023-06-05T16:02:30Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.4%	90.9%	0.5

EmmaMunley · 2023-06-05T18:16:50Z

Opened issue here for flakey test: #6772

tekton-robot · 2023-06-05T18:16:51Z

@EmmaMunley: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test pull-tekton-pipeline-alpha-integration-tests
/test pull-tekton-pipeline-beta-integration-tests
/test pull-tekton-pipeline-build-tests
/test pull-tekton-pipeline-integration-tests
/test tekton-pipeline-unit-tests

The following commands are available to trigger optional jobs:

/test pull-tekton-pipeline-go-coverage

Use /test all to run all jobs.

In response to this:

/retest Opened issue here for flakey test: #6772

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.

EmmaMunley · 2023-06-05T18:17:44Z

/retest

lbernick

thanks Emma!

I was thinking about whether it would make sense for this unit test to replace the corresponding integration test. I think it probably can't, since the integration test also verifies the behavior of the taskrun on error. However (not necessarily in scope for this PR) we could trim down the integration test to test only the taskrun behavior, since IMO the pipelineRun behavior is adequately tested by this unit test. That would make the test suite just a tiny bit faster and less flaky. thoughts?

pkg/reconciler/pipelinerun/pipelinerun_test.go

tekton-robot · 2023-06-05T21:56:35Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.4%	90.9%	0.5

tekton-robot · 2023-06-05T22:00:15Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.4%	90.9%	0.5

QuanZhang-William · 2023-06-06T13:54:16Z

pkg/reconciler/pipelinerun/pipelinerun_test.go

+    kind: TaskRun
+    name: test-pipeline-missing-results-task1
+    pipelineTaskName: task1
+  provenance:


The provenance field looks like irrelevant of this test and maybe we could consider ignoring the comparison for it. But it is totally optional 😃

@QuanZhang-William I'm not sure if we have an ignoreProvenance field. It looks like the provenance field was introduced in this PR: #6495 and added to all of the expectedPR statuses. Maybe for consistency we leave as is and then can scope to a separate PR if we want to add an ignoreProvenance field?

Yongxuanzhang

/lgtm

tekton-robot · 2023-06-08T16:01:41Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.5%	91.0%	0.5

tekton-robot · 2023-06-08T16:02:33Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.5%	91.0%	0.5

tekton-robot · 2023-06-08T16:07:13Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.5%	91.0%	0.5

tekton-robot · 2023-06-08T17:29:58Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.6%	91.1%	0.5

EmmaMunley · 2023-06-08T18:36:41Z

/test pull-tekton-pipeline-go-coverage-df

tekton-robot · 2023-06-08T18:36:43Z

@EmmaMunley: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-tekton-pipeline-alpha-integration-tests
/test pull-tekton-pipeline-beta-integration-tests
/test pull-tekton-pipeline-build-tests
/test pull-tekton-pipeline-integration-tests
/test tekton-pipeline-unit-tests

The following commands are available to trigger optional jobs:

/test pull-tekton-pipeline-go-coverage

Use /test all to run all jobs.

In response to this:

/test pull-tekton-pipeline-go-coverage-df

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.

EmmaMunley · 2023-06-08T18:37:20Z

/test pull-tekton-pipeline-go-coverage

tekton-robot · 2023-06-08T18:41:12Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.6%	91.1%	0.5

tekton-robot · 2023-06-08T18:44:25Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.6%	91.1%	0.5

tekton-robot · 2023-06-08T18:45:03Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.6%	91.1%	0.5

test/ignore_step_error_test.go

tekton-robot · 2023-06-09T21:53:20Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.7%	91.2%	0.5

tekton-robot · 2023-06-09T21:54:22Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.7%	91.2%	0.5

test/ignore_step_error_test.go

tekton-robot · 2023-06-12T13:59:14Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.7%	91.2%	0.5

tekton-robot · 2023-06-12T14:02:39Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.7%	91.2%	0.5

tekton-robot · 2023-06-12T14:19:00Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.7%	91.2%	0.5

tekton-robot · 2023-06-12T14:21:38Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.7%	91.2%	0.5

EmmaMunley · 2023-06-12T19:54:38Z

/retest

Chaingaurd registry was down https://status.cgr.dev/

… e2e test: TestFailingStepOnContinue This commit adds test coverage for a pipelinerun that has 2 tasks where the first task produces 2 results that are consumed in the 2nd task. However, the first task contains a failing step so only 1 of the 2 results are produced which results in a failing pipeline run. Prior to this commit, this was an E2E integration test, but missing unit test coverage. This increases the test coverage and also modifies the e2e test to only test the behavior of the TaskRun, since the PipelineRun behavior is now being tested in unit tests to reduce redundancies, improve speed, and reduce flakes.

tekton-robot · 2023-06-13T00:36:18Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.7%	91.2%	0.5

tekton-robot · 2023-06-13T00:40:24Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.7%	91.2%	0.5

tekton-robot · 2023-06-13T13:28:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lbernick

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~~ [lbernick]

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

Yongxuanzhang · 2023-06-13T15:20:58Z

/lgtm

tekton-robot added kind/bug Categorizes issue or PR as related to a bug. release-note-none Denotes a PR that doesnt merit a release note. labels Jun 5, 2023

tekton-robot requested review from abayer and afrittoli June 5, 2023 15:52

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 5, 2023

jerop self-assigned this Jun 5, 2023

EmmaMunley mentioned this pull request Jun 5, 2023

Flake: authenticating-git-commands #6772

Closed

lbernick reviewed Jun 5, 2023

View reviewed changes

EmmaMunley force-pushed the TestMissingResultWhenStepErrorIsIgnored branch from da5127e to b85e4ef Compare June 5, 2023 21:51

QuanZhang-William reviewed Jun 6, 2023

View reviewed changes

Yongxuanzhang reviewed Jun 6, 2023

View reviewed changes

tekton-robot assigned Yongxuanzhang Jun 6, 2023

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

EmmaMunley requested a review from lbernick June 7, 2023 14:41

EmmaMunley force-pushed the TestMissingResultWhenStepErrorIsIgnored branch from b85e4ef to 59557e4 Compare June 8, 2023 15:55

tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 8, 2023

EmmaMunley force-pushed the TestMissingResultWhenStepErrorIsIgnored branch from 59557e4 to 130a958 Compare June 8, 2023 15:57

EmmaMunley changed the title ~~Add Unit Test for TestMissingResultWhenStepErrorIsIgnored~~ Add Unit Tests for TestMissingResultWhenStepErrorIsIgnored and Update e2e test: TestFailingStepOnContinue Jun 8, 2023

EmmaMunley force-pushed the TestMissingResultWhenStepErrorIsIgnored branch from 130a958 to ed7c2bc Compare June 8, 2023 17:22

lbernick reviewed Jun 8, 2023

View reviewed changes

test/ignore_step_error_test.go Outdated Show resolved Hide resolved

EmmaMunley force-pushed the TestMissingResultWhenStepErrorIsIgnored branch from ed7c2bc to c2af18a Compare June 9, 2023 21:47

lbernick reviewed Jun 11, 2023

View reviewed changes

test/ignore_step_error_test.go Outdated Show resolved Hide resolved

test/ignore_step_error_test.go Show resolved Hide resolved

EmmaMunley force-pushed the TestMissingResultWhenStepErrorIsIgnored branch from c2af18a to b6e4d4c Compare June 12, 2023 13:55

EmmaMunley force-pushed the TestMissingResultWhenStepErrorIsIgnored branch from b6e4d4c to c46b69f Compare June 12, 2023 14:14

EmmaMunley force-pushed the TestMissingResultWhenStepErrorIsIgnored branch from c46b69f to 68ffd1a Compare June 13, 2023 00:32

lbernick approved these changes Jun 13, 2023

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 13, 2023

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 13, 2023

tekton-robot merged commit 1bf65c5 into tektoncd:main Jun 13, 2023

EmmaMunley deleted the TestMissingResultWhenStepErrorIsIgnored branch June 13, 2023 16:54

Add Unit Tests for TestMissingResultWhenStepErrorIsIgnored and Update e2e test: TestFailingStepOnContinue #6771

Add Unit Tests for TestMissingResultWhenStepErrorIsIgnored and Update e2e test: TestFailingStepOnContinue #6771

Conversation

EmmaMunley commented Jun 5, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Jun 5, 2023

Changes

Submitter Checklist

Release Notes

tekton-robot commented Jun 5, 2023

tekton-robot commented Jun 5, 2023

EmmaMunley commented Jun 5, 2023 • edited Loading

tekton-robot commented Jun 5, 2023

EmmaMunley commented Jun 5, 2023

lbernick left a comment

Choose a reason for hiding this comment

tekton-robot commented Jun 5, 2023

tekton-robot commented Jun 5, 2023

QuanZhang-William Jun 6, 2023

Choose a reason for hiding this comment

EmmaMunley Jun 6, 2023

Choose a reason for hiding this comment

Yongxuanzhang left a comment

Choose a reason for hiding this comment

tekton-robot commented Jun 8, 2023

tekton-robot commented Jun 8, 2023

tekton-robot commented Jun 8, 2023

tekton-robot commented Jun 8, 2023

EmmaMunley commented Jun 8, 2023

tekton-robot commented Jun 8, 2023

EmmaMunley commented Jun 8, 2023

tekton-robot commented Jun 8, 2023

tekton-robot commented Jun 8, 2023

tekton-robot commented Jun 8, 2023

tekton-robot commented Jun 9, 2023

tekton-robot commented Jun 9, 2023

tekton-robot commented Jun 12, 2023

tekton-robot commented Jun 12, 2023

tekton-robot commented Jun 12, 2023

tekton-robot commented Jun 12, 2023

EmmaMunley commented Jun 12, 2023

tekton-robot commented Jun 13, 2023

tekton-robot commented Jun 13, 2023

tekton-robot commented Jun 13, 2023

Yongxuanzhang commented Jun 13, 2023

EmmaMunley commented Jun 5, 2023 •

edited

Loading

EmmaMunley commented Jun 5, 2023 •

edited

Loading