Finished implementing config tests

11 files changed, 228 insertions, 36 deletions

diff --git a/config/app_test.go b/config/app_test.go
index cda891d..5a6cf41 100644
--- a/config/app_test.go
+++ b/config/app_test.go
@@ -55,9 +55,7 @@ func TestAppvalidate(t *testing.T) {
 		t.Fatal("Negative or zero MenuMaxIdleTime should not be valid.")
 	}
 
-	//PostLoginCommands is tested from command.go
-
-	// A valid App
+	//PostLoginCommands are mostly tested from command_test.go
 	app = App{
 		WorkingDirectory: "var/",
 		MaxUsers:         512,
@@ -65,6 +63,28 @@ func TestAppvalidate(t *testing.T) {
 		MenuMaxIdleTime:  60,
 	}
 	if err := app.validate(); err != nil {
+		t.Fatal("Empty PostLoginCommands list should be valid")
+	}
+	app = App{
+		WorkingDirectory:  "var/",
+		MaxUsers:          512,
+		MaxNickLen:        15,
+		MenuMaxIdleTime:   60,
+		PostLoginCommands: []string{"invalid"},
+	}
+	if err := app.validate(); err == nil {
+		t.Fatal("Invalid command in PostLoginCommands should not be valid")
+	}
+
+	// A valid App
+	app = App{
+		WorkingDirectory:  "var/",
+		MaxUsers:          512,
+		MaxNickLen:        15,
+		MenuMaxIdleTime:   60,
+		PostLoginCommands: []string{"wait"},
+	}
+	if err := app.validate(); err != nil {
 		t.Fatal("A valid app should pass")
 	}
 }
diff --git a/config/config.go b/config/config.go
index aa1672a..9a161ed 100644
--- a/config/config.go
+++ b/config/config.go
@@ -44,20 +44,20 @@ func LoadFile(path string) (config Config, err error) {
 	var f *os.File
 	f, err = os.Open(path)
 	if err != nil {
-		return
+		return config, errors.Wrapf(err, "Failed to open configuration file %s", path)
 	}
 	defer f.Close()
 	decoder := yaml.NewDecoder(f)
 	if err = decoder.Decode(&config); err != nil {
-		return
+		return config, errors.Wrap(err, "Failed to decode configuration file")
 	}
 	if err = config.validate(); err != nil {
-		return
+		return config, errors.Wrap(err, "Failed to validate configuration")
 	}
 	// If all looks good we validate menu consistency
 	for _, v := range config.Menus {
 		if err = v.validateConsistency(&config); err != nil {
-			return
+			return config, errors.Wrap(err, "Failed menu consistency checks")
 		}
 	}
 	return
diff --git a/config/config_test.go b/config/config_test.go
index 44df5ed..2bc5489 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -54,16 +54,6 @@ func TestLoadFile(t *testing.T) {
 		t.Fatalf("minimal example failed:\nerror %v\nwant:%+v\ngot: %+v", err, want, config)
 	}
 
-	// TODO test non existant game in play actions, and duplicate
-	//menuEntry = MenuEntry{
-	//Key:    "p",
-	//Label:  "play non existant game",
-	//Action: "play nonexistant",
-	//}
-	//if err := menuEntry.validate(); err == nil {
-	//t.Fatal("An inexistant game cannot be played")
-	//}
-
 	t.Cleanup(func() { os.RemoveAll("var/") })
 	// Invalid App
 	if _, err := LoadFile("test_data/invalid_app.yaml"); err == nil {
@@ -101,10 +91,18 @@ func TestLoadFile(t *testing.T) {
 	if _, err := LoadFile("test_data/unreachable_menu.yaml"); err == nil {
 		t.Fatal("unreachable menu should fail to load")
 	}
+	// invalid game
+	if _, err := LoadFile("test_data/invalid_game.yaml"); err == nil {
+		t.Fatal("invalid game should fail to load")
+	}
 	// unreachable game
 	if _, err := LoadFile("test_data/unreachable_game.yaml"); err == nil {
 		t.Fatal("unreachable game should fail to load")
 	}
+	// duplicate game
+	if _, err := LoadFile("test_data/duplicate_game.yaml"); err == nil {
+		t.Fatal("unreachable game should fail to load")
+	}
 
 	// Complexe example
 	want = Config{
@@ -205,12 +203,8 @@ func TestLoadFile(t *testing.T) {
 		},
 		Games: map[string]Game{
 			"nethack3.7": Game{
-				ChrootPath: "/opt/nethack",
+				ChrootPath: "test_data/fake_nethack_directory",
 				FileMode:   "0666",
-				ScoreCommands: []string{
-					"exec /games/nethack -s all",
-					"wait",
-				},
 				Commands: []string{
 					"cp /games/var/save/%u%n.gz /games/var/save/%u%n.gz.bak",
 					"exec /games/nethack -u %n",
diff --git a/config/game.go b/config/game.go
index c34c697..b5f4d39 100644
--- a/config/game.go
+++ b/config/game.go
@@ -1,11 +1,15 @@
 package config
 
 import (
-	"errors"
 	"regexp"
+
+	"github.com/pkg/errors"
+	"golang.org/x/sys/unix"
 )
 
 var reValidGameName = regexp.MustCompile(`^[\w\._]+$`)
+var reValidFileMode = regexp.MustCompile(`^0[\d]{3}$`)
+var reSpace = regexp.MustCompile(`^\s$`)
 
 // Game struct containers the configuration for a game
 type Game struct {
@@ -15,8 +19,6 @@ type Game struct {
 	FileMode string `yaml:"FileMode"`
 	// Commands is the command list
 	Commands []string `yaml:"Commands"`
-	// ScoreFile is relative to the chroot path for the game
-	ScoreCommands []string `yaml:"ScoreCommands"`
 	// Env is the environment in which to exec the commands
 	Env map[string]string `yaml:"Env"`
 }
@@ -27,9 +29,35 @@ func (g *Game) validate(name string) error {
 		return errors.New("Invalid Game name, must match regex `^[\\w\\._]+$` : " + name)
 	}
 	// ChrootPath  TODO
+	if err := unix.Access(g.ChrootPath, unix.R_OK|unix.X_OK); err != nil {
+		return errors.Wrapf(err, "Invalid ChrootPath : %s", g.ChrootPath)
+	}
 	// FileMode
+	if ok := reValidFileMode.MatchString(g.FileMode); !ok {
+		return errors.New("Invalid File Mode, must match regex `^0[\\d]{3}$` : " + name)
+	}
 	// Commands
-	// ScoreFile
+	if len(g.Commands) == 0 {
+		return errors.New("Invalid game " + name + " has no commands")
+	}
+	for i := 0; i < len(g.Commands); i++ {
+		if err := validateCommand(g.Commands[i]); err != nil {
+			return errors.Wrapf(err, "Failed to validate Commands for game %s", name)
+		}
+	}
 	// Env
+	for k, _ := range g.Env {
+		for _, c := range k {
+			switch c {
+			case '=':
+				return errors.New("Environment variable key must not contain equal sign")
+			case '\000':
+				return errors.New("Environment variable key must not contain null character")
+			}
+			if reSpace.MatchString(string(c)) {
+				return errors.New("Environment variable key must not contain spaces")
+			}
+		}
+	}
 	return nil
 }
diff --git a/config/game_test.go b/config/game_test.go
index d295ce2..b3bf107 100644
--- a/config/game_test.go
+++ b/config/game_test.go
@@ -3,8 +3,105 @@ package config
 import "testing"
 
 func TestGameValidate(t *testing.T) {
-	empty := Game{}
-	if err := empty.validate("invalid game name because of spaces"); err == nil {
-		t.Fatal("invalid game name")
+	// Game name
+	game := Game{}
+	if err := game.validate("invalid game name because of spaces"); err == nil {
+		t.Fatal("game name with spaces should not be valid")
+	}
+	// ChrootPath
+	game = Game{ChrootPath: "test_data/non_existant"}
+	if err := game.validate("test"); err == nil {
+		t.Fatal("non_existant ChrootPath should not be valid")
+	}
+	// FileMode
+	game = Game{
+		ChrootPath: "test_data/fake_nethack_directory",
+	}
+	if err := game.validate("test"); err == nil {
+		t.Fatal("Invalid FileMode should not be valid")
+	}
+	game = Game{
+		ChrootPath: "test_data/fake_nethack_directory",
+		FileMode:   "abcd",
+	}
+	if err := game.validate("test"); err == nil {
+		t.Fatal("Invalid FileMode should not be valid")
+	}
+	game = Game{
+		ChrootPath: "test_data/fake_nethack_directory",
+		FileMode:   "777",
+	}
+	if err := game.validate("test"); err == nil {
+		t.Fatal("Invalid FileMode should not be valid")
+	}
+	// Commands are mostly tested from command_test.go
+	game = Game{
+		ChrootPath: "test_data/fake_nethack_directory",
+		FileMode:   "0777",
+	}
+	if err := game.validate("test"); err == nil {
+		t.Fatal("Empty Commands list should not be valid")
+	}
+	game = Game{
+		ChrootPath: "test_data/fake_nethack_directory",
+		FileMode:   "0777",
+		Commands:   []string{"invalid"},
+	}
+	if err := game.validate("test"); err == nil {
+		t.Fatal("Invalid command in Commands should not be valid")
+	}
+	// Env
+	game = Game{
+		ChrootPath: "test_data/fake_nethack_directory",
+		FileMode:   "0777",
+		Commands:   []string{"wait"},
+	}
+	if err := game.validate("test"); err != nil {
+		t.Fatal("Empty env list should be valid")
+	}
+	game = Game{
+		ChrootPath: "test_data/fake_nethack_directory",
+		FileMode:   "0777",
+		Commands:   []string{"wait"},
+		Env: map[string]string{
+			"test invalid": "test",
+		},
+	}
+	if err := game.validate("test"); err == nil {
+		t.Fatal("Spaces in environnement variable name are invalid")
+	}
+	game = Game{
+		ChrootPath: "test_data/fake_nethack_directory",
+		FileMode:   "0777",
+		Commands:   []string{"wait"},
+		Env: map[string]string{
+			"test\000invalid": "test",
+		},
+	}
+	if err := game.validate("test"); err == nil {
+		t.Fatal("null character in environnement variable name are invalid")
+	}
+	game = Game{
+		ChrootPath: "test_data/fake_nethack_directory",
+		FileMode:   "0777",
+		Commands:   []string{"wait"},
+		Env: map[string]string{
+			"test=invalid": "test",
+		},
+	}
+	if err := game.validate("test"); err == nil {
+		t.Fatal("equals symbol in environnement variable name are invalid")
+	}
+	// Valid Game entry
+	game = Game{
+		ChrootPath: "test_data/fake_nethack_directory",
+		FileMode:   "0777",
+		Commands:   []string{"wait"},
+		Env: map[string]string{
+			"test": "test",
+		},
+	}
+	if err := game.validate("test"); err != nil {
+		t.Fatalf("Valid game entry should pass but got error %s", err)
 	}
 }
diff --git a/config/menu.go b/config/menu.go
index f405ec5..a8ea7b0 100644
--- a/config/menu.go
+++ b/config/menu.go
@@ -70,9 +70,10 @@ func (m *Menu) validateConsistency(c *Config) error {
 		return errors.New("No logged_in menu declared")
 	}
 	// Validate actions
-	menus := make(map[string]bool)
-	menus["anonymous"] = true
-	menus["logged_in"] = true
+	menus := map[string]bool{
+		"anonymous": true,
+		"logged_in": true,
+	}
 	playable := make(map[string]bool)
 	for k, v := range c.Menus {
 		for _, e := range v.MenuEntries {
diff --git a/config/test_data/duplicate_game.yaml b/config/test_data/duplicate_game.yaml
new file mode 100644
index 0000000..f01a017
--- /dev/null
+++ b/config/test_data/duplicate_game.yaml
@@ -0,0 +1,30 @@
+App:
+  WorkingDirectory: var/
+  MaxUsers: 1
+  AllowRegistration: true
+  MaxNickLen: 15
+  MenuMaxIdleTime: 600
+
+Menus:
+  anonymous:
+    MenuEntries:
+      - Key: q
+        Label: quit
+        Action: quit
+  logged_in:
+    MenuEntries:
+      - Key: p
+        Label: play
+        Action: play test
+
+Games:
+  test:
+    ChrootPath: test_data/fake_nethack_directory
+    FileMode:   0777
+    Commands:
+      - wait
+  test:
+    ChrootPath: test_data/fake_nethack_directory
+    FileMode:   0777
+    Commands:
+      - wait
diff --git a/config/test_data/fake_nethack_directory/.keep b/config/test_data/fake_nethack_directory/.keep
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/config/test_data/fake_nethack_directory/.keep
diff --git a/config/test_data/invalid_game.yaml b/config/test_data/invalid_game.yaml
new file mode 100644
index 0000000..d58c3ee
--- /dev/null
+++ b/config/test_data/invalid_game.yaml
@@ -0,0 +1,21 @@
+App:
+  WorkingDirectory: var/
+  MaxUsers: 1
+  AllowRegistration: true
+  MaxNickLen: 15
+  MenuMaxIdleTime: 600
+
+Menus:
+  anonymous:
+    MenuEntries:
+      - Key: q
+        Label: quit
+        Action: quit
+  logged_in:
+    MenuEntries:
+      - Key: p
+        Label: play
+        Action: play test
+
+Games:
+  test:
diff --git a/config/test_data/unreachable_game.yaml b/config/test_data/unreachable_game.yaml
index 5af610b..f2f22e2 100644
--- a/config/test_data/unreachable_game.yaml
+++ b/config/test_data/unreachable_game.yaml
@@ -19,3 +19,7 @@ Menus:
 
 Games:
   unreachable:
+    ChrootPath: test_data/fake_nethack_directory
+    FileMode:   0777
+    Commands:
+      - wait
diff --git a/example/complete.yaml b/example/complete.yaml
index 2677285..18a33d7 100644
--- a/example/complete.yaml
+++ b/example/complete.yaml
@@ -64,11 +64,8 @@ Menus:
 
 Games:
   nethack3.7:
-    ChrootPath: /opt/nethack
+    ChrootPath: test_data/fake_nethack_directory
     FileMode: "0666"
-    ScoreCommands:
-      - exec /games/nethack -s all
-      - wait
     Commands:
       - cp /games/var/save/%u%n.gz /games/var/save/%u%n.gz.bak
       - exec /games/nethack -u %n