Add test for store helper ListIngresses #2399
Conversation
k8s-ci-robot
added
approved
k8s-ci-robot
added
the
cncf-cla: yes
Codecov Report
@@ Coverage Diff @@
## master #2399 +/- ##
==========================================
+ Coverage 39.45% 39.58% +0.13%
==========================================
Files 73 73
Lines 5216 5216
==========================================
+ Hits 2058 2065 +7
+ Misses 2870 2862 -8
- Partials 288 289 +1
Continue to review full report at Codecov.
|
aledbf
force-pushed
the
add-tests
branch
2 times, most recently
from
539f470
to
996acf0
Compare
| @@ -740,3 +740,76 @@ func TestUpdateSecretIngressMap(t *testing.T) { | |||
| } | |||
| }) | |||
| } | |||
|
|
|||
| // Test to avoid regression fixed in https://github.com/kubernetes/ingress-nginx/pull/2361 | |||
| func TestListIngressWithoutPath(t *testing.T) { | |||
There was a problem hiding this comment.
From what I understand you're actually testing the function completely rather than one aspect of it, therefore I think this should just be called TestListIngresses.
IMHO the comment does not add any value.
| } | ||
| s.listers.Ingress.Add(ing1) | ||
|
|
||
| ing2 := &v1beta1.Ingress{ |
There was a problem hiding this comment.
Could you call this ingressToIgnore or ingressWithDifferentClass that way the test case will also serve as a documentation and it will make it easier to figure out that this is the one that we are expecting to be ignored by ListIngresses function.
| }, | ||
| } | ||
| s.listers.Ingress.Add(ing3) | ||
|
|
There was a problem hiding this comment.
For completeness I'd also add another ingress similar to ing2 but this time with the default supported class of nginx.
k8s-ci-robot
added
size/L
| } | ||
| s.listers.Ingress.Add(ingressWithoutPath) | ||
|
|
||
| ingressWithNginxClass := &v1beta1.Ingress{ |
There was a problem hiding this comment.
I think the names got mixed between this and previous ingress. Since the previous ingress is the one with "kubernetes.io/ingress.class": "nginx",
| s.listers.Ingress.Add(ingressWithNginxClass) | ||
|
|
||
| ingresses := s.ListIngresses() | ||
| if s := len(ingresses); s != 2 { |
|
@ElvinEfendi tests updated |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, ElvinEfendi 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 |
No description provided.