Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Add namespaced e2e tests #2598

Merged
merged 1 commit into from
Apr 10, 2019

Conversation

jberkhahn
Copy link
Contributor

@jberkhahn jberkhahn commented Apr 5, 2019

  • add e2e tests that use namespace resources
  • change build-e2e make target to compile on host os/arch
  • refactor test/util helper methods to work on both cluster/ns resources

This PR is a

  • Feature Implementation
  • Bug Fix
  • Documentation

Fixes #2408

Merge Checklist:

  • New feature
    • Tests
    • Documentation
  • SVCat CLI flag
  • Server Flag for config
    • Chart changes
    • removing a flag by marking deprecated and hiding to avoid
      breaking the chart release and existing clients who provide a
      flag that will get an error when they try to update

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 5, 2019
@k8s-ci-robot k8s-ci-robot requested review from jboyd01 and MHBauer April 5, 2019 00:52
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 5, 2019
@jberkhahn jberkhahn force-pushed the e2e_namespace branch 3 times, most recently from 5210c5f to 72fd45c Compare April 5, 2019 20:45
@@ -175,7 +175,7 @@ $(BINDIR)/%-gen: $$(shell find vendor/k8s.io/code-generator/cmd/$$*-gen vendor/k

.PHONY: $(BINDIR)/e2e.test
$(BINDIR)/e2e.test: .init
$(DOCKER_CMD) go test -c -o $@ $(SC_PKG)/test/e2e
$(DOCKER_CMD) env GOOS=$(CLIENT_PLATFORM) GOARCH=$(ARCH) go test -c -o $@ $(SC_PKG)/test/e2e
Copy link
Contributor Author

@jberkhahn jberkhahn Apr 8, 2019

Choose a reason for hiding this comment

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

@MHBauer @jboyd01 could you run this make task on your machine to make sure it works somewhere other than my machine?

Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

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

builds and runs for me on both windows and linux.
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 8, 2019
Copy link
Contributor

@jboyd01 jboyd01 left a comment

Choose a reason for hiding this comment

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

Nice use of variadic for cluster vs namespaced. Just one nit from me on a typo. Glad to see the e2e enhanced to cover ns broker. Updated make target works fine for me on fedora.

genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
utilfeature "k8s.io/apiserver/pkg/util/feature"
)

// WaitForBrokerCondition waits for the status of the named broker to contain
// a condition whose type and status matches the supplied one.
func WaitForBrokerCondition(client v1beta1servicecatalog.ServicecatalogV1beta1Interface, name string, condition v1beta1.ServiceBrokerCondition) error {
// a condition whose type and status matches the supplied onei. Checks for a
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: one vs onei

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

- change build-e2e make target to compile on host os/arch
- refactor test/util helper methods to work on both cluster/ns resources
@k8s-ci-robot k8s-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2019
@jboyd01
Copy link
Contributor

jboyd01 commented Apr 9, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jboyd01

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 9, 2019
@jberkhahn
Copy link
Contributor Author

/retest

@MHBauer
Copy link
Contributor

MHBauer commented Apr 10, 2019

onei
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 10, 2019
@k8s-ci-robot k8s-ci-robot merged commit 72f79d3 into kubernetes-retired:master Apr 10, 2019
viviyww pushed a commit to viviyww/service-catalog that referenced this pull request May 10, 2019
- change build-e2e make target to compile on host os/arch
- refactor test/util helper methods to work on both cluster/ns resources
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance E2E testing to include ns scoped resources
4 participants