Merge pull request #62 from andywaltlova/fix/use-table-driven-approac…

…h-in-tests

Rewrite all tests with table driven approach

src/logger_test.go

@@ -9,50 +9,75 @@ import (
 )
 
 func TestSetupLogger(t *testing.T) {
-	// Create a temporary directory for the log folder
-	logFolderName := "log-test"
-	logFileName := "log-file"
-	defer os.RemoveAll(logFolderName)
-
-	// Test case 1: YGG_LOG_LEVEL doesn't exist, info level should be set to info
-	setupLogger(logFolderName, logFileName)
-	level := log.CurrentLevel()
-	if log.CurrentLevel() != log.LevelInfo {
-		t.Errorf("Incorrect log level. Expected: %v, Got: %v", log.LevelInfo, level)
-	}
-	// Test case 2: Unparsable level in env variable
-	os.Setenv("YGG_LOG_LEVEL", "....")
-	setupLogger(logFolderName, logFileName)
-	level = log.CurrentLevel()
-	if log.CurrentLevel() != log.LevelInfo {
-		t.Errorf("Incorrect log level. Expected: %v, Got: %v", log.LevelInfo, level)
+	testCases := []struct {
+		name          string
+		logLevelEnv   string
+		expectedLevel log.Level
+		expectedFlags int
+	}{
+		{
+			name:          "YGG_LOG_LEVEL doesn't exist, info level should be set to info",
+			logLevelEnv:   "",
+			expectedLevel: log.LevelInfo,
+			expectedFlags: log.Lshortfile | log.LstdFlags,
+		},
+		{
+			name:          "Unparsable level in env variable",
+			logLevelEnv:   "....",
+			expectedLevel: log.LevelInfo,
+			expectedFlags: log.Lshortfile | log.LstdFlags,
+		},
+		{
+			name:          "Everything set up correctly",
+			logLevelEnv:   "debug",
+			expectedLevel: log.LevelDebug,
+			expectedFlags: log.Lshortfile | log.LstdFlags,
+		},
 	}
 
-	// Test case 3: Everything set up correctly
-	os.Setenv("YGG_LOG_LEVEL", "debug")
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Create a temporary directory for the log folder
+			logFolderName := "log-test"
+			logFileName := "log-file"
+			defer os.RemoveAll(logFolderName)
 
-	logfile := setupLogger(logFolderName, logFileName)
-	if _, err := os.Stat(logFolderName); os.IsNotExist(err) {
-		t.Errorf("Log folder not created: %v", err)
-	}
-	logFilePath := filepath.Join(logFolderName, logFileName)
-	if _, err := os.Stat(logFilePath); os.IsNotExist(err) {
-		t.Errorf("Log file not created: %v", err)
-	}
-	level = log.CurrentLevel()
-	if level != log.LevelDebug {
-		t.Errorf("Incorrect log level. Expected: %v, Got: %v", log.LevelDebug, level)
-	}
-	flags := log.Flags()
-	expectedFlags := log.Lshortfile | log.LstdFlags
-	if flags != expectedFlags {
-		t.Errorf("Incorrect log flags. Expected: %v, Got: %v", expectedFlags, flags)
-	}
+			// Set YGG_LOG_LEVEL environment variable
+			if tc.logLevelEnv != "" {
+				os.Setenv("YGG_LOG_LEVEL", tc.logLevelEnv)
+			}
+
+			logfile := setupLogger(logFolderName, logFileName)
+
+			// Verify log level
+			level := log.CurrentLevel()
+			if level != tc.expectedLevel {
+				t.Errorf("Incorrect log level. Expected: %v, Got: %v", tc.expectedLevel, level)
+			}
+
+			// Verify log file and folder
+			if _, err := os.Stat(logFolderName); os.IsNotExist(err) {
+				t.Errorf("Log folder not created: %v", err)
+			}
+			logFilePath := filepath.Join(logFolderName, logFileName)
+			if _, err := os.Stat(logFilePath); os.IsNotExist(err) {
+				t.Errorf("Log file not created: %v", err)
+			}
+
+			// Verify log flags
+			flags := log.Flags()
+			if flags != tc.expectedFlags {
+				t.Errorf("Incorrect log flags. Expected: %v, Got: %v", tc.expectedFlags, flags)
+			}
 
-	// Cleanup - close the log file
-	defer os.Unsetenv("YGG_LOG_LEVEL")
-	err := logfile.Close()
-	if err != nil {
-		t.Errorf("Failed to close log file: %v", err)
+			// Cleanup - close the log file and unset the YGG_LOG_LEVEL environment variable
+			defer func() {
+				os.Unsetenv("YGG_LOG_LEVEL")
+				err := logfile.Close()
+				if err != nil {
+					t.Errorf("Failed to close log file: %v", err)
+				}
+			}()
+		})
 	}
 }

