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.

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

Moved to ginkgo based tests in scaffolding 2.0 #679

Merged

Conversation

Projects
None yet
4 participants
@droot
Copy link
Contributor

commented Apr 26, 2019

With scaffolding version 2.0,

  • scaffolded tests for API types use ginkgo.
  • Re-enabled scaffolding for simple CRD manifest until we have crd-generation working for scaffolding 2.0
  • Added one more kind in test project to enable testing for multiple kinds.
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: droot

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

@k8s-ci-robot k8s-ci-robot requested review from pmorie and pwittrock Apr 26, 2019

@droot droot changed the title Feature/test scaffolding 2.0 Moved to ginkgo based tests in scaffolding 2.0 Apr 30, 2019

@@ -37,6 +37,7 @@ var (

func init() {

crewv1.AddToScheme(scheme)
crewv1.AddToScheme(scheme)

This comment has been minimized.

Copy link
@droot

droot Apr 30, 2019

Author Contributor

This is a bug where crewv1.AddToScheme is getting duplicated. Will fix this in a separate PR.

This comment has been minimized.

Copy link
@mengqiy

mengqiy May 4, 2019

Contributor

We can have a TODO here.

@droot

This comment has been minimized.

Copy link
Contributor Author

commented Apr 30, 2019

@DirectXMan12 Can you pl. take a look at this one.

@droot droot assigned mengqiy and unassigned DirectXMan12 May 2, 2019

@droot droot force-pushed the droot:feature/test-scaffolding-2.0 branch from f13dc01 to da8092c May 2, 2019

@mengqiy
Copy link
Contributor

left a comment

Other LG

var _ = BeforeSuite(func() {
By("bootstrapping test environment")
testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crds")},

This comment has been minimized.

Copy link
@mengqiy

mengqiy May 4, 2019

Contributor

We need to append bases when #644 is merged.

This comment has been minimized.

Copy link
@droot

droot May 8, 2019

Author Contributor

Ack.

@@ -37,6 +37,7 @@ var (

func init() {

crewv1.AddToScheme(scheme)
crewv1.AddToScheme(scheme)

This comment has been minimized.

Copy link
@mengqiy

mengqiy May 4, 2019

Contributor

We can have a TODO here.

@@ -52,6 +52,7 @@ scaffold_test_project() {
$kb create api --group policy --version v1beta1 --kind HealthCheckPolicy --example=false --controller=true --resource=true --namespaced=false --make=false
elif [ $version == "2" ]; then
$kb create api --group crew --version v1 --kind FirstMate --controller=true --resource=true --make=false
$kb create api --group crew --version v1 --kind SecondMate --controller=true --resource=true --make=false

This comment has been minimized.

This comment has been minimized.

Copy link
@mengqiy

mengqiy May 6, 2019

Contributor

We can drop this line now.
You will see crew Captain after rebase.

This comment has been minimized.

Copy link
@droot

droot May 8, 2019

Author Contributor

Removed the SecondMate kind and rebased.

//
// These tests are written in BDD-style using Ginkgo framework. Refer to
// http://onsi.github.io/ginkgo to learn more.
//

This comment has been minimized.

Copy link
@mengqiy

mengqiy May 4, 2019

Contributor

nit: these 2 extra // make it looks funny. I'd suggest using either

// blah
// blah

or

/////////////
// blah    //
// blah    //
/////////////

This comment has been minimized.

Copy link
@droot

droot May 8, 2019

Author Contributor

killed the lines with empty comment.

// your API definition.
// Avoid adding tests for vanilla CRUD operations because they would
// test Kubernetes API server, which isn't the goal here.
//

This comment has been minimized.

Copy link
@mengqiy

mengqiy May 4, 2019

Contributor

Same here

This comment has been minimized.

Copy link
@droot

droot May 8, 2019

Author Contributor

Fixed all of these.

//
// These tests are written in BDD-style using Ginkgo framework. Refer to
// http://onsi.github.io/ginkgo to learn more.
//

This comment has been minimized.

Copy link
@mengqiy

mengqiy May 4, 2019

Contributor

Same here

// your API definition.
// Avoid adding tests for vanilla CRUD operations because they would
// test Kubernetes API server, which isn't the goal here.
//

This comment has been minimized.

Copy link
@mengqiy

mengqiy May 4, 2019

Contributor

Same here

//
// These tests use Ginkgo (BDD-style Go testing framework). Refer to
// http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
//

This comment has been minimized.

Copy link
@mengqiy

mengqiy May 4, 2019

Contributor

Same here

@droot
Copy link
Contributor Author

left a comment

Thanks for the review @mengqiy . Addressed the review comments. I did a rebase and found out that we got rid of scaffolded CRD generation in one of the merged PR, so now tests fail :). I will wait for the controller-gen PR to get merged.

@@ -52,6 +52,7 @@ scaffold_test_project() {
$kb create api --group policy --version v1beta1 --kind HealthCheckPolicy --example=false --controller=true --resource=true --namespaced=false --make=false
elif [ $version == "2" ]; then
$kb create api --group crew --version v1 --kind FirstMate --controller=true --resource=true --make=false
$kb create api --group crew --version v1 --kind SecondMate --controller=true --resource=true --make=false

This comment has been minimized.

Copy link
@droot

droot May 8, 2019

Author Contributor

Removed the SecondMate kind and rebased.

//
// These tests are written in BDD-style using Ginkgo framework. Refer to
// http://onsi.github.io/ginkgo to learn more.
//

This comment has been minimized.

Copy link
@droot

droot May 8, 2019

Author Contributor

killed the lines with empty comment.

// your API definition.
// Avoid adding tests for vanilla CRUD operations because they would
// test Kubernetes API server, which isn't the goal here.
//

This comment has been minimized.

Copy link
@droot

droot May 8, 2019

Author Contributor

Fixed all of these.

var _ = BeforeSuite(func() {
By("bootstrapping test environment")
testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crds")},

This comment has been minimized.

Copy link
@droot

droot May 8, 2019

Author Contributor

Ack.

@droot droot force-pushed the droot:feature/test-scaffolding-2.0 branch from da8092c to 910f9c8 May 8, 2019

@k8s-ci-robot k8s-ci-robot added size/L and removed size/XL labels May 8, 2019

@droot droot force-pushed the droot:feature/test-scaffolding-2.0 branch from 910f9c8 to 3892305 May 16, 2019

@k8s-ci-robot k8s-ci-robot added size/XXL and removed size/L labels May 16, 2019

Merge pull request #704 from droot/running/switch-to-go-modules
🏃 switch to using Go modules in build tooling

@droot droot force-pushed the droot:feature/test-scaffolding-2.0 branch from 3892305 to ed46300 May 16, 2019

@mengqiy

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label May 16, 2019

@k8s-ci-robot k8s-ci-robot merged commit a43f866 into kubernetes-sigs:master May 16, 2019

3 of 4 checks passed

tide Not mergeable. Needs lgtm label.
Details
cla/linuxfoundation k8s-ci-robot authorized
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
netlify/kubebuilder/deploy-preview Deploy preview ready!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.