Skip to content

Improve some k8s executor tests

Tomasz Maczukin requested to merge improve-k8s-tests into master

What does this MR do?

Replaces "manual" checks in executors/kubernetes/util_test.go with usage of github.com/stretchr/testify/assert and github.com/stretchr/testify/require

Why was this MR needed?

  1. It makes the test consistent with rest of our unit tests, where we're using require and assert packages to define assertions.

  2. It improves the visibility of differences between expected and received object. It makes easier to find out what's wrong with failures like https://gitlab.com/redbaron1/gitlab-runner/-/jobs/85359364, where the current output is:

    --- FAIL: TestGetKubeClientConfig/Complete_cert_based_auth_take_precendece_over_in_cluster_config (0.00s)
    	util_test.go:157: expected: '&{another_host  {  <nil> <nil>}    { [] map[]} <nil> <nil> <nil> {false  crt key ca [] [] []}  <nil> <nil> 0 0 <nil> 0s <nil>}', got: '&{another_host  {  <nil> <nil>}    { [] map[]} <nil> <nil> <nil> {false  crt key ca [] [] []} gitlab-runner development version (HEAD; go1.10; linux/amd64) <nil> <nil> 0 0 <nil> 0s <nil>}'

    and the output after merging this MR would be:

    --- FAIL: TestGetKubeClientConfig/Complete_cert_based_auth_take_precendece_over_in_cluster_config (0.00s)
    Error Trace:	util_test.go:160
    Error:      	Not equal: 
                	expected: &rest.Config{Host:"another_host", APIPath:"", ContentConfig:rest.ContentConfig{AcceptContentTypes:"", ContentType:"", GroupVersion:(*schema.GroupVersion)(nil), NegotiatedSerializer:runtime.NegotiatedSerializer(nil)}, Username:"", Password:"", BearerToken:"", Impersonate:rest.ImpersonationConfig{UserName:"", Groups:[]string(nil), Extra:map[string][]string(nil)}, AuthProvider:(*api.AuthProviderConfig)(nil), AuthConfigPersister:rest.AuthProviderConfigPersister(nil), ExecProvider:(*api.ExecConfig)(nil), TLSClientConfig:rest.TLSClientConfig{Insecure:false, ServerName:"", CertFile:"crt", KeyFile:"key", CAFile:"ca", CertData:[]uint8(nil), KeyData:[]uint8(nil), CAData:[]uint8(nil)}, UserAgent:"", Transport:http.RoundTripper(nil), WrapTransport:(func(http.RoundTripper) http.RoundTripper)(nil), QPS:0, Burst:0, RateLimiter:flowcontrol.RateLimiter(nil), Timeout:0, Dial:(func(string, string) (net.Conn, error))(nil)}
                	actual: &rest.Config{Host:"another_host", APIPath:"", ContentConfig:rest.ContentConfig{AcceptContentTypes:"", ContentType:"", GroupVersion:(*schema.GroupVersion)(nil), NegotiatedSerializer:runtime.NegotiatedSerializer(nil)}, Username:"", Password:"", BearerToken:"", Impersonate:rest.ImpersonationConfig{UserName:"", Groups:[]string(nil), Extra:map[string][]string(nil)}, AuthProvider:(*api.AuthProviderConfig)(nil), AuthConfigPersister:rest.AuthProviderConfigPersister(nil), ExecProvider:(*api.ExecConfig)(nil), TLSClientConfig:rest.TLSClientConfig{Insecure:false, ServerName:"", CertFile:"crt", KeyFile:"key", CAFile:"ca", CertData:[]uint8(nil), KeyData:[]uint8(nil), CAData:[]uint8(nil)}, UserAgent:"gitlab-runner development version (HEAD; go1.10; linux/amd64)", Transport:http.RoundTripper(nil), WrapTransport:(func(http.RoundTripper) http.RoundTripper)(nil), QPS:0, Burst:0, RateLimiter:flowcontrol.RateLimiter(nil), Timeout:0, Dial:(func(string, string) (net.Conn, error))(nil)}
                	
                	Diff:
                	--- Expected
                	+++ Actual
                	@@ -30,3 +30,3 @@
                	  },
                	- UserAgent: (string) "",
                	+ UserAgent: (string) (len=61) "gitlab-runner development version (HEAD; go1.10; linux/amd64)",
                	  Transport: (http.RoundTripper) <nil>,
    

Are there points in the code the reviewer needs to double check?

Does this MR meet the acceptance criteria?

  • Documentation created/updated
  • Tests
    • Added for this feature/bug
    • All builds are passing
  • Branch has no merge conflicts with master (if you do - rebase it please)

What are the relevant issue numbers?

Merge request reports