From 973713bc05f102aa075cbf0c2771db2b2aab3197 Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Fri, 26 Jun 2020 14:37:26 +0200
Subject: [PATCH] ReadKVEnvStrings/ReadKVStrings return nil if empty, and add
 tests

If no env-vars were loaded from "files", and "overrides" was nil,
the code returned an empty slice instead of a `nil` value.

Also add a test for this function, as no unit test was present yet.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 opts/parse.go      |  2 +-
 opts/parse_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 opts/parse_test.go

diff --git a/opts/parse.go b/opts/parse.go
index 70b60e142f..327c2775f6 100644
--- a/opts/parse.go
+++ b/opts/parse.go
@@ -23,7 +23,7 @@ func ReadKVEnvStrings(files []string, override []string) ([]string, error) {
 }
 
 func readKVStrings(files []string, override []string, emptyFn func(string) (string, bool)) ([]string, error) {
-	variables := []string{}
+	var variables []string
 	for _, ef := range files {
 		parsedVars, err := parseKeyValueFile(ef, emptyFn)
 		if err != nil {
diff --git a/opts/parse_test.go b/opts/parse_test.go
new file mode 100644
index 0000000000..c6152f815b
--- /dev/null
+++ b/opts/parse_test.go
@@ -0,0 +1,72 @@
+package opts
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+	"gotest.tools/v3/env"
+	"gotest.tools/v3/fs"
+)
+
+func TestReadKVEnvStrings(t *testing.T) {
+	emptyEnvFile := fs.NewFile(t, t.Name())
+	defer emptyEnvFile.Remove()
+
+	// EMPTY_VAR should be set, but with an empty string as value
+	// FROM_ENV value should be substituted with FROM_ENV in the current environment
+	// NO_SUCH_ENV does not exist in the current environment, so should be omitted
+	envFile1 := fs.NewFile(t, t.Name(), fs.WithContent(`Z1=z
+EMPTY_VAR=
+FROM_ENV
+NO_SUCH_ENV
+`))
+	defer envFile1.Remove()
+	envFile2 := fs.NewFile(t, t.Name(), fs.WithContent("Z2=z\nA2=a"))
+	defer envFile2.Remove()
+	defer env.Patch(t, "FROM_ENV", "from-env")()
+
+	tests := []struct {
+		name      string
+		files     []string
+		overrides []string
+		expected  []string
+	}{
+		{
+			name: "empty",
+		},
+		{
+			name:  "empty file",
+			files: []string{emptyEnvFile.Path()},
+		},
+		{
+			name:     "single file",
+			files:    []string{envFile1.Path()},
+			expected: []string{"Z1=z", "EMPTY_VAR=", "FROM_ENV=from-env"},
+		},
+		{
+			name:     "two files",
+			files:    []string{envFile1.Path(), envFile2.Path()},
+			expected: []string{"Z1=z", "EMPTY_VAR=", "FROM_ENV=from-env", "Z2=z", "A2=a"},
+		},
+		{
+			name:      "single file and override",
+			files:     []string{envFile1.Path()},
+			overrides: []string{"Z1=override", "EXTRA=extra"},
+			expected:  []string{"Z1=z", "EMPTY_VAR=", "FROM_ENV=from-env", "Z1=override", "EXTRA=extra"},
+		},
+		{
+			name:      "overrides only",
+			overrides: []string{"Z1=z", "EMPTY_VAR="},
+			expected:  []string{"Z1=z", "EMPTY_VAR="},
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			envs, err := ReadKVEnvStrings(tc.files, tc.overrides)
+			assert.NilError(t, err)
+			assert.DeepEqual(t, tc.expected, envs)
+		})
+	}
+}