Restructure e2e Tests to Improve Organization #979

danielhelfand · 2020-05-11T14:11:07Z

This pull request moves e2e tests out of the e2e package and into separate packages based on the subcommand being tested.

Why is this needed?

Better organization as far as helping contributors know where to look for/contribute e2e tests with pull requests
Make sure tests for one subcommand don't depend on other tests (e.g. currently the task start command relies on helper functions in the pipeline e2e test).
Help to set up e2e testing standards to help with the process of building out our e2e test suite and make maintaining these tests easier in the future

My thought is that each subcommand of the cli should have its own package and that test files can follow the naming pattern subcommand_actioncommand_e2e_test.go (e.g. task_start_e2e_test.go). All e2e tests related to tkn task start can then live under that file.

I will be continuing to make future modifications to the e2e suite this release as this pull request doesn't fully address organization issues around our tests, but I think this is a good first step.

Submitter Checklist

Includes tests (if functionality changed/added)
Run the code checkers with make check
Regenerate the manpages, docs and go formatting with make generated
Commit messages follow commit message best practices

Release Notes

N/A

vdemeester

Why is this needed?
* Better organization as far as helping contributors know where to look for/contribute e2e tests with pull requests
* Make sure tests for one subcommand don't depend on other tests (e.g. currently the task start command relies on helper functions in the pipeline e2e test).
* Help to set up e2e testing standards to help with the process of building out our e2e test suite and make maintaining these tests easier in the future

I do agree 👏

My thought is that each subcommand of the cli should have its own package and that test files can follow the naming pattern subcommand_actioncommand_e2e_test.go (e.g. task_start_e2e_test.go). All e2e tests related to tkn task start can then live under that file.

So, I don't think we need to add e2 in the name, it's already part of the package path.

vdemeester · 2020-05-12T09:03:42Z

test/e2e/e2epipeline/pipeline_e2e_test.go

@@ -12,7 +13,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.

-package e2e
+package e2epipeline


Why not naming the package pipeline (under e2e) ?

Sure, my original thought was to avoid any confusion with other pipeline package, but I agree and changed it.

vdemeester · 2020-05-12T09:04:04Z

test/e2e/e2etask/task_start_e2e_test.go

@@ -12,27 +13,34 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.

-package e2e
+package e2etask


Same here, task

danielhelfand · 2020-05-12T12:41:30Z

So, I don't think we need to add e2 in the name, it's already part of the package path.

level=warning msg="[runner] Can't run linter goanalysis_metalinter: assign: failed prerequisites: [[email protected]/tektoncd/cli/test/e2e/task [github.com/tektoncd/cli/test/e2e/task.test]: analysis skipped: errors in package: [/home/prow/go/src/github.com/tektoncd/cli/test/e2e/task/task_start_e2e_test.go:39:18: undeclared name: Setup]]"
level=error msg="Running error: assign: failed prerequisites: [[email protected]/tektoncd/cli/test/e2e/task [github.com/tektoncd/cli/test/e2e/task.test]: analysis skipped: errors in package: [/home/prow/go/src/github.com/tektoncd/cli/test/e2e/task/task_start_e2e_test.go:39:18: undeclared name: Setup]]"
make: *** [Makefile:72: lint-go] Error 3

Appears to fail linting for build tests without e2e. I also noticed would not allow it unless e2e was specified locally, but wasn't sure if I had some IDE setting causing this. If there is a way to do this, more than happy to, but not sure if there is some nuance I am missing.

vdemeester · 2020-05-12T13:00:09Z

So, I don't think we need to add e2 in the name, it's already part of the package path.
level=warning msg="[runner] Can't run linter goanalysis_metalinter: assign: failed prerequisites: [[email protected]/tektoncd/cli/test/e2e/task [github.com/tektoncd/cli/test/e2e/task.test]: analysis skipped: errors in package: [/home/prow/go/src/github.com/tektoncd/cli/test/e2e/task/task_start_e2e_test.go:39:18: undeclared name: Setup]]"
level=error msg="Running error: assign: failed prerequisites: [[email protected]/tektoncd/cli/test/e2e/task [github.com/tektoncd/cli/test/e2e/task.test]: analysis skipped: errors in package: [/home/prow/go/src/github.com/tektoncd/cli/test/e2e/task/task_start_e2e_test.go:39:18: undeclared name: Setup]]"
make: *** [Makefile:72: lint-go] Error 3
Appears to fail linting for build tests without e2e. I also noticed would not allow it unless e2e was specified locally, but wasn't sure if I had some IDE setting causing this. If there is a way to do this, more than happy to, but not sure if there is some nuance I am missing.

golangci-lint is acting weird sometimes.. There is some issue with memory & co too that are being worked-on upstream. I think this might be some falkes ?

danielhelfand · 2020-05-12T14:18:31Z

Unfortunately, I keep running into build errors with build tests/integration tests when not using e2e explicitly:

