Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add tkn interactive tests #976

Merged
merged 1 commit into from
Jun 29, 2020
Merged

Add tkn interactive tests #976

merged 1 commit into from
Jun 29, 2020

Conversation

praveen4g0
Copy link

@praveen4g0 praveen4g0 commented May 11, 2020 •
edited
Loading

Add interactive tests for Resource create, taskrun, pipeline run logs ,pipeline start interactively..

Changes

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • 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

See the contribution guide
for more details.

Release Notes

release-note

@tekton-robot tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 11, 2020
@praveen4g0 praveen4g0 changed the title Add tkn interactive tests [WIP] Add tkn interactive tests May 11, 2020
@tekton-robot 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
@pradeepitm12 pradeepitm12 mentioned this pull request May 19, 2020
Merged
4 tasks
@praveen4g0 praveen4g0 force-pushed the cli-e2e branch 2 times, most recently from f05f8d2 to 2ac8919 Compare May 26, 2020 09:20
@tekton-robot tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 27, 2020
@praveen4g0 praveen4g0 force-pushed the cli-e2e branch 2 times, most recently from 3d40759 to 986f9f3 Compare May 27, 2020 15:16
@praveen4g0 praveen4g0 changed the title [WIP] Add tkn interactive tests Add tkn interactive tests Jun 4, 2020
@tekton-robot 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 Jun 4, 2020
danielhelfand
danielhelfand suggested changes Jun 11, 2020
View reviewed changes
test/e2e/cli.go Outdated

err = cmd.Start()
if err != nil {
t.Fatal(err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nice to make this configurable to allow for errors in the event we want to test for errors with interactive tests. Perhaps a param for the func that allows for error checking or fails the test based on an error?

test/e2e/cli.go Outdated Show resolved Hide resolved
test/e2e/cli.go
@@ -15,10 +15,16 @@
package e2e
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From an organization perspective, I am wondering if it would make sense to keep interactive test helper functions in a separate file? Maybe keep cli.go as where to define things that will be needed by all testing helper commands? WDYT?

test/e2e/pipeline/start_test.go Outdated Show resolved Hide resolved
test/e2e/pipeline/start_test.go

})

t.Run("Validate interactive pipeline logs, with follow mode (-f) ", func(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this include actually selecting and starting the logs of the PipelineRun?

test/e2e/resource/create_test.go Outdated Show resolved Hide resolved
test/e2e/resource/create_test.go Outdated Show resolved Hide resolved
test/e2e/resource/create_test.go Outdated Show resolved Hide resolved
test/e2e/resource/create_test.go Outdated Show resolved Hide resolved
test/e2e/task/task_start_test.go Show resolved Hide resolved
@praveen4g0 praveen4g0 force-pushed the cli-e2e branch 3 times, most recently from 49eb9ba to 567d1b5 Compare June 26, 2020 05:04
@vdemeester vdemeester added this to the 0.11.0 milestone Jun 26, 2020
@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 26, 2020
@danielhelfand danielhelfand mentioned this pull request Jun 26, 2020
Closed
4 tasks
vdemeester
vdemeester approved these changes Jun 29, 2020
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 29, 2020
@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danielhelfand, 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 [danielhelfand,vdemeester]

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

@tekton-robot tekton-robot merged commit 70b1c6e into tektoncd:master Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vdemeester vdemeester vdemeester approved these changes

@danielhelfand danielhelfand danielhelfand approved these changes

@pradeepitm12 pradeepitm12 Awaiting requested review from pradeepitm12

Assignees

@vdemeester vdemeester

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
0.11.0
Development

Successfully merging this pull request may close these issues.
4 participants
@praveen4g0 @tekton-robot @vdemeester @danielhelfand