src/runner.go

@@ -20,6 +20,13 @@ type signedYamlFile struct {
 	} `yaml:"vars"`
 }
 
+var verificationCommand = "insights-client"
+
+var verificationArgs = []string{
+	"-m", "insights.client.apps.ansible.playbook_verifier",
+	"--quiet", "--payload", "noop", "--content-type", "noop",
+}
+
 // Verify that no one tampered with yaml file
 func verifyYamlFile(yamlData []byte) bool {
 
@@ -31,18 +38,14 @@ func verifyYamlFile(yamlData []byte) bool {
 	// --payload here will be a no-op because no upload is performed when using the verifier
 	//   but, it will allow us to update the egg!
 
-	args := []string{
-		"-m", "insights.client.apps.ansible.playbook_verifier",
-		"--quiet", "--payload", "noop", "--content-type", "noop",
-	}
 	env := os.Environ()
 
 	if !*config.InsightsCoreGPGCheck {
-		args = append(args, "--no-gpg")
+		verificationArgs = append(verificationArgs, "--no-gpg")
 		env = append(env, "BYPASS_GPG=True")
 	}
 
-	cmd := exec.Command("insights-client", args...)
+	cmd := exec.Command(verificationCommand, verificationArgs...)
 	cmd.Env = env
 	stdin, err := cmd.StdinPipe()
 	if err != nil {

src/runner_test.go

@@ -7,85 +7,125 @@ import (
 )
 
 func TestProcessSignedScript(t *testing.T) {
-	shouldVerifyYaml := false
-	shouldDoInsightsCoreGPGCheck := false
-	temporaryWorkerDirectory := "test-dir"
-	config = &Config{
-		VerifyYAML:               &shouldVerifyYaml,
-		TemporaryWorkerDirectory: &temporaryWorkerDirectory,
-		InsightsCoreGPGCheck:     &shouldDoInsightsCoreGPGCheck,
-	}
-
-	defer os.RemoveAll(temporaryWorkerDirectory)
-
-	// Test case 1: verification disabled, no yaml data supplied = empty output
-	yamlData := []byte{}
-	expectedResult := ""
-	result := processSignedScript(yamlData)
-	if result != expectedResult {
-		t.Errorf("Expected %q, but got %q", expectedResult, result)
-	}
-
-	// Test case 2: verification disabled, yaml data supplied = non-empty output
-	yamlData = []byte(`
+	testCases := []struct {
+		name           string
+		verifyYAML     bool
+		yamlData       []byte
+		expectedResult string
+	}{
+		{
+			name:           "verification disabled, no yaml data supplied = empty output",
+			verifyYAML:     false,
+			yamlData:       []byte{},
+			expectedResult: "",
+		},
+		{
+			name:       "verification disabled, yaml data supplied = non-empty output",
+			verifyYAML: false,
+			yamlData: []byte(`
 vars:
-    _insights_signature: "invalid-signature"
-    _insights_signature_exclude: "/vars/insights_signature,/vars/content_vars"
+    insights_signature: "invalid-signature"
+    insights_signature_exclude: "/vars/insights_signature,/vars/content_vars"
     content: |
         #!/bin/sh
         echo "$RHC_WORKER_FOO $RHC_WORKER_BAR!"
     content_vars:
         FOO: Hello
-        BAR: World`)
-	expectedResult = "Hello World!\n"
-	result = processSignedScript(yamlData)
-	if result != expectedResult {
-		t.Errorf("Expected %q, but got %q", expectedResult, result)
+        BAR: World`),
+			expectedResult: "Hello World!\n",
+		},
+		{
+			name:       "verification enabled, invalid signature = error msg returned",
+			verifyYAML: true,
+			yamlData: []byte(`
+vars:
+    insights_signature: "invalid-signature"
+    insights_signature_exclude: "/vars/insights_signature,/vars/content_vars"
+    content: |
+        #!/bin/sh
+        echo "$RHC_WORKER_FOO $RHC_WORKER_BAR!"
+    content_vars:
+        FOO: Hello
+        BAR: World`),
+			expectedResult: "Signature of yaml file is invalid",
+		},
 	}
 
-	// FIXME: This is false success because verification fails on missing insighs-client
-	// Test case 3: verification enabled, invalid signature = error msg returned
-	shouldVerifyYaml = true
-	shouldDoInsightsCoreGPGCheck = true
-	expectedResult = "Signature of yaml file is invalid"
-	result = processSignedScript(yamlData)
-	if result != expectedResult {
-		t.Errorf("Expected %q, but got %q", expectedResult, result)
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			shouldVerifyYaml := tc.verifyYAML
+			shouldDoInsightsCoreGPGCheck := tc.verifyYAML // Assume the same value for simplicity
+			temporaryWorkerDirectory := t.TempDir()
+			config = &Config{
+				VerifyYAML:               &shouldVerifyYaml,
+				TemporaryWorkerDirectory: &temporaryWorkerDirectory,
+				InsightsCoreGPGCheck:     &shouldDoInsightsCoreGPGCheck,
+			}
+
+			defer os.RemoveAll(temporaryWorkerDirectory)
+
+			result := processSignedScript(tc.yamlData)
+			if result != tc.expectedResult {
+				t.Errorf("Expected %q, but got %q", tc.expectedResult, result)
+			}
+		})
 	}
 }
 
 func TestVerifyYamlFile(t *testing.T) {
-	shouldVerifyYaml := false
-	shouldDoInsightsCoreGPGCheck := false
-
-	config = &Config{
-		VerifyYAML:           &shouldVerifyYaml,
-		InsightsCoreGPGCheck: &shouldDoInsightsCoreGPGCheck,
-	}
-	// Test case 1: verification disabled
-	expectedResult := true
-	result := verifyYamlFile([]byte{})
-	if result != expectedResult {
-		t.Errorf("Expected %v, but got %v", expectedResult, result)
+	testCases := []struct {
+		name                         string
+		yamlData                     []byte
+		verifyYAML                   bool
+		verificationCommand          string
+		verificationArgs             []string
+		shouldDoInsightsCoreGPGCheck bool
+		expectedResult               bool
+	}{
+		{
+			name:                         "verification disabled",
+			verifyYAML:                   false,
+			yamlData:                     []byte{},
+			shouldDoInsightsCoreGPGCheck: false,
+			expectedResult:               true,
+		},
+		{
+			name:                         "verification enabled and verification succeeds",
+			verifyYAML:                   true,
+			yamlData:                     []byte("valid-yaml"),
+			verificationCommand:          "true",
+			verificationArgs:             []string{},
+			shouldDoInsightsCoreGPGCheck: false,
+			expectedResult:               true,
+		},
+		{
+			name:                         "verification is enabled and verification fails",
+			verifyYAML:                   true,
+			yamlData:                     []byte("invalid-yaml"),
+			verificationCommand:          "false",
+			verificationArgs:             []string{},
+			shouldDoInsightsCoreGPGCheck: false,
+			expectedResult:               false,
+		},
 	}
 
-	// Test case 2: verification enabled and verification succeeds
-	shouldVerifyYaml = true
-	// FIXME: This should succedd but now verification fails on missing insighs-client
-	// We also need valid signature
-	expectedResult = false
-	result = verifyYamlFile([]byte("valid-yaml"))
-	if result != expectedResult {
-		t.Errorf("Expected %v, but got %v", expectedResult, result)
-	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			shouldVerifyYaml := tc.verifyYAML
+			shouldDoInsightsCoreGPGCheck := tc.shouldDoInsightsCoreGPGCheck
+			verificationCommand = tc.verificationCommand
+			verificationArgs = tc.verificationArgs
+
+			config = &Config{
+				VerifyYAML:           &shouldVerifyYaml,
+				InsightsCoreGPGCheck: &shouldDoInsightsCoreGPGCheck,
+			}
 
-	// FIXME: Valid test case but fails because of missing insights-client
-	// Test case 3: sverification is enabled and verification fails
-	// shouldVerifyYaml = true
-	expectedResult = false
-	result = verifyYamlFile([]byte("invalid-yaml")) // Replace with your YAML data
-	if result != expectedResult {
-		t.Errorf("Expected %v, but got %v", expectedResult, result)
+			result := verifyYamlFile(tc.yamlData)
+			if result != tc.expectedResult {
+				t.Errorf("Expected %v, but got %v", tc.expectedResult, result)
+			}
+		})
 	}
 }