From 9e73be7a490a93d6c7f07b4f9503d61297fa0afc Mon Sep 17 00:00:00 2001
From: Jakub Al-Khalili <jakubalkhalili@gmail.com>
Date: Wed, 10 Jul 2019 15:19:26 +0200
Subject: [PATCH] #43 Improve tests and fix GetJenkinsOpts

---
 .../jenkins/configuration/base/reconcile.go   |  7 +-
 .../configuration/base/reconcile_test.go      | 91 ++++++++++++++-----
 pkg/controller/jenkins/jenkins_controller.go  |  6 +-
 3 files changed, 75 insertions(+), 29 deletions(-)

diff --git a/pkg/controller/jenkins/configuration/base/reconcile.go b/pkg/controller/jenkins/configuration/base/reconcile.go
index 024b85f3..1709e37d 100644
--- a/pkg/controller/jenkins/configuration/base/reconcile.go
+++ b/pkg/controller/jenkins/configuration/base/reconcile.go
@@ -136,12 +136,15 @@ func GetJenkinsOpts(jenkins *v1alpha2.Jenkins) map[string]string {
 		if v.Name == "JENKINS_OPTS" {
 			jenkinsOptsEnv := envs[k]
 			jenkinsOptsWithDashes := jenkinsOptsEnv.Value
-			jenkinsOptsWithDashes = strings.ReplaceAll(jenkinsOptsWithDashes, "--", "") // Remove dashes
+			if len(jenkinsOptsWithDashes) == 0 {
+				return nil
+			}
+			
 			jenkinsOptsWithEqOperators := strings.Split(jenkinsOptsWithDashes, " ")
 
 			for _, vx := range jenkinsOptsWithEqOperators {
 				opt := strings.Split(vx, "=")
-				jenkinsOpts[opt[0]] = opt[1]
+				jenkinsOpts[strings.ReplaceAll(opt[0], "--", "")] = opt[1]
 			}
 
 			return jenkinsOpts
diff --git a/pkg/controller/jenkins/configuration/base/reconcile_test.go b/pkg/controller/jenkins/configuration/base/reconcile_test.go
index 478f5b06..d8599f81 100644
--- a/pkg/controller/jenkins/configuration/base/reconcile_test.go
+++ b/pkg/controller/jenkins/configuration/base/reconcile_test.go
@@ -15,40 +15,81 @@ import (
 )
 
 func TestGetJenkinsOpts(t *testing.T) {
-	envs := []corev1.EnvVar{
-		{Name: "JENKINS_OPTS", Value: "--prefix=/jenkins --httpPort=8080"},
-	}
-
-	jenkins := &v1alpha2.Jenkins{
-		Spec: v1alpha2.JenkinsSpec{
-			Master: v1alpha2.JenkinsMaster{
-				Containers: []v1alpha2.Container{
-					{
-						Env: envs,
+	t.Run("JENKINS_OPTS is empty", func(t *testing.T) {
+		jenkins := &v1alpha2.Jenkins{
+			Spec: v1alpha2.JenkinsSpec{
+				Master: v1alpha2.JenkinsMaster{
+					Containers: []v1alpha2.Container{
+						{
+							Env: []corev1.EnvVar{
+								{Name: "JENKINS_OPTS", Value: ""},
+							},
+						},
 					},
 				},
 			},
-		},
-	}
+		}
 
-	opts := GetJenkinsOpts(jenkins)
-
-	t.Run("equal env vars", func(t *testing.T) {
-		assert.Equal(t, opts["prefix"], "/jenkins")
-		assert.Equal(t, opts["httpPort"], "8080")
+		opts := GetJenkinsOpts(jenkins)
+		assert.Equal(t, 0, len(opts))
 	})
 
-	t.Run("not equal env vars", func(t *testing.T) {
-		assert.NotEqual(t, opts["prefix"], "/jenkins_not_equal")
-		assert.NotEqual(t, opts["httpPort"], "80808")
+	t.Run("JENKINS_OPTS have --prefix argument ", func(t *testing.T) {
+		jenkins := &v1alpha2.Jenkins{
+			Spec: v1alpha2.JenkinsSpec{
+				Master: v1alpha2.JenkinsMaster{
+					Containers: []v1alpha2.Container{
+						{
+							Env: []corev1.EnvVar{
+								{Name: "JENKINS_OPTS", Value: "--prefix=/jenkins"},
+							},
+						},
+					},
+				},
+			},
+		}
+
+		opts := GetJenkinsOpts(jenkins)
+
+		assert.Equal(t, 1, len(opts))
+
+		t.Run("ensure that JENKINS_OPTS not contains --httpPort", func(t *testing.T) {
+			assert.NotContains(t, opts, "httpPort")
+		})
+
+		t.Run("ensure that argument is --prefix", func(t *testing.T) {
+			assert.Contains(t, opts, "prefix")
+		})
 	})
 
-	t.Run("should exists", func(t *testing.T) {
-		assert.Contains(t, opts, "httpPort")
-	})
+	t.Run("JENKINS_OPTS have --prefix and --httpPort argument", func(t *testing.T) {
+		jenkins := &v1alpha2.Jenkins{
+			Spec: v1alpha2.JenkinsSpec{
+				Master: v1alpha2.JenkinsMaster{
+					Containers: []v1alpha2.Container{
+						{
+							Env: []corev1.EnvVar{
+								{Name: "JENKINS_OPTS", Value: "--prefix=/jenkins --httpPort=8080"},
+							},
+						},
+					},
+				},
+			},
+		}
 
-	t.Run("should exists", func(t *testing.T) {
-		assert.NotContains(t, opts, "should_not_exists")
+		opts := GetJenkinsOpts(jenkins)
+
+		assert.Equal(t, 2, len(opts))
+
+		t.Run("ensure that argument is --prefix with value /jenkins", func(t *testing.T) {
+			assert.Contains(t, opts, "prefix")
+			assert.Equal(t, opts["prefix"], "/jenkins")
+		})
+
+		t.Run("ensure that argument is --httpPort with value 8080", func(t *testing.T) {
+			assert.Contains(t, opts, "httpPort")
+			assert.Equal(t, opts["httpPort"], "8080")
+		})
 	})
 }
 
diff --git a/pkg/controller/jenkins/jenkins_controller.go b/pkg/controller/jenkins/jenkins_controller.go
index 83e6ec92..ea56a719 100644
--- a/pkg/controller/jenkins/jenkins_controller.go
+++ b/pkg/controller/jenkins/jenkins_controller.go
@@ -3,7 +3,8 @@ package jenkins
 import (
 	"context"
 	"fmt"
-	"github.com/go-logr/logr"
+	"reflect"
+
 	"github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2"
 	jenkinsclient "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/client"
 	"github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/configuration/base"
@@ -15,6 +16,8 @@ import (
 	"github.com/jenkinsci/kubernetes-operator/pkg/event"
 	"github.com/jenkinsci/kubernetes-operator/pkg/log"
 	"github.com/jenkinsci/kubernetes-operator/version"
+
+	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -24,7 +27,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
-	"reflect"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/handler"