tests: Allow to run maddyctl commands in integration tests

internal/cli/app.go

@@ -1,7 +1,6 @@
 package maddycli
 
 import (
-	"flag"
 	"fmt"
 	"os"
 	"strings"
@@ -30,10 +29,6 @@ databases used by it (all other subcommands).
 	}
 	app.ExitErrHandler = func(c *cli.Context, err error) {
 		cli.HandleExitCoder(err)
-		if err != nil {
-			log.Println(err)
-			cli.OsExiter(1)
-		}
 	}
 	app.EnableBashCompletion = true
 	app.Commands = []*cli.Command{
@@ -66,9 +61,6 @@ databases used by it (all other subcommands).
 
 func AddGlobalFlag(f cli.Flag) {
 	app.Flags = append(app.Flags, f)
-	if err := f.Apply(flag.CommandLine); err != nil {
-		log.Println("GlobalFlag", f, "could not be mapped to stdlib flag:", err)
-	}
 }
 
 func AddSubcommand(cmd *cli.Command) {
@@ -83,15 +75,27 @@ func AddSubcommand(cmd *cli.Command) {
 			return cmd.Action(c)
 		}
 		app.Flags = append(app.Flags, cmd.Flags...)
-		for _, f := range cmd.Flags {
-			if err := f.Apply(flag.CommandLine); err != nil {
-				log.Println("GlobalFlag", f, "could not be mapped to stdlib flag:", err)
-			}
-		}
 	}
 }
 