level=warning msg="[runner] Can't run linter goanalysis_metalinter: assign: failed prerequisites: [[email protected]/tektoncd/cli/test/e2e/pipeline [github.com/tektoncd/cli/test/e2e/pipeline.test]: analysis skipped: errors in package: [/home/prow/go/src/github.com/tektoncd/cli/test/e2e/pipeline/pipeline_e2e_test.go:47:18: undeclared name: Setup /home/prow/go/src/github.com/tektoncd/cli/test/e2e/pipeline/pipeline_e2e_test.go:48:42: undeclared name: TearDown /home/prow/go/src/github.com/tektoncd/cli/test/e2e/pipeline/pipeline_e2e_test.go:49:8: undeclared name: TearDown /home/prow/go/src/github.com/tektoncd/cli/test/e2e/pipeline/pipeline_e2e_test.go:51:14: undeclared name: NewTknRunner /home/prow/go/src/github.com/tektoncd/cli/test/e2e/pipeline/pipeline_e2e_test.go:80:15: undeclared name: ListAllTasksOutput /home/prow/go/src/github.com/tektoncd/cli/test/e2e/pipeline/pipeline_e2e_test.go:81:8: undeclared name: TaskData /home/prow/go/src/github.com/tektoncd/cli/test/e2e/pipeline/pipeline_e2e_test.go:84:8: undeclared name: TaskData /home/prow/go/src/github.com/tektoncd/cli/test/e2e/pipeline/pipeline_e2e_test.go:99:15: undeclared name: ListAllPipelinesOutput

test/e2e/pipeline/pipeline_e2e_test.go:47:18: undefined: Setup
test/e2e/pipeline/pipeline_e2e_test.go:48:42: undefined: TearDown
test/e2e/pipeline/pipeline_e2e_test.go:49:8: undefined: TearDown
test/e2e/pipeline/pipeline_e2e_test.go:51:14: undefined: NewTknRunner
test/e2e/pipeline/pipeline_e2e_test.go:80:15: undefined: ListAllTasksOutput
test/e2e/pipeline/pipeline_e2e_test.go:81:8: undefined: TaskData
test/e2e/pipeline/pipeline_e2e_test.go:84:8: undefined: TaskData
test/e2e/pipeline/pipeline_e2e_test.go:99:15: undefined: ListAllPipelinesOutput
test/e2e/pipeline/pipeline_e2e_test.go:100:8: undefined: PipelinesListData

Adding them back for now.

danielhelfand · 2020-05-17T15:16:52Z

So, I don't think we need to add e2 in the name, it's already part of the package path.

@vdemeester Addressed

vdemeester

/lgtm

pradeepitm12

/lgtm

tekton-robot · 2020-05-19T06:40:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pradeepitm12

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

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

tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 11, 2020

tekton-robot requested review from chmouel and vdemeester May 11, 2020 14:11

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 11, 2020

danielhelfand changed the title ~~WIP: Move e2e Tests into Subcommand Packages to Improve Organization~~ WIP: Restructure e2e Tests to Improve Organization May 11, 2020

danielhelfand changed the title ~~WIP: Restructure e2e Tests to Improve Organization~~ Restructure e2e Tests to Improve Organization May 11, 2020

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 11, 2020

vdemeester reviewed May 12, 2020

View reviewed changes

tekton-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 12, 2020

move e2e tests into subcommand packages

69b20c2

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 17, 2020

vdemeester reviewed May 18, 2020

View reviewed changes

tekton-robot assigned vdemeester May 18, 2020

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 18, 2020

pradeepitm12 reviewed May 19, 2020

View reviewed changes

tekton-robot assigned pradeepitm12 May 19, 2020

pradeepitm12 approved these changes May 19, 2020

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 19, 2020

tekton-robot merged commit 8e29999 into tektoncd:master May 19, 2020

praveen4g0 mentioned this pull request Jun 9, 2020

Add e2e tags to cli tests #806

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restructure e2e Tests to Improve Organization #979

Restructure e2e Tests to Improve Organization #979

danielhelfand commented May 11, 2020 •

edited

Loading

vdemeester left a comment

vdemeester May 12, 2020

danielhelfand May 12, 2020

vdemeester May 12, 2020

danielhelfand commented May 12, 2020

vdemeester commented May 12, 2020

danielhelfand commented May 12, 2020

danielhelfand commented May 17, 2020

vdemeester left a comment

pradeepitm12 left a comment

tekton-robot commented May 19, 2020

Restructure e2e Tests to Improve Organization #979

Restructure e2e Tests to Improve Organization #979

Conversation

danielhelfand commented May 11, 2020 • edited Loading

Submitter Checklist

Release Notes

vdemeester left a comment

Choose a reason for hiding this comment

vdemeester May 12, 2020

Choose a reason for hiding this comment

danielhelfand May 12, 2020

Choose a reason for hiding this comment

vdemeester May 12, 2020

Choose a reason for hiding this comment

danielhelfand commented May 12, 2020

vdemeester commented May 12, 2020

danielhelfand commented May 12, 2020

danielhelfand commented May 17, 2020

vdemeester left a comment

Choose a reason for hiding this comment

pradeepitm12 left a comment

Choose a reason for hiding this comment

tekton-robot commented May 19, 2020

danielhelfand commented May 11, 2020 •

edited

Loading