Tests for CreateService and annotations

[cdrage]

Adds some tests for CreateService, specifically the initial generation
as well as providing an edge case when specifying "kompose.service.type"

[cdrage]

Fixes 1/2 of #242

[cdrage]

Still newbish to Go, so let me know what things I can improve on to make this more idiomatic!

[kadel]

I'm don't know that much about writing tests :-(

[kadel]

Shouldn't be there at least some kind of test that service has been created?
Maybe I just don't understand it, but it looks like this is not testing anything :-)

[cdrage]

@kadel you're right, i forgot to add the checking err in this test, was initially a copy-and-paste from the test below. thank you!

[cdrage]

@kadel me neither for the tests, I updated it to check that information is generated. I tried doing a comparison between port numbers (passing 123 and making sure we get 123 back). But I'm having a bit of difficulty digging through k8s code to find the correct way of doing it. Hopefully this update to the PR is sufficient!

[kadel]

there already is a function for comparing ports in kubernetes_test.go

[kadel]

you commited vim swap file ;-) - pkg/transformer/kubernetes/.k8sutils_test.go.swp

[cdrage]

@kadel boo, that's why I added the .gitignore PR earlier xD okay, so i've updated this PR again with new code, although I'm not sure why the tests are erroring.

Updated the PR, passes now! Ready for another review.

[kadel]

One last thing ;-)
You test only NodePort, it would be great to test also other options for service.type.
This might be great case for something like this: https://github.com/golang/go/wiki/TableDrivenTests

[cdrage]

@kadel sure, i'll work on that. i'll include it in the top test however since this test is specific to supplying nodeport (testing the annotations / labels 👍 )

[cdrage]

@kadel so I don't think I can compare anything else (svc.Spec.Type is blank, same as svc.Spec.ClusterIP, svc.Spec.LoadBalancerIP, etc.)

Should I just compare that it comes up as blank?

[kadel]

@cdrage sorry my comment might be little bit confusing. I should do it as line comment.

What I meant was that it might be nice to make this test more generic. Using table driven tests you can do test that will test all possible values for kompose.service.type in one test. Basicaly create more services with different kompose.service.type label and test them all

[kadel]

We can do something like this to test all possible label values:

    // An example object generated via k8s runtime.Objects()
    kompose_object := kobject.KomposeObject{
        ServiceConfigs: map[string]kobject.ServiceConfig{"app": service},
    }
    k := Kubernetes{}

    tests := []struct {
        labelValue  string
        serviceType string
    }{
        {"NodePort", "NodePort"},
        {"nodeport", "NodePort"},
        {"LoadBalancer", "LoadBalancer"},
        {"loadbalancer", "LoadBalancer"},
        {"ClusterIP", "ClusterIP"},
        {"clusterip", "ClusterIP"},
    }

    for _, tt := range tests {
        kompose_object.ServiceConfigs["app"].Annotations["kompose.service.type"] = tt.labelValue

        objects := k.Transform(kompose_object, kobject.ConvertOptions{CreateD: true, Replicas: 3})

        // Test the creation of the service with modified annotations (kompose.service.type)
        svc := k.CreateService("foo", service, objects)
        if svc.Spec.Type != api.ServiceType(tt.serviceType) {
            t.Errorf("Expected NodePort, actual %d", svc.Spec.Type)
        }
    }

It would be great if we could also test invalid values, but it is not currently possible as CreateService logs error using logrus.Errorf instead of returning it :-(

[cdrage]

@kadel AHHHH that makes sense! Thought you wanted to test for different keys, rather than different values. I'll add that! Thanks for doing the work for it too haha

[cdrage]

@kadel updated with your code :) thanks man!

[kadel]

@kadel updated with your code :) thanks man!

thanks ;-)
Sorry for confusion, sometimes it is easier for me to express myself in code than in english 😆

[cdrage]

@kadel

01101110 01101111 00100000 01110000 01110010 01101111 01100010 01101100 01100101 01101101

:)

[kadel]

00111010 01000100