+// RunWithoutExit is like Run but returns exit code instead of calling os.Exit
+// To be used in maddy.cover.
+func RunWithoutExit() int {
+	code := 0
+
+	cli.OsExiter = func(c int) { code = c }
+	defer func() {
+		cli.OsExiter = os.Exit
+	}()
+
+	Run()
+
+	return code
+}
+
 func Run() {
+	mapStdlibFlags(app)
+
 	// Actual entry point is registered in maddy.go.
 
 	// Print help when called via maddyctl executable. To be removed

internal/cli/extflag.go

@@ -0,0 +1,60 @@
+package maddycli
+
+import (
+	"flag"
+
+	"github.com/urfave/cli/v2"
+)
+
+// extFlag implements cli.Flag via standard flag.Flag.
+type extFlag struct {
+	f *flag.Flag
+}
+
+func (e *extFlag) Apply(fs *flag.FlagSet) error {
+	fs.Var(e.f.Value, e.f.Name, e.f.Usage)
+	return nil
+}
+
+func (e *extFlag) Names() []string {
+	return []string{e.f.Name}
+}
+
+func (e *extFlag) IsSet() bool {
+	return false
+}
+
+func (e *extFlag) String() string {
+	return cli.FlagStringer(e)
+}
+
+func (e *extFlag) IsVisible() bool {
+	return true
+}
+
+func (e *extFlag) TakesValue() bool {
+	return false
+}
+
+func (e *extFlag) GetUsage() string {
+	return e.f.Usage
+}
+
+func (e *extFlag) GetValue() string {
+	return e.f.Value.String()
+}
+
+func (e *extFlag) GetDefaultText() string {
+	return e.f.DefValue
+}
+
+func (e *extFlag) GetEnvVars() []string {
+	return nil
+}
+
+func mapStdlibFlags(app *cli.App) {
+	// Modified AllowExtFlags from cli lib with -test.* exception removed.
+	flag.VisitAll(func(f *flag.Flag) {
+		app.Flags = append(app.Flags, &extFlag{f})
+	})
+}

maddy.go

@@ -284,7 +284,7 @@ func ensureDirectoryWritable(path string) error {
 		return err
 	}
 	testFile.Close()
-	return os.Remove(testFile.Name())
+	return os.RemoveAll(testFile.Name())
 }
 
 func ReadGlobals(cfg []config.Node) (map[string]interface{}, []config.Node, error) {

tests/cover_test.go

@@ -42,8 +42,10 @@ import (
 	"os"
 	"testing"
 
-	"github.com/foxcpp/maddy"
-	"github.com/urfave/cli/v2"
+	_ "github.com/foxcpp/maddy"                  // To register run command
+	_ "github.com/foxcpp/maddy/internal/cli/ctl" // To register other CLI commands.
+
+	maddycli "github.com/foxcpp/maddy/internal/cli"
 )
 
 func TestMain(m *testing.M) {
@@ -56,16 +58,14 @@ func TestMain(m *testing.M) {
 		panic(err)
 	}
 
+	// Skip flag parsing and make flag.Parse no-op so when
+	// m.Run calls it it will not error out on maddy flags.
+	args := os.Args
+	os.Args = []string{"command"}
 	flag.Parse()
+	os.Args = args
 
-	app := cli.NewApp()
-	// maddycli wrapper registers all necessary flags with flag.CommandLine by default
-	ctx := cli.NewContext(app, flag.CommandLine, nil)
-	err = maddy.Run(ctx)
-	code := 0
-	if ec, ok := err.(cli.ExitCoder); ok {
-		code = ec.ExitCode()
-	}
+	code := maddycli.RunWithoutExit()
 
 	if err := os.Chdir(wd); err != nil {
 		panic(err)

tests/t.go

@@ -25,6 +25,7 @@ package tests
 
 import (
 	"bufio"
+	"bytes"
 	"flag"
 	"fmt"
 	"math/rand"
@@ -34,6 +35,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -129,14 +131,7 @@ func (t *T) Env(kv string) {
 	t.env = append(t.env, kv)
 }
 
-// Run completes the configuration of test environment and starts the test server.
-//
-// T.Close should be called by the end of test to release any resources and
-// shutdown the server.
-//
-// The parameter waitListeners specifies the amount of listeners the server is
-// supposed to configure. Run() will block before all of them are up.
-func (t *T) Run(waitListeners int) {
+func (t *T) ensureCanRun() {
 	if t.cfg == "" {
 		panic("tests: Run called without configuration set")
 	}
@@ -146,66 +141,75 @@ func (t *T) Run(waitListeners int) {
 		// any DNS queries to the real world.
 		t.Log("NOTE: Explicit DNS(nil) is recommended.")
 		t.DNS(nil)
+
+		t.Cleanup(func() {
+			// Shutdown the DNS server after maddy to make sure it will not spend time
+			// timing out queries.
+			if err := t.dnsServ.Close(); err != nil {
+				t.Log("Unable to stop the DNS server:", err)
+			}
+			t.dnsServ = nil
+		})
 	}
 
 	// Setup file system, create statedir, runtimedir, write out config.
-	testDir, err := os.MkdirTemp("", "maddy-tests-")
-	if err != nil {
-		t.Fatal("Test configuration failed:", err)
-	}
-	t.testDir = testDir
+	if t.testDir == "" {
+		testDir, err := os.MkdirTemp("", "maddy-tests-")
+		if err != nil {
+			t.Fatal("Test configuration failed:", err)
+		}
+		t.testDir = testDir
+		t.Log("using", t.testDir)
 
-	t.Log("Using", t.testDir)
-
-	defer func() {
-		if !t.Failed() {
-			return
+		if err := os.MkdirAll(filepath.Join(t.testDir, "statedir"), os.ModePerm); err != nil {
+			t.Fatal("Test configuration failed:", err)
+		}
+		if err := os.MkdirAll(filepath.Join(t.testDir, "runtimedir"), os.ModePerm); err != nil {
+			t.Fatal("Test configuration failed:", err)
 		}
 
-		// Clean-up on test failure (if Run failed somewhere)
+		t.Cleanup(func() {
+			if !t.Failed() {
+				return
+			}
 
-		t.dnsServ.Close()
-		t.dnsServ = nil
-
-		os.RemoveAll(t.testDir)
-		t.testDir = ""
-	}()
-
-	if err := os.MkdirAll(filepath.Join(t.testDir, "statedir"), os.ModePerm); err != nil {
-		t.Fatal("Test configuration failed:", err)
-	}
-	if err := os.MkdirAll(filepath.Join(t.testDir, "runtimedir"), os.ModePerm); err != nil {
-		t.Fatal("Test configuration failed:", err)
+			t.Log("removing", t.testDir)
+			os.RemoveAll(t.testDir)
+			t.testDir = ""
+		})
 	}
 
 	configPreable := "state_dir " + filepath.Join(t.testDir, "statedir") + "\n" +
-		"runtime_dir " + filepath.Join(t.testDir, "runtime") + "\n\n"
+		"runtime_dir " + filepath.Join(t.testDir, "runtimedir") + "\n\n"
 
-	err = os.WriteFile(filepath.Join(t.testDir, "maddy.conf"), []byte(configPreable+t.cfg), os.ModePerm)
+	err := os.WriteFile(filepath.Join(t.testDir, "maddy.conf"), []byte(configPreable+t.cfg), os.ModePerm)
 	if err != nil {
 		t.Fatal("Test configuration failed:", err)
 	}
+}
 
+func (t *T) buildCmd(additionalArgs ...string) *exec.Cmd {
 	// Assigning 0 by default will make outbound SMTP unusable.
 	remoteSmtp := "0"
 	if port := t.ports["remote_smtp"]; port != 0 {
 		remoteSmtp = strconv.Itoa(int(port))
 	}
 
-	cmd := exec.Command(TestBinary,
-		"-config", filepath.Join(t.testDir, "maddy.conf"),
+	args := []string{"-config", filepath.Join(t.testDir, "maddy.conf"),
 		"-debug.smtpport", remoteSmtp,
 		"-debug.dnsoverride", t.dnsServ.LocalAddr().String(),
-		"-log", "stderr")
+		"-log", "/tmp/test.log"}
 
 	if CoverageOut != "" {
-		cmd.Args = append(cmd.Args, "-test.coverprofile", CoverageOut+"."+strconv.FormatInt(time.Now().UnixNano(), 16))
+		args = append(args, "-test.coverprofile", CoverageOut+"."+strconv.FormatInt(time.Now().UnixNano(), 16))
 	}
 	if DebugLog {
-		cmd.Args = append(cmd.Args, "-debug")
+		args = append(args, "-debug")
 	}
 
-	t.Logf("launching %v", cmd.Args)
+	args = append(args, additionalArgs...)
+
+	cmd := exec.Command(TestBinary, args...)
 
 	pwd, err := os.Getwd()
 	if err != nil {
@@ -217,19 +221,79 @@ func (t *T) Run(waitListeners int) {
 	cmd.Env = append(cmd.Env,
 		"TEST_PWD="+pwd,
 		"TEST_STATE_DIR="+filepath.Join(t.testDir, "statedir"),
-		"TEST_RUNTIME_DIR="+filepath.Join(t.testDir, "statedir"),
+		"TEST_RUNTIME_DIR="+filepath.Join(t.testDir, "runtimedir"),
 	)
 	for name, port := range t.ports {
 		cmd.Env = append(cmd.Env, fmt.Sprintf("TEST_PORT_%s=%d", name, port))
 	}
 	cmd.Env = append(cmd.Env, t.env...)
 
+	return cmd
+}
+
+func (t *T) MustRunCLIGroup(args ...[]string) {
+	t.ensureCanRun()
+
+	wg := sync.WaitGroup{}
+	for _, arg := range args {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			_, err := t.RunCLI(arg...)
+			if err != nil {
+				t.Fatalf("maddy %v: %v", arg, err)
+			}
+		}()
+	}
+	wg.Wait()
+}
+
+func (t *T) MustRunCLI(args ...string) string {
+	s, err := t.RunCLI(args...)
+	if err != nil {
+		t.Fatalf("maddy %v: %v", args, err)
+	}
+	return s
+}
+
+func (t *T) RunCLI(args ...string) (string, error) {
+	t.ensureCanRun()
+	cmd := t.buildCmd(args...)
+
+	var stderr, stdout bytes.Buffer
+	cmd.Stderr = &stderr
+	cmd.Stdout = &stdout
+
+	t.Log("launching maddy", cmd.Args)
+	if err := cmd.Run(); err != nil {
+		t.Log("Stderr:", stderr.String())
+		t.Fatal("Test configuration failed:", err)
+	}
+
+	t.Log("Stderr:", stderr.String())
+
+	return stdout.String(), nil
+}
+
+// Run completes the configuration of test environment and starts the test server.
+//
+// T.Close should be called by the end of test to release any resources and
+// shutdown the server.
+//
+// The parameter waitListeners specifies the amount of listeners the server is
+// supposed to configure. Run() will block before all of them are up.
+func (t *T) Run(waitListeners int) {
+	t.ensureCanRun()
+	cmd := t.buildCmd("run")
+
 	// Capture maddy log and redirect it.
 	logOut, err := cmd.StderrPipe()
 	if err != nil {
 		t.Fatal("Test configuration failed:", err)
 	}
 
+	t.Log("launching maddy", cmd.Args)
 	if err := cmd.Start(); err != nil {
 		t.Fatal("Test configuration failed:", err)
 	}
@@ -264,6 +328,8 @@ func (t *T) Run(waitListeners int) {
 	}
 
 	t.servProc = cmd
+
+	t.Cleanup(t.killServer)
 }
 
 func (t *T) StateDir() string {
@@ -274,7 +340,7 @@ func (t *T) RuntimeDir() string {
 	return filepath.Join(t.testDir, "statedir")
 }
 
-func (t *T) Close() {
+func (t *T) killServer() {
 	if err := t.servProc.Process.Signal(os.Interrupt); err != nil {
 		t.Log("Unable to kill the server process:", err)
 		os.RemoveAll(t.testDir)
@@ -299,13 +365,10 @@ func (t *T) Close() {
 		t.Log("Failed to remove test directory:", err)
 	}
 	t.testDir = ""
+}
 
-	// Shutdown the DNS server after maddy to make sure it will not spend time
-	// timing out queries.
-	if err := t.dnsServ.Close(); err != nil {
-		t.Log("Unable to stop the DNS server:", err)
-	}
-	t.dnsServ = nil
+func (t *T) Close() {
+	t.Log("close is no-op")
 }
 
 // Printf implements Logger interfaces used by some libraries.