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

Establish end-to-end testing procedure #38

Merged
merged 2 commits into from
Oct 18, 2018

Conversation

ironcladlou
Copy link
Contributor

Introduce a consistent method for testing changes against a 4.0 cluster.

  1. Provide an uninstaller.
  2. Provide a way to publish and install a locally built operator.
  3. Document the procedure for executing upstream e2e tests against the cluster.
  4. Remove the existing integration tests, which are already covered
    comprehensively by the e2e tests and which were misleading as to whether the
    installed operator setup was functional.

Introduce a consistent method for testing changes against a 4.0 cluster.

1. Provide an uninstaller.
2. Provide a way to publish and install a locally built operator.
3. Document the procedure for executing upstream e2e tests against the cluster.
4. Remove the existing integration tests, which are already covered
comprehensively by the e2e tests and which were misleading as to whether the
installed operator setup was functional.
@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 17, 2018
@ironcladlou
Copy link
Contributor Author

Awaiting merge of #37 since this depends on it.

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 17, 2018
@ironcladlou
Copy link
Contributor Author

/retest

To run all of them, try:

```
$ FOCUS='DNS' SKIP='\[Disabled:.+\]|\[Disruptive\]|\[Skipped\]|\[Slow\]|\[Flaky\]|\[local\]|\[Local\]' TEST_ONLY=1 test/extended/conformance.sh

Choose a reason for hiding this comment

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

test/extended/conformance.sh file from which repo? cluster-dns-operator repo doesn't contain this file path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From OpenShift... updated docs.

To run the bare minimum smoke test, try:

```
$ FOCUS='should provide DNS for the cluster' SKIP='\[Disabled:.+\]|\[Disruptive\]|\[Skipped\]|\[Slow\]|\[Flaky\]|\[local\]|\[Local\]' TEST_ONLY=1 test/extended/conformance.sh

Choose a reason for hiding this comment

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

Same as above, test/extended/conformance.sh from which repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From OpenShift... updated docs.

# Required for the operator-sdk.
export KUBERNETES_CONFIG="${KUBECONFIG}"

go test -v -tags integration ./test/integration

Choose a reason for hiding this comment

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

What are we testing? integration_test.go is empty
Are we not removing integration tests in favor of comprehensive origin/upstream e2e dns tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can imagine adding some other more focused operator-centric tests here later, didn't want to tear down all the machinery yet if it's benign.

@@ -1,28 +1,9 @@
package manifests

Choose a reason for hiding this comment

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

Do we still need to keep this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can imagine adding some other more focused operator-centric tests here later, didn't want to tear down all the machinery yet if it's benign.

@ironcladlou
Copy link
Contributor Author

#37 merged.

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 17, 2018
Copy link

@pravisankar pravisankar left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ironcladlou, pravisankar

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

@ironcladlou
Copy link
Contributor Author

/retest

@ironcladlou
Copy link
Contributor Author

/retest

1 similar comment
@ironcladlou
Copy link
Contributor Author

/retest

@openshift-merge-robot openshift-merge-robot merged commit ae59f18 into openshift:master Oct 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/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.

4 participants