Refactor ww4test to testenv

- Remove global state
- Remove redundant data
- Remove unused functionality (e.g., file permissions)
- Separate directory creation from file writing
- Additional helper functions for file reading and cleanup

Signed-off-by: Jonathon Anderson <janderson@ciq.com>
Signed-off-by: Christian Goll <cgoll@suse.com>

CHANGELOG.md

@@ -99,8 +99,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   removes a limit on the number of overlays that could be included in a node or
   profile. #852, #876, #883, #896, #903
 - During node discovery, prefer the primary network interface, if defined. #775
-- added general test framework un ww4test
+- added general test framework which creates temporary directories on the fly
-
 ## [4.4.0] 2023-01-18
 
 ### Added

internal/app/wwctl/node/set/main_test.go

@@ -2,11 +2,10 @@ package set
 
 import (
 	"bytes"
-	"os"
 	"testing"
 
 	"github.com/hpcng/warewulf/internal/pkg/warewulfd"
-	"github.com/hpcng/warewulf/internal/pkg/ww4test"
+	"github.com/hpcng/warewulf/internal/pkg/testenv"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -15,16 +14,15 @@ type test_description struct {
 	args    []string
 	wantErr bool
 	stdout  string
-	outDb   string
 	inDB    string
+	outDb   string
 }
 
 func run_test(t *testing.T, test test_description) {
-	//wwlog.SetLogLevel(wwlog.DEBUG)
+	env := testenv.New(t)
-	var env ww4test.WarewulfTestEnv
+	defer env.RemoveAll(t)
-	env.NodesConf = test.inDB
+
-	env.New(t)
+	env.WriteFile(t, "etc/warewulf/nodes.conf", test.inDB)
-	defer os.RemoveAll(env.BaseDir)
 	warewulfd.SetNoDaemon()
 	name := test.name
 	if name == "" {
@@ -43,9 +41,8 @@ func run_test(t *testing.T, test test_description) {
 		} else {
 			assert.NoError(t, err)
 			assert.Equal(t, buf.String(), test.stdout)
-			content, err := os.ReadFile(env.NodesConfFile)
+			content := env.ReadFile(t, "etc/warewulf/nodes.conf")
-			assert.NoError(t, err)
+			assert.Equal(t, test.outDb, content)
-			assert.Equal(t, test.outDb, string(content))
 		}
 	})
 }

internal/pkg/testenv/testenv.go

