From c75a35099eb5b759a90f78a50cbb014fdc0a4a04 Mon Sep 17 00:00:00 2001
From: gkGaneshR <gkganesh126@gmail.com>
Date: Mon, 5 Mar 2018 13:49:10 +0530
Subject: [PATCH] Avoided changing hostname and changing var names

1. Why is this change necessary ?
 The program avoids changing hostname and the variable name "Options" is changed to
"options". Also, added more comments and formatted. Removed hostname in options since
it will not be changed in tests.

2. How does this change address the issue ?
 While the program is being run, the hostname is not changed. And options can't be
accessed outside(not exported).

3. How to verify this change ?
 Run, make test

Signed-off-by: gkGaneshR <gkganesh126@gmail.com>
---
 cmd/options/options_test.go | 100 +++++++++---------------------------
 1 file changed, 25 insertions(+), 75 deletions(-)

diff --git a/cmd/options/options_test.go b/cmd/options/options_test.go
index 2ee69e0f..c7df5513 100644
--- a/cmd/options/options_test.go
+++ b/cmd/options/options_test.go
@@ -17,73 +17,43 @@ limitations under the License.
 package options
 
 import (
-	"fmt"
 	"os"
-	"os/exec"
 	"testing"
 )
 
-type Options struct {
+type options struct {
 	Nodename         string
-	Hostname         string
 	HostnameOverride string
 }
 
 // TestSetNodeNameOrDie tests for permutations of nodename, hostname and hostnameoverride
 func TestSetNodeNameOrDie(t *testing.T) {
-	options := map[string]struct {
-		Expected         Options
+	option := map[string]struct {
+		Expected         options
 		ObtainedNodeName string
 	}{
 		"Check Node and HostnameOverride only": {
-			Expected: Options{
+			Expected: options{
 				Nodename:         "my-node-name",
-				Hostname:         "",
 				HostnameOverride: "override",
 			},
 		},
 		"Check Nodename only": {
-			Expected: Options{
+			Expected: options{
 				Nodename:         "my-node-name",
-				Hostname:         "",
 				HostnameOverride: "",
 			},
 		},
 
 		"Check HostnameOverride only": {
-			Expected: Options{
+			Expected: options{
 				Nodename:         "",
-				Hostname:         "",
 				HostnameOverride: "override",
 			},
 		},
-		"Check Hostname only": {
-			Expected: Options{
+		"Check empty": {
+			Expected: options{
 				Nodename:         "",
-				Hostname:         "my-host-name",
-				HostnameOverride: "",
-			},
-		},
-		"Check Node, host and HostnameOverride only": {
-			Expected: Options{
-				Nodename:         "my-node-name",
-				Hostname:         "my-host-name",
-				HostnameOverride: "override",
-			},
-		},
-
-		"Check Host and HostnameOverride only": {
-			Expected: Options{
-				Nodename:         "",
-				Hostname:         "my-host-name",
-				HostnameOverride: "override",
-			},
-		},
-
-		"Check Node and hostname": {
-			Expected: Options{
-				Nodename:         "my-node-name",
-				Hostname:         "my-host-name",
 				HostnameOverride: "",
 			},
 		},
@@ -92,59 +62,39 @@ func TestSetNodeNameOrDie(t *testing.T) {
 	orig_node_name := os.Getenv("NODE_NAME")
 	orig_host_name, err := os.Hostname()
 	if err != nil {
-		fmt.Println("Unable to get hostname")
+		t.Errorf("Unable to get hostname")
 	}
 
-	for str, opt := range options {
-		if opt.Expected.Nodename != "" {
-			err = os.Setenv("NODE_NAME", opt.Expected.Nodename)
-			if err != nil {
-				t.Errorf("Unable to set env NODE_NAME")
-			}
-		}
+	for str, opt := range option {
 
-		if opt.Expected.Hostname != "" {
-			// Changing hostname
-			cmd := exec.Command("hostname", opt.Expected.Hostname)
-			_, err := cmd.CombinedOutput()
-			if err != nil {
-				// If changing hostname requires admin privilege
-				cmd = exec.Command("sudo", "hostname", opt.Expected.Hostname)
-				_, err := cmd.CombinedOutput()
-				if err != nil {
-					fmt.Println("Unable to change hostname")
-					return
-				}
-			}
+		// Setting with expected(desired) NODE_NAME env
+		err = os.Setenv("NODE_NAME", opt.Expected.Nodename)
+		if err != nil {
+			t.Errorf("Unable to set env NODE_NAME")
 		}
 
 		npdObj := NewNodeProblemDetectorOptions()
+
+		// Setting with expected(desired) HostnameOverride
 		npdObj.HostnameOverride = opt.Expected.HostnameOverride
+
 		npdObj.SetNodeNameOrDie()
 		opt.ObtainedNodeName = npdObj.NodeName
 
+		// Setting back the original node name
+		err = os.Setenv("NODE_NAME", orig_node_name)
+		if err != nil {
+			t.Errorf("Unable to set original : env NODE_NAME")
+		}
+
+		// Checking for obtained node name
 		if opt.ObtainedNodeName != opt.Expected.HostnameOverride &&
 			opt.ObtainedNodeName != opt.Expected.Nodename &&
-			opt.ObtainedNodeName != opt.Expected.Hostname {
+			opt.ObtainedNodeName != orig_host_name {
 			t.Errorf("Error at : %+v", str)
 			t.Errorf("Wanted: %+v. \nGot: %+v", opt.Expected.Nodename, opt.ObtainedNodeName)
 		}
 
-		err = os.Setenv("NODE_NAME", "")
-		if err != nil {
-			t.Errorf("Unable to set env NODE_NAME empty")
-		}
-
-	}
-	// Setting back the original attributes
-	err = os.Setenv("NODE_NAME", orig_node_name)
-	if err != nil {
-		fmt.Println("Unable to set original : env NODE_NAME")
-	}
-	cmd := exec.Command("sudo", "hostname", orig_host_name)
-	_, err = cmd.CombinedOutput()
-	if err != nil {
-		fmt.Println("Unable to set hostname")
 	}
 
 }