Add ApiServer source update and describe commands #556

navidshaikh · 2019-12-15T16:49:06Z

Proposed Changes

Add kn source apiserver update and kn source apiserver describe
Dont return created ApiServer source object but only error
Align creating ApiServer source client, removes unit tests
Add ApiServer source update command
Rename TestMockKnClient to TestMockKnCronJobSourceClient
Add mock client for ApiServer Source and its tests
Add mock unit tests for create, delete and update
Add e2e tests for ApiServer source update
- Add a test for ApiServer source sink update
- Verify the updated sink name after the ApiServer source is updated
- Update resource names in existing tests
Use builder pattern to generate ApiServer source

/lint

- After ApiServer source object is created, we don't need it to pass around in caller function.

- Add a test for apiserver source sink update - Verify the updated sink name after the apiserver source is created - Update resource names in existing tests

knative-prow-robot

@navidshaikh: 23 warnings.

In response to this:

Proposed Changes

Dont return created ApiServer source object but only error,

Align creating ApiServer source client, removes unit tests

Add ApiServer source update command

Rename TestMockKnClient to TestMockKnCronJobSourceClient

Add mock client for ApiServer Source and its tests

Add mock unit tests for create, delete and update

Add e2e tests for apiserver source update

Add a test for apiserver source sink update

Verify the updated sink name after the apiserver source is created

Update resource names in existing tests

TODO:

better unit tests coverage

more e2e tests

/lint

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.

pkg/kn/commands/source/apiserver/apiserver.go

pkg/kn/commands/source/apiserver/apiserver_test.go

pkg/eventing/sources/v1alpha1/apiserver_client_mock.go

pkg/eventing/sources/v1alpha1/apiserver_client.go

docs/cmd/kn_source_apiserver.md

daisy-ycguo

lgtm. Thank you for the improvement to apiserver.

docs/cmd/kn_source_apiserver.md

navidshaikh · 2019-12-16T14:23:28Z

/lint

knative-prow-robot

@navidshaikh: 1 warning.

In response to this:

/lint

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.

test/e2e/source_apiserver_test.go

- Add command and unit tests - TODO for later: Add 'Controller Selector' section for --verbose

knative-metrics-robot · 2019-12-16T16:33:15Z

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

File	Old Coverage	New Coverage	Delta
pkg/eventing/sources/v1alpha1/apiserver_client.go	62.5%	100.0%	37.5
pkg/eventing/sources/v1alpha1/apiserver_client_mock.go	Do not exist	89.5%
pkg/kn/commands/source/apiserver/apiserver.go	100.0%	57.1%	-42.9
pkg/kn/commands/source/apiserver/create.go	81.2%	85.2%	3.9
pkg/kn/commands/source/apiserver/delete.go	76.5%	82.4%	5.9
pkg/kn/commands/source/apiserver/describe.go	Do not exist	84.3%
pkg/kn/commands/source/apiserver/flags.go	Do not exist	95.2%
pkg/kn/commands/source/apiserver/update.go	Do not exist	78.9%

rhuss

thanks, looks very good ! Love the appreciation of the details (like naming it APIServerSource)

Let's get that merged and ready for tomorrow.

/lgtm

knative-prow-robot · 2019-12-16T17:41:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: navidshaikh, rhuss

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~~ [navidshaikh,rhuss]

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

navidshaikh added 7 commits December 15, 2019 15:50

Dont return created ApiServer source object but only error

af61ff8

- After ApiServer source object is created, we don't need it to pass around in caller function.

Align creating ApiServer source client, removes unit tests

f6a15d1

Add ApiServer source update command

5718067

Rename TestMockKnClient to TestMockKnCronJobSourceClient

31881ef

Add mock client for ApiServer Source and its tests

99b3952

Add mock unit tests for create, delete and update

72fcd6a

Add e2e tests for apiserver source update

43408fc

- Add a test for apiserver source sink update - Verify the updated sink name after the apiserver source is created - Update resource names in existing tests

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Dec 15, 2019

knative-prow-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 15, 2019

knative-prow-robot reviewed Dec 15, 2019

View reviewed changes

knative-prow-robot requested review from mattmoor and maximilien December 15, 2019 16:49

navidshaikh commented Dec 15, 2019

View reviewed changes

docs/cmd/kn_source_apiserver.md Outdated Show resolved Hide resolved

navidshaikh added 4 commits December 16, 2019 15:33

Uses builder pattern for ApiServer source create command

72063c4

Update ApiServer source create/update flags and required config

23adcdd

Uses builder pattern for ApiServer source update command

f7af01a

Align create/update/delete description and error messages

6418749

navidshaikh force-pushed the pr/apiserver-update branch from 03a7ad2 to 6418749 Compare December 16, 2019 10:09

navidshaikh added 2 commits December 16, 2019 17:16

Add unit tests for get/create/update/delete in apiserver_client.go

101e627

Update e2e tests expected output per change in commands output

7a2e3d3

knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 16, 2019

daisy-ycguo reviewed Dec 16, 2019

View reviewed changes

Golint fixes, Api -> API and add exported method docs

da7ceef

navidshaikh commented Dec 16, 2019

View reviewed changes

docs/cmd/kn_source_apiserver.md Outdated Show resolved Hide resolved

knative-prow-robot reviewed Dec 16, 2019

View reviewed changes

test/e2e/source_apiserver_test.go Outdated Show resolved Hide resolved

navidshaikh added 2 commits December 16, 2019 19:56

Rename a test method and source update command description

1264b66

Add ApiServer source describe command

3b747a7

- Add command and unit tests - TODO for later: Add 'Controller Selector' section for --verbose

navidshaikh changed the title ~~Add ApiServer source update command~~ Add ApiServer source update and describe command Dec 16, 2019

navidshaikh changed the title ~~Add ApiServer source update and describe command~~ Add ApiServer source update and describe commands Dec 16, 2019

rhuss approved these changes Dec 16, 2019

View reviewed changes

knative-prow-robot assigned rhuss Dec 16, 2019

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2019

knative-prow-robot merged commit 6ac25cd into knative:master Dec 16, 2019

navidshaikh deleted the pr/apiserver-update branch December 16, 2019 19:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ApiServer source update and describe commands #556

Add ApiServer source update and describe commands #556

navidshaikh commented Dec 15, 2019 •

edited

Loading

knative-prow-robot left a comment

daisy-ycguo left a comment

navidshaikh commented Dec 16, 2019

knative-prow-robot left a comment

knative-metrics-robot commented Dec 16, 2019

rhuss left a comment

knative-prow-robot commented Dec 16, 2019

Add ApiServer source update and describe commands #556

Add ApiServer source update and describe commands #556

Conversation

navidshaikh commented Dec 15, 2019 • edited Loading

Proposed Changes

knative-prow-robot left a comment

Choose a reason for hiding this comment

Proposed Changes

daisy-ycguo left a comment

Choose a reason for hiding this comment

navidshaikh commented Dec 16, 2019

knative-prow-robot left a comment

Choose a reason for hiding this comment

knative-metrics-robot commented Dec 16, 2019

rhuss left a comment

Choose a reason for hiding this comment

knative-prow-robot commented Dec 16, 2019

navidshaikh commented Dec 15, 2019 •

edited

Loading