@@ -0,0 +1,139 @@
+// Package testenv provides functions and data structures for
+// constructing and manipulating a temporary Warewulf environment for
+// use during automated testing.
+//
+// The testenv package should only be used in tests.
+package testenv
+
+import (
+	"os"
+	"path"
+	"path/filepath"
+	"testing"
+
+	"github.com/hpcng/warewulf/internal/pkg/config"
+	"github.com/hpcng/warewulf/internal/pkg/node"
+	"github.com/stretchr/testify/assert"
+)
+
+const initWarewulfConf = `WW_INTERNAL: 0`
+const initDefaultsConf = `WW_INTERNAL: 43`
+const initNodesConf = `WW_INTERNAL: 43
+nodeprofiles:
+  default: {}
+nodes:
+  node1: {}
+`
+
+type TestEnv struct {
+	BaseDir string
+}
+
+// New creates a test environment in a temporary directory and configures
+// Warewulf to use it.
+//
+// Caller is responsible to delete env.BaseDir by calling
+// env.RemoveAll. Note that this does not restore Warewulf to its
+// previous state.
+//
+// Asserts no errors occur.
+func New(t *testing.T) (env *TestEnv) {
+	env = new(TestEnv)
+
+	tmpDir, err := os.MkdirTemp(os.TempDir(), "ww4test-*")
+	assert.NoError(t, err)
+	env.BaseDir = tmpDir
+
+	env.WriteFile(t, "etc/warewulf/nodes.conf", initNodesConf)
+	env.WriteFile(t, "etc/warewulf/warewulf.conf", initWarewulfConf)
+	env.WriteFile(t, "share/warewulf/defaults.conf", initDefaultsConf)
+
+	// re-read warewulf.conf
+	conf := config.New()
+	err = conf.Read(env.GetPath("etc/warewulf/warewulf.conf"))
+	assert.NoError(t, err)
+
+	conf.Paths.Sysconfdir = env.GetPath("etc")
+	conf.Paths.Bindir = env.GetPath("bin")
+	conf.Paths.Datadir = env.GetPath("share")
+	conf.Paths.Localstatedir = env.GetPath("var/local")
+	conf.Paths.Srvdir = env.GetPath("srv")
+	conf.Paths.Tftpdir = env.GetPath("srv/tftp")
+	conf.Paths.Firewallddir = env.GetPath("usr/lib/firewalld/services")
+	conf.Paths.Systemddir = env.GetPath("usr/lib/systemd/system")
+	conf.Paths.WWOverlaydir = env.GetPath(path.Join(conf.Paths.Localstatedir, "warewulfoverlays"))
+	conf.Paths.WWChrootdir = env.GetPath(path.Join(conf.Paths.Localstatedir, "warewulf/chroots"))
+	conf.Paths.WWProvisiondir = env.GetPath(path.Join(conf.Paths.Srvdir, "warewulf"))
+	conf.Paths.WWClientdir = env.GetPath("warewulf")
+
+	for _, confPath := range []string{
+		conf.Paths.Sysconfdir,
+		conf.Paths.Bindir,
+		conf.Paths.Datadir,
+		conf.Paths.Localstatedir,
+		conf.Paths.Srvdir,
+		conf.Paths.Tftpdir,
+		conf.Paths.Firewallddir,
+		conf.Paths.Systemddir,
+		conf.Paths.WWOverlaydir,
+		conf.Paths.WWChrootdir,
+		conf.Paths.WWProvisiondir,
+		conf.Paths.WWClientdir,
+	}{
+		env.MkdirAll(t, confPath)
+	}
+
+	// node.init() has already run, so set the config path again
+	node.ConfigFile = env.GetPath("etc/warewulf/nodes.conf")
+
+	return
+}
+
+// GetPath returns the absolute path name for fileName specified
+// relative to the test environment.
+func (env *TestEnv) GetPath(fileName string) string {
+	return path.Join(env.BaseDir, fileName)
+}
+
+// MkdirAll creates dirName and any intermediate directories relative
+// to the test environment.
+//
+// Asserts no errors occur.
+func (env *TestEnv) MkdirAll(t *testing.T, dirName string) {
+	err := os.MkdirAll(env.GetPath(dirName), 0755)
+	assert.NoError(t, err)
+}
+
+// WriteFile writes content to fileName, creating any necessary
+// intermediate directories relative to the test environment.
+//
+// Asserts no errors occur.
+func (env *TestEnv) WriteFile(t *testing.T, fileName string, content string) {
+	dirName := filepath.Dir(fileName)
+	env.MkdirAll(t, dirName)
+
+	f, err := os.Create(env.GetPath(fileName))
+	assert.NoError(t, err)
+	defer f.Close()
+	_, err = f.WriteString(content)
+	assert.NoError(t, err)
+}
+
+// ReadFile returns the content of fileName as converted to a
+// string.
+//
+// Asserts no errors occur.
+func (env *TestEnv) ReadFile(t *testing.T, fileName string) string {
+	buffer, err := os.ReadFile(env.GetPath(fileName))
+	assert.NoError(t, err)
+	return string(buffer)
+}
+
+// RemoveAll deletes the temporary directory, and all its contents,
+// for the test environment.
+//
+// Asserts no errors occur.
+func (env *TestEnv) RemoveAll(t *testing.T) {
+	err := os.RemoveAll(env.BaseDir)
+	assert.NoError(t, err)
+}

internal/pkg/testenv/testenv_test.go

@@ -1,7 +1,6 @@
-package ww4test
+package testenv
 
 import (
-	"os"
 	"testing"
 
 	"github.com/hpcng/warewulf/internal/pkg/node"
@@ -9,8 +8,8 @@ import (
 )
 
 func Test_Basic(t *testing.T) {
-	Env.New(t)
+	env := New(t)
-	defer os.RemoveAll(Env.BaseDir)
+	defer env.RemoveAll(t)
 	nodedb, err := node.New()
 	assert.NoError(t, err)
 	nodes, err := nodedb.FindAllNodes()
@@ -19,15 +18,15 @@ func Test_Basic(t *testing.T) {
 }
 
 func Test_two_nodes(t *testing.T) {
-	Env.NodesConf = `WW_INTERNAL: 43
+	env := New(t)
+	env.WriteFile(t, "etc/warewulf/nodes.conf", `WW_INTERNAL: 43
 nodeprofiles:
   default: {}
 nodes:
   node1: {}
   node2: {}
-`
+`)
-	Env.New(t)
+	defer env.RemoveAll(t)
-	defer os.RemoveAll(Env.BaseDir)
 	nodedb, err := node.New()
 	assert.NoError(t, err)
 	nodes, err := nodedb.FindAllNodes()

internal/pkg/ww4test/ww4test.go

@@ -1,136 +0,0 @@
-package ww4test
-
-import (
-	"fmt"
-	"io/fs"
-	"os"
-	"path"
-	"testing"
-
-	"github.com/hpcng/warewulf/internal/pkg/config"
-	"github.com/hpcng/warewulf/internal/pkg/node"
-	"github.com/pkg/errors"
-	"github.com/stretchr/testify/assert"
-)
-
-var (
-	Env *WarewulfTestEnv
-)
-
-func init() {
-	Env = new(WarewulfTestEnv)
-	Env.WarewulfConf = `WW_INTERNAL: 0`
-	Env.DefaultsConf = `WW_INTERNAL: 43`
-	Env.NodesConf = `WW_INTERNAL: 43
-nodeprofiles:
-  default: {}
-nodes:
-  node1: {}
-`
-
-}
-
-type ConfFile struct {
-	// directory in which file exists
-	Dir string
-	// filename, if empty only directory is create
-	FileName string
-	// content of file
-	Content string
-	// permissions of file, 644 for file and 755 for
-	// leading directory otherwise
-	Mode fs.FileMode
-}
-
-type WarewulfTestEnv struct {
-	// content of warewulf.conf
-	WarewulfConf string
-	// path to warewulf.conf
-	WarewulfConfFile string
-	// content of nodes.conf
-	NodesConf string
-	// path to nodes.conf
-	NodesConfFile string
-	// content for defaults.conf
-	DefaultsConf string
-	// tmpe dir where the files are created
-	BaseDir string
-	// additional files to be created by New(t)
-	WarewulfFiles []ConfFile
-}
-
-// creates the given file, if DIR is "" only the dir is created
-// any error is returned
-func (env *WarewulfTestEnv) CreateFile(file ConfFile) (err error) {
-	if env.BaseDir == "" {
-		return fmt.Errorf("the variable Basedir is empty, perhaps New() wasn't called")
-	}
-	var dirM fs.FileMode = file.Mode
-	// var fileM fs.FileMode = file.Mode
-	if file.Mode == 0 {
-		dirM = 0755
-		// fileM = 0644
-	}
-	err = os.MkdirAll(path.Join(env.BaseDir, file.Dir), dirM)
-	if err != nil {
-		return errors.Wrap(err, fmt.Sprintf("couldn't create dir: %s", path.Join(env.BaseDir, file.Dir)))
-	}
-	if file.FileName != "" {
-		f, err := os.Create(path.Join(env.BaseDir, file.Dir, file.FileName))
-		if err != nil {
-			return errors.Wrap(err, fmt.Sprintf("couldn't open file: %s", path.Join(env.BaseDir, file.Dir, file.FileName)))
-		}
-		defer f.Close()
-		_, err = f.WriteString(file.Content)
-		return err
-	}
-	return
-}
-
-/*
-Creates a test environment so that warewulf functions can be tested.
-nodes.conf content can be provided either via global Env.WarewulfConf.
-Caller is responsible to delete the created Env.BaseDir
-*/
-func (env *WarewulfTestEnv) New(t *testing.T) {
-	tmpDir, err := os.MkdirTemp(os.TempDir(), "ww4test-*")
-	assert.NoError(t, err)
-	env.BaseDir = tmpDir
-	err = env.CreateFile(ConfFile{
-		Dir:      "etc/warewulf",
-		FileName: "nodes.conf",
-		Content:  env.NodesConf,
-	})
-	assert.NoError(t, err)
-	err = env.CreateFile(ConfFile{
-		Dir:      "etc/warewulf",
-		FileName: "warewulf.conf",
-		Content:  env.WarewulfConf,
-	})
-	assert.NoError(t, err)
-	err = env.CreateFile(ConfFile{
-		Dir:      "share/warewulf",
-		FileName: "defaults.conf",
-		Content:  env.WarewulfConf,
-	})
-	assert.NoError(t, err)
-	env.WarewulfConfFile = path.Join(env.BaseDir, "etc/warewulf/warewulf.conf")
-	env.NodesConfFile = path.Join(env.BaseDir, "etc/warewulf/nodes.conf")
-	conf := config.New()
-	err = conf.Read(env.WarewulfConfFile)
-	assert.NoError(t, err)
-	// init() of node has run before, so set the config path again
-	node.ConfigFile = env.NodesConfFile
-	conf.Paths.Sysconfdir = path.Join(env.BaseDir, "etc")
-	conf.Paths.Bindir = path.Join(env.BaseDir, "bin")
-	conf.Paths.Datadir = path.Join(env.BaseDir, "share")
-	conf.Paths.Localstatedir = path.Join(env.BaseDir, "var/local")
-	conf.Paths.Srvdir = path.Join(env.BaseDir, "srv")
-	conf.Paths.Tftpdir = path.Join(env.BaseDir, "srv/tftp")
-	conf.Paths.Firewallddir = path.Join(env.BaseDir, "usr/lib/firewalld/services")
-	conf.Paths.Systemddir = path.Join(env.BaseDir, "usr/lib/systemd/system")
-	conf.Paths.WWOverlaydir = path.Join(env.BaseDir, conf.Paths.Localstatedir, "warewulfoverlays")
-	conf.Paths.WWChrootdir = path.Join(env.BaseDir, conf.Paths.Localstatedir, "warewulf/chroots")
-	conf.Paths.WWProvisiondir = path.Join(env.BaseDir, conf.Paths.Srvdir, "warewulf")
-	conf.Paths.WWClientdir = path.Join(env.BaseDir, "warewulf")
-}