Skip to content
This repository has been archived by the owner on Mar 9, 2021. It is now read-only.

[kn-admin]Add e2e tests for kn admin plugin #63

Merged
merged 5 commits into from
Aug 4, 2020

Conversation

xliuxu
Copy link
Contributor

@xliuxu xliuxu commented Jun 30, 2020

E2E tests need to fill-up:

  • Autoscaling
  • autoscaling update --scale-to-zero
  • autoscaling update --no-scale-to-zero
  • Domain
  • domain set
  • domain set with selector
  • domain unset
  • Registry
  • registry add
  • registry remove

@knative-prow-robot knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 30, 2020
@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 30, 2020
@knative-prow-robot
Copy link

Hi @lanceliuu. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@knative-prow-robot knative-prow-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 30, 2020
@xliuxu xliuxu changed the title [WIP]Add e2e tests for kn admin plugin [WIP][kn-admin]Add e2e tests for kn admin plugin Jun 30, 2020
@zhanggbj
Copy link

/ok-to-test

@knative-prow-robot knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 30, 2020
@zhanggbj
Copy link

Hi @evankanderson, would you please help to add @lanceliuu to knative org, he's mainly focused on knative/client-contrib now, thanks!

Copy link
Contributor

@maximilien maximilien left a comment

Choose a reason for hiding this comment

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

Please address the copyright.

I realize this is a DRAFT so leaving early feedback.

Can you please provide a description and perhaps a TODO list of the tests you plan to do. That will help review and you executing. Thanks.

cc @zhanggbj

plugins/admin/test/e2e/kn_admin_test.go Outdated Show resolved Hide resolved
plugins/admin/test/e2e/kn_admin_test.go Outdated Show resolved Hide resolved
@maximilien
Copy link
Contributor

@lanceliuu you can add your tests plan as a TODO / tasks list in the description of this PR.

@xliuxu
Copy link
Contributor Author

xliuxu commented Jul 14, 2020

/retest

Signed-off-by: Lance Liu <[email protected]>
@xliuxu
Copy link
Contributor Author

xliuxu commented Jul 14, 2020

/retest

Signed-off-by: Lance Liu <[email protected]>
@xliuxu
Copy link
Contributor Author

xliuxu commented Jul 15, 2020

/retest

@knative-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lanceliuu, zhanggbj

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:

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 19, 2020
@zhanggbj
Copy link

per offline discussion with @lanceliuu, this PR is the initial e2e setup and only cover domain subcommand.
Other e2e are listed here #14 and will be covered by other subcommand stories.
I think this looks good and we can merge it first and request e2e test for future PRs. Thanks!

CC @maximilien who helped to review this PR^^^

@xliuxu xliuxu marked this pull request as ready for review July 21, 2020 15:34
@maximilien
Copy link
Contributor

OK @zhanggbj. Let's also edit the description of this PR and removing the unfinished TODOs and move them to the other list if not there.

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 30, 2020
@xliuxu xliuxu changed the title [WIP][kn-admin]Add e2e tests for kn admin plugin [kn-admin]Add e2e tests for kn admin plugin Jul 31, 2020
@knative-prow-robot knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 31, 2020
@zhanggbj
Copy link

Hi @maximilien @daisy-ycguo @rhuss

The integration tests failed due to service creation failure. The Knative Ingress never becomes ready, it could be an error of e2e cluster side, any suggestions?

installing 'kn' plugin 'kn-source-kafka' from path '/home/prow/go/src/knative.dev/client-contrib/plugins/source-kafka' to config path: /root/.config/kn
--- FAIL: TestSourceKafka (646.32s)
    result_collector.go:75: ERROR: Error: timeout: service 'sinksvc' not ready after 600 seconds
        Run 'kn --help' for usage
    result_collector.go:101: === FAIL: =======================[[ERROR]]========================
        🦆 kn service create sinksvc --image gcr.io/knative-samples/helloworld-go --namespace kne2etests0
        ┃ Creating service 'sinksvc' in namespace 'kne2etests0':
        ┃ 
        ┃   0.143s The Configuration is still working to reflect the latest desired specification.
        ┃   0.258s The Route is still working to reflect the latest desired specification.
skipped 663 lines unfold_more
2020/07/31 09:12:15 process.go:155: Step '/tmp/kubernetes.D5Hjz6Osu0/e2e-test.sh' finished in 21m27.166505541s
2020/07/31 09:12:15 main.go:316: Something went wrong: encountered 1 errors: [error during /tmp/kubernetes.D5Hjz6Osu0/e2e-test.sh: exit status 1]
Test subprocess exited with code 1
Artifacts were written to /logs/artifacts
Test result code is 1
Step failed: test/e2e-tests.sh 
Step failed: default_integration_test_runner
          status:
            address:
              url: http://sinksvc.kne2etests0.svc.cluster.local
            conditions:
            - lastTransitionTime: "2020-07-31T08:59:50Z"
              status: "True"
              type: AllTrafficAssigned
            - lastTransitionTime: "2020-07-31T08:59:50Z"
              message: autoTLS is not enabled
              reason: TLSNotEnabled
              status: "True"
              type: CertificateProvisioned
            - lastTransitionTime: "2020-07-31T08:59:50Z"
              message: Ingress has not yet been reconciled.
              reason: IngressNotConfigured
              status: Unknown
              type: IngressReady
            - lastTransitionTime: "2020-07-31T08:59:50Z"
              message: Ingress has not yet been reconciled.
              reason: IngressNotConfigured
              status: Unknown
              type: Ready

@maximilien
Copy link
Contributor

/test pull-knative-client-contrib-integration-tests

@lanceliuu can you dig into why above ^^^ is failing? thx.

@knative-prow-robot knative-prow-robot merged commit 226665a into knative:master Aug 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants