Skip to content

DRYed test cases in TestDNS #1377

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

Closed
wants to merge 0 commits into from
Closed

Conversation

sallyom
Copy link
Contributor

@sallyom sallyom commented Mar 19, 2015

@nhr, @mfojtik
I condensed the 2 cases to test for len(in.Answer) !<1...is there a specific reason why len(in.Answer) for "foo.kubernetes.default.local." was tested for exactly 1? Thanks.

@mfojtik
Copy link
Contributor

mfojtik commented Mar 19, 2015

@sallyom this will need rebase but otherwise LGTM, good job!

dnsQuestionName: "foo.kubernetes.default.local.",
},
{
dnsQuestionName: "www.google.com.",
Copy link
Contributor

Choose a reason for hiding this comment

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

These aren't the same test. www.google.com is recursive (testing that we forward resolution), foo.kubernetes.default.local is not.

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 see that now, thanks.

@sallyom
Copy link
Contributor Author

sallyom commented Mar 19, 2015

@smarterclayton please re-review, thanks.

@sallyom sallyom force-pushed the extend_tests branch 2 times, most recently from 94958ca to 9df9bf3 Compare March 20, 2015 14:26
@sallyom
Copy link
Contributor Author

sallyom commented Mar 20, 2015

[test]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1440/)

@sallyom
Copy link
Contributor Author

sallyom commented Mar 20, 2015

[test]

@smarterclayton
Copy link
Contributor

LGTM [merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge FAILURE (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1351/)

@sallyom sallyom force-pushed the extend_tests branch 2 times, most recently from b0f506d to a97d54c Compare March 24, 2015 15:06
@smarterclayton
Copy link
Contributor

[merge]

1 similar comment
@smarterclayton
Copy link
Contributor

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin up to a97d54c

@openshift-bot
Copy link
Contributor

Origin Action Required: Pull request cannot be automatically merged, please rebase your branch from latest HEAD and push again

@smarterclayton
Copy link
Contributor

Merged as part of my change - thanks

@sallyom sallyom deleted the extend_tests branch June 26, 2015 09:44
jboyd01 pushed a commit to jboyd01/origin that referenced this pull request Oct 17, 2017
…service-catalog/' changes from 3aacfedec6..aa27078754

aa27078754 origin build: add origin tooling
bcf37fd 0.1.0-rc2 chart updates (openshift#1410)
4ab0a0a add back 'Processing' message for instance deletion (openshift#1332)
0ecbcb1 Update logs for Cluster service plans. (openshift#1389)
8b491ef Fix a quoting nit (openshift#1400)
63685e4 add orphan mitigation-specific conditions for instances (openshift#1378)
adee662 Updated missed fields in service and plan specs (openshift#1406)
2095919 Handle default plan setting when using k8s names (openshift#1405)
607ba66 Document rbacEnable. (openshift#1404)
268294e Adding rbac definition for v1 api endpoint. (openshift#1284)
103288d differentiate between failed updates and provisions during deletion (openshift#1383)
eba8ba4 enable API aggregation and Service Catalog RBAC on Jenkins (openshift#1333)
5a93315 Validate relistDuration is non-negative (openshift#1395)
e279d21 Fix log messages for secrets (openshift#1385)
87fa8c9 fix status update when starting orphan mitigation (openshift#1372)
11f18f3 Switch to wget for integration apiserver checks (openshift#1384)
8c44a7d update OSB client to 2.13 (openshift#1392)
e64bbd1 default plan admission controller: filter list of service plans/service classes by the class name (openshift#1351)
6648c0e Check field names. Fix issue 1291 (openshift#1379)
5319841 update comment for instance generation check (openshift#1382)
7d5823f remove internal poll method (openshift#1381)
07d3068 Rework the logging for controller_instance. (openshift#1371)
5f4ca01 address PR comment as a followup (openshift#1380)
485d5e6 Add support for specifying plan using K8S names. (openshift#1377)
662bba8 Log number of secret keys created for binding credential (openshift#1375)
8ad6a31 Move controller constants into correct files (openshift#1373)
7bd66dd Adding type to log. (openshift#1339)
1ce5c4d Remove k8s/k8s dependency (openshift#1355)
b458323 Adding log formatting for BindingController. (openshift#1352)
275eb11 rename test variables to be consistent (openshift#1315)
ffd6b8b travis: skip cleanup before deploy (openshift#1368)
d5ecc04 fix travis tag checker (openshift#1365)
2cae0ee Minor updates to README (openshift#1360)
REVERT: 3aacfedec6 carry: Set external plan name for service-catalog walkthrough
REVERT: 3ec9e5b07a origin build: add origin tooling

git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog
git-subtree-split: aa2707875461dd51be3731b1d94b5cfc3b9a3976
jpeeler pushed a commit to jpeeler/origin that referenced this pull request Feb 1, 2018
* Add support for specifying plan using K8S names.

* some wording clarifications and forgot to check in the admission controller file change

* fix e2e tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants