Expand test descriptions and simplify some tests

As part of reviewing this PR, I expanded the test definitions to make it
easier to discern the intent of each test at a glance. In so doing I
understood most tests behaving as normal, but I did make a few small
changes:

- Some tests no longer produce any errors, so remove this variable from
  the test definitions.
- Replace comma-separated overlay definitions with explicit slices.
  Splitting comma-separated strings led to erroneous behavior where a
  null overlay list was processed as a single null overlay rather than
  an empty slice.

Signed-off-by: Jonathon Anderson <janderson@ciq.com>

internal/pkg/overlay/overlay_test.go

@@ -9,7 +9,6 @@ import (
 	"os"
 	"path"
 	"sort"
-	"strings"
 	"testing"
 )
 
@@ -22,18 +21,110 @@ var buildOverlayTests = []struct {
 	contents    []string
 	hasFiles    bool
 }{
-	{"empty", "", "", nil, "", nil, false},
+	{
-	{"empty node", "node1", "", nil, "", nil, false},
+		description: "if no node, context, or overlays are specified then no overlay image is generated",
-	{"empty context", "", "system", nil, "", nil, false},
+		nodeName:    "",
-	{"empty overlay", "", "", []string{"o1"}, "o1.img", []string{"o1.txt"}, true},
+		context:     "",
-	{"single overlay", "node1", "", []string{"o1"}, "node1/o1.img", []string{"o1.txt"}, true},
+		overlays:    nil,
-	{"multiple overlays", "node1", "", []string{"o1", "o2"}, "node1/o1-o2.img", []string{"o1.txt", "o2.txt"}, true},
+		image:       "",
-	{"empty system overlay", "node1", "system", nil, "", nil, false},
+		contents:    nil,
-	{"empty runtime overlay", "node1", "runtime", nil, "", nil, false},
+	},
-	{"single system overlay", "node1", "system", []string{"o1"}, "node1/__SYSTEM__.img", []string{"o1.txt"}, true},
+	{
-	{"single runtime overlay", "node1", "runtime", []string{"o1"}, "node1/__RUNTIME__.img", []string{"o1.txt"}, true},
+		description: "if only node is specified then no overlay image is generated",
-	{"two system overlays", "node1", "system", []string{"o1", "o2"}, "node1/__SYSTEM__.img", []string{"o1.txt", "o2.txt"}, true},
+		nodeName:    "node1",
-	{"two runtime overlays", "node1", "runtime", []string{"o1", "o2"}, "node1/__RUNTIME__.img", []string{"o1.txt", "o2.txt"}, true},
+		context:     "",
+		overlays:    nil,
+		image:       "",
+		contents:    nil,
+	},
+	{
+		description: "if only context is specified then no overlay image is generated",
+		nodeName:    "",
+		context:     "system",
+		overlays:    nil,
+		image:       "",
+		contents:    nil,
+	},
+	{
+		description: "if an overlay is specified without a node, then the overlay is built directly in the overlay directory",
+		nodeName:    "",
+		context:     "",
+		overlays:    []string{"o1"},
+		image:       "o1.img",
+		contents:    []string{"o1.txt"},
+	},
+	{
+		description: "if multiple overlays are specified without a node, then the combined overlay is built directly in the overlay directory",
+		nodeName:    "",
+		context:     "",
+		overlays:    []string{"o1", "o2"},
+		image:       "o1-o2.img",
+		contents:    []string{"o1.txt", "o2.txt"},
+	},
+	{
+		description: "if a single node overlay is specified, then the overlay is built in a node overlay directory",
+		nodeName:    "node1",
+		context:     "",
+		overlays:    []string{"o1"},
+		image:       "node1/o1.img",
+		contents:    []string{"o1.txt"},
+	},
+	{
+		description: "if multiple node overlays are specified, then the combined overlay is built in a node overlay directory",
+		nodeName:    "node1",
+		context:     "",
+		overlays:    []string{"o1", "o2"},
+		image:       "node1/o1-o2.img",
+		contents:    []string{"o1.txt", "o2.txt"},
+	},
+	{
+		description: "if no node system overlays are specified, then no overlay image is generated",
+		nodeName:    "node1",
+		context:     "system",
+		overlays:    nil,
+		image:       "",
+		contents:    nil,
+	},
+	{
+		description: "if no node runtime overlays are specified, then no overlay image is generated",
+		nodeName:    "node1",
+		context:     "runtime",
+		overlays:    nil,
+		image:       "",
+		contents:    nil,
+	},
+	{
+		description: "if a single node system overlay is specified, then a system overlay image is generated in a node overlay directory",
+		nodeName:    "node1",
+		context:     "system",
+		overlays:    []string{"o1"},
+		image:       "node1/__SYSTEM__.img",
+		contents:    []string{"o1.txt"},
+	},
+	{
+		description: "if a single node runtime overlay is specified, then a runtime overlay image is generated in a node overlay directory",
+		nodeName:    "node1",
+		context:     "runtime",
+		overlays:    []string{"o1"},
+		image:       "node1/__RUNTIME__.img",
+		contents:    []string{"o1.txt"},
+	},
+	{
+		description: "if multiple node system overlays are specified, then a system overlay image is generated with the contents of both overlays",
+		nodeName:    "node1",
+		context:     "system",
+		overlays:    []string{"o1", "o2"},
+		image:       "node1/__SYSTEM__.img",
+		contents:    []string{"o1.txt", "o2.txt"},
+	},
+	{
+		description: "if multiple node runtime overlays are specified, then a runtime overlay image is generated with the contents of both overlays",
+		nodeName:    "node1",
+		context:     "runtime",
+		overlays:    []string{"o1", "o2"},
+		image:       "node1/__RUNTIME__.img",
+		contents:    []string{"o1.txt", "o2.txt"},
+	},
 }
 
 func Test_BuildOverlay(t *testing.T) {
@@ -66,10 +157,10 @@ func Test_BuildOverlay(t *testing.T) {
 			conf.Paths.WWProvisiondir = provisionDir
 
 			err := BuildOverlay(nodeInfo, tt.context, tt.overlays)
-			if tt.hasFiles {
+			assert.NoError(t, err)
+			if tt.image != "" {
 				image := path.Join(provisionDir, "overlays", tt.image)
 				assert.FileExists(t, image)
-				assert.NoError(t, err)
 
 				sort.Strings(tt.contents)
 				files := cpioFiles(t, image)
@@ -84,29 +175,80 @@ func Test_BuildOverlay(t *testing.T) {
 	}
 }
 
+// Although these tests specify system and runtime overlays for the
+// nodes, these overlays define the overlays that are defined in the
+// configuration. BuildAllOverlays doesn't receive these as arguments,
+// but builds all the overlays that are configured on the given node.
 var buildAllOverlaysTests = []struct {
 	description     string
 	nodes           []string
-	systemOverlays  []string
+	systemOverlays  [][]string
-	runtimeOverlays []string
+	runtimeOverlays [][]string
-	succeed         bool
 	createdOverlays []string
 }{
-	{"no nodes", nil, nil, nil, true, nil},
+	{
-	{"single empty node", []string{"node1"}, nil, nil, true, nil},
+		description:     "empty input creates no overlays",
-	{"two empty node", []string{"node1", "node2"}, nil, nil, true, nil},
+		nodes:           nil,
-	{"single node with system overlay", []string{"node1"},
+		systemOverlays:  nil,
-		[]string{"o1"}, nil, true, []string{"__SYSTEM__.img.gz"}},
+		runtimeOverlays: nil,
-	{"two nodes with system overlays", []string{"node1", "node2"},
+		createdOverlays: nil,
-		[]string{"o1", "o1,o2"}, nil, true, []string{"__SYSTEM__.img.gz"}},
+	},
-	{"single node with runtime overlay", []string{"node1"},
+	{
-		nil, []string{"o1"}, true, []string{"__RUNTIME__.img.gz"}},
+		description:     "a node with no overlays creates no overlays",
-	{"two nodes with runtime overlays", []string{"node1", "node2"},
+		nodes:           []string{"node1"},
-		nil, []string{"o1", "o1,o2"}, true, []string{"__RUNTIME__.img.gz"}},
+		systemOverlays:  nil,
-	{"single node with full overlays", []string{"node1"},
+		runtimeOverlays: nil,
-		[]string{"o1"}, []string{"o2"}, true, []string{"__RUNTIME__.img.gz", "__SYSTEM__.img.gz"}},
+		createdOverlays: nil,
-	{"two nodes with full overlays", []string{"node1", "node2"},
+	},
-		[]string{"o1", "o1,o2"}, []string{"o2", "o2"}, true, []string{"__RUNTIME__.img.gz", "__SYSTEM__.img.gz"}},
+	{
+		description:     "multiple nodes with no overlays creates no overlays",
+		nodes:           []string{"node1", "node2"},
+		systemOverlays:  nil,
+		runtimeOverlays: nil,
+		createdOverlays: nil,
+	},
+	{
+		description:     "a system overlay for a node generates a system overlay for that node",
+		nodes:           []string{"node1"},
+		systemOverlays:  [][]string{{"o1"}},
+		runtimeOverlays: nil,
+		createdOverlays: []string{"node1/__SYSTEM__.img.gz"},
+	},
+	{
+		description:     "two nodes with different system overlays generates a system overlay for each node",
+		nodes:           []string{"node1", "node2"},
+		systemOverlays:  [][]string{{"o1"}, {"o1", "o2"}},
+		runtimeOverlays: nil,
+		createdOverlays: []string{"node1/__SYSTEM__.img.gz", "node2/__SYSTEM__.img.gz"},
+	},
+	{
+		description:     "two nodes with a single runtime overlay generates a runtime overlay for the first node",
+		nodes:           []string{"node1"},
+		systemOverlays:  nil,
+		runtimeOverlays: [][]string{{"o1"}},
+		createdOverlays: []string{"node1/__RUNTIME__.img.gz"},
+	},
+	{
+		description:     "two nodes with different runtime overlays generates a system overlay for each node",
+		nodes:           []string{"node1", "node2"},
+		systemOverlays:  nil,
+		runtimeOverlays: [][]string{{"o1"}, {"o1", "o2"}},
+		createdOverlays: []string{"node1/__RUNTIME__.img.gz", "node2/__RUNTIME__.img.gz"},
+	},
+	{
+		description:     "a node with both a runtime and system overlay generates an image for each",
+		nodes:           []string{"node1"},
+		systemOverlays:  [][]string{{"o1"}},
+		runtimeOverlays: [][]string{{"o2"}},
+		createdOverlays: []string{"node1/__RUNTIME__.img.gz", "node1/__SYSTEM__.img.gz"},
+	},
+	{
+		description:     "two nodes with both runtime and system overlays generates each image for each node",
+		nodes:           []string{"node1", "node2"},
+		systemOverlays:  [][]string{{"o1"}, {"o1", "o2"}},
+		runtimeOverlays: [][]string{{"o2"}, {"o2"}},
+		createdOverlays: []string{"node1/__RUNTIME__.img.gz", "node1/__SYSTEM__.img.gz", "node2/__RUNTIME__.img.gz", "node2/__SYSTEM__.img.gz"},
+	},
 }
 
 func Test_BuildAllOverlays(t *testing.T) {
@@ -130,23 +272,21 @@ func Test_BuildAllOverlays(t *testing.T) {
 				nodeInfo := node.NodeInfo{}
 				nodeInfo.Id.Set(nodeName)
 				if tt.systemOverlays != nil {
-					nodeInfo.SystemOverlay.SetSlice(strings.Split(tt.systemOverlays[i], ","))
+					nodeInfo.SystemOverlay.SetSlice(tt.systemOverlays[i])
 				}
 				if tt.runtimeOverlays != nil {
-					nodeInfo.RuntimeOverlay.SetSlice(strings.Split(tt.runtimeOverlays[i], ","))
+					nodeInfo.RuntimeOverlay.SetSlice(tt.runtimeOverlays[i])
 				}
 				nodes = append(nodes, nodeInfo)
 			}
 			err := BuildAllOverlays(nodes)
-			if !tt.succeed {
+			assert.NoError(t, err)
-				assert.Error(t, err)
+			if tt.createdOverlays == nil {
-			} else {
+				dirName := path.Join(provisionDir, "overlays")
-				assert.NoError(t, err)
+				assert.True(t, dirIsEmpty(t, dirName), "%v should be empty, but isn't", dirName)
-				for _, nodeName := range tt.nodes {
+			}
-					for _, file := range tt.createdOverlays {
+			for _, overlayPath := range tt.createdOverlays {
-						assert.FileExists(t, path.Join(provisionDir, "overlays", nodeName, file))
+				assert.FileExists(t, path.Join(provisionDir, "overlays", overlayPath))
-					}
-				}
 			}
 		})
 	}
@@ -155,21 +295,59 @@ func Test_BuildAllOverlays(t *testing.T) {
 var buildSpecificOverlaysTests = []struct {
 	description string
 	nodes       []string
-	overlays    string
+	overlays    []string
 	images      []string
 	succeed     bool
 }{
-	{"no nodes", nil, "", nil, true},
+	{
-	{"single empty node", []string{"node1"}, "", nil, false},
+		description: "building no overlays for no nodes generates no error and no images",
-	{"two empty node", []string{"node1", "node2"}, "", nil, false},
+		nodes:       nil,
-	{"single node with single overlay", []string{"node1"}, "o1",
+		overlays:    nil,
-		[]string{"node1/o1.img"}, true},
+		images:      nil,
-	{"two nodes with single overlay", []string{"node1", "node2"}, "o1",
+		succeed:     true,
-		[]string{"node1/o1.img", "node2/o1.img"}, true},
+	},
-	{"single node with multi overlay", []string{"node1"}, "o1,o2",
+	{
-		[]string{"node1/o1.img", "node1/o2.img"}, true},
+		description: "building no overlays for a node generates no error and no images",
-	{"two nodes with multi overlays", []string{"node1", "node2"}, "o1,o2",
+		nodes:       []string{"node1"},
-		[]string{"node1/o1.img", "node1/o2.img", "node2/o1.img", "node2/o2.img"}, true},
+		overlays:    nil,
+		images:      nil,
+		succeed:     true,
+	},
+	{
+		description: "building no overlays for two nodes generates no error and no images",
+		nodes:       []string{"node1", "node2"},
+		overlays:    nil,
+		images:      nil,
+		succeed:     true,
+	},
+	{
+		description: "building an overlay for a node generates an overlay image in that node's overlay directory",
+		nodes:       []string{"node1"},
+		overlays:    []string{"o1"},
+		images:      []string{"node1/o1.img"},
+		succeed:     true,
+	},
+	{
+		description: "building an overlay for two nodes generates an overlay image in each node's overlay directory",
+		nodes:       []string{"node1", "node2"},
+		overlays:    []string{"o1"},
+		images:      []string{"node1/o1.img", "node2/o1.img"},
+		succeed:     true,
+	},
+	{
+		description: "building multiple overlays for a node generates an overlay image for each overlay in the node's overlay directory",
+		nodes:       []string{"node1"},
+		overlays:    []string{"o1", "o2"},
+		images:      []string{"node1/o1.img", "node1/o2.img"},
+		succeed:     true,
+	},
+	{
+		description: "building multiple overlays for two nodes generates an overlay image for each overlay in each node's overlay directory",
+		nodes:       []string{"node1", "node2"},
+		overlays:    []string{"o1", "o2"},
+		images:      []string{"node1/o1.img", "node1/o2.img", "node2/o1.img", "node2/o2.img"},
+		succeed:     true,
+	},
 }
 
 func Test_BuildSpecificOverlays(t *testing.T) {
@@ -194,7 +372,7 @@ func Test_BuildSpecificOverlays(t *testing.T) {
 				nodeInfo.Id.Set(nodeName)
 				nodes = append(nodes, nodeInfo)
 			}
-			err := BuildSpecificOverlays(nodes, strings.Split(tt.overlays, ","))
+			err := BuildSpecificOverlays(nodes, tt.overlays)
 			if !tt.succeed {
 				assert.Error(t, err)
 			} else {

internal/pkg/warewulfd/util_test.go

@@ -17,16 +17,63 @@ var getOverlayFileTests = []struct {
 	context     string
 	overlays    []string
 	result      string
-	succeed     bool
 }{
-	{"empty", "", "", nil, "", true},
+	{
-	{"empty node", "node1", "", nil, "", true},
+		description: "empty inputs produces no result",
-	{"specific overlays without node", "", "", []string{"o1", "o2"}, "overlays/o1-o2.img", true}, // will fail as node is empty
+		node:        "",
-	{"system overlay", "node1", "system", []string{"o1"}, "overlays/node1/__SYSTEM__.img", true},
+		context:     "",
-	{"runtime overlay", "node1", "runtime", nil, "overlays/node1/__RUNTIME__.img", true},
+		overlays:    nil,
-	{"specific overlay", "node1", "", []string{"o1"}, "overlays/node1/o1.img", true},
+		result:      "",
-	{"multiple specific overlays", "node1", "", []string{"o1", "o2"}, "overlays/node1/o1-o2.img", true},
+	},
-	{"multiple specific overlays with context", "node1", "system", []string{"o1", "o2"}, "overlays/node1/__SYSTEM__.img", true},
+	{
+		description: "a node with no context or overlays produces no result",
+		node:        "node1",
+		context:     "",
+		overlays:    nil,
+		result:      "",
+	},
+	{
+		description: "overlays with no node or context points to a combined overlay image",
+		node:        "",
+		context:     "",
+		overlays:    []string{"o1", "o2"},
+		result:      "overlays/o1-o2.img",
+	},
+	{
+		description: "system overlay for a node points to the node's system overlay image",
+		node:        "node1",
+		context:     "system",
+		overlays:    []string{"o1"},
+		result:      "overlays/node1/__SYSTEM__.img",
+	},
+	{
+		description: "runtime overlay for a node points to the node's runtime overlay image",
+		node:        "node1",
+		context:     "runtime",
+		overlays:    nil,
+		result:      "overlays/node1/__RUNTIME__.img",
+	},
+	{
+		description: "a specific overlay for a node points to that specific overlay image for that node",
+		node:        "node1",
+		context:     "",
+		overlays:    []string{"o1"},
+		result:      "overlays/node1/o1.img",
+	},
+	{
+		description: "a specific set of overlays for a node points to a combined overlay image for that node",
+		node:        "node1",
+		context:     "",
+		overlays:    []string{"o1", "o2"},
+		result:      "overlays/node1/o1-o2.img",
+	},
+	{
+		description: "a specific set of overlays for a node while also specifying a context points to the contextual overlay image for that node",
+		node:        "node1",
+		context:     "system",
+		overlays:    []string{"o1", "o2"},
+		result:      "overlays/node1/__SYSTEM__.img",
+	},
 }
 
 func Test_getOverlayFile(t *testing.T) {
@@ -50,11 +97,7 @@ func Test_getOverlayFile(t *testing.T) {
 			nodeInfo.RuntimeOverlay.SetSlice(tt.overlays)
 			nodeInfo.SystemOverlay.SetSlice(tt.overlays)
 			result, err := getOverlayFile(nodeInfo, tt.context, tt.overlays, false)
-			if !tt.succeed {
+			assert.NoError(t, err)
-				assert.Error(t, err)
-			} else {
-				assert.NoError(t, err)
-			}
 			if tt.result != "" {
 				tt.result = path.Join(overlayPDir, tt.result)
 			}