search

zalando-incubator / kube-ingress-aws-controller

Configures AWS Load Balancers according to Kubernetes Ingress resources
MIT License
375 stars 83 forks source link

Add some more goldenfile tests #614

Closed MustafaSaber closed 10 months ago

MustafaSaber commented 1 year ago

Follow up on https://github.com/zalando-incubator/kube-ingress-aws-controller/pull/587 for better coverage.

lucastt commented 1 year ago

So, when reading the expected CF templates I noticed something weird, they seem to reference values that would be received as parameters, but there is no place showing the values of these parameters. To confirm my theory I executed a vimdiff between both expected files: testdata/ingress_rg_shared_alb/expected.cf and testdata/ingress_rg_notshared_alb/expected.cf and they yield no diff, meaning they are exactly the same.

When I created the structure of these tests I didn't notice it because I just made two simple tests, on creating ALB and the other creating NLB, which yield different templates. But now I see there may be a problem in the way I built these golden file tests. 🤔

Did I get something wrong?

lucastt commented 1 year ago

So, when reading the expected CF templates I noticed something weird, they seem to reference values that would be received as parameters, but there is no place showing the values of these parameters. To confirm my theory I executed a vimdiff between both expected files: testdata/ingress_rg_shared_alb/expected.cf and testdata/ingress_rg_notshared_alb/expected.cf and they yield no diff, meaning they are exactly the same.

When I created the structure of these tests I didn't notice it because I just made two simple tests, on creating ALB and the other creating NLB, which yield different templates. But now I see there may be a problem in the way I built these golden file tests. 🤔

Did I get something wrong?

Created a new issue for this. https://github.com/zalando-incubator/kube-ingress-aws-controller/issues/626

lucastt commented 10 months ago

👍

szuecs commented 10 months ago

@MustafaSaber the changes are great progress for tests! If you do nonshared / share, please keep routegroup in ingress with annotations in-sync. You can change the .kube file (at least in skipper tests) to configure the defaults of the controller and then test defaults, too. From my pov, to merge the only thing to make sure is that the tests are consistent in their annotation use.

MustafaSaber commented 10 months ago

Thanks @lucastt for taking over 🙏!

lucastt commented 10 months ago

👍

lucastt commented 10 months ago

👍

szuecs commented 10 months ago

:+1: