testscript: phase out func() int in RunMain

cmd/testscript/main.go

@@ -41,37 +41,28 @@ func (e *envVarsFlag) Set(v string) error {
 }
 
 func main() {
-	os.Exit(main1())
-}
-
-func main1() int {
 	switch err := mainerr(); err {
 	case nil:
-		return 0
-	case flag.ErrHelp:
-		return 2
 	default:
 		fmt.Fprintln(os.Stderr, err)
-		return 1
+		os.Exit(1)
 	}
 }
 
 func mainerr() (retErr error) {
-	fs := flag.NewFlagSet(os.Args[0], flag.ContinueOnError)
-	fs.Usage = func() {
+	flag.Usage = func() {
 		mainUsage(os.Stderr)
+		os.Exit(2)
 	}
 	var envVars envVarsFlag
-	fUpdate := fs.Bool("u", false, "update archive file if a cmp fails")
-	fWork := fs.Bool("work", false, "print temporary work directory and do not remove when done")
-	fContinue := fs.Bool("continue", false, "continue running the script if an error occurs")
-	fVerbose := fs.Bool("v", false, "run tests verbosely")
-	fs.Var(&envVars, "e", "pass through environment variable to script (can appear multiple times)")
-	if err := fs.Parse(os.Args[1:]); err != nil {
-		return err
-	}
-
-	files := fs.Args()
+	fUpdate := flag.Bool("u", false, "update archive file if a cmp fails")
+	fWork := flag.Bool("work", false, "print temporary work directory and do not remove when done")
+	fContinue := flag.Bool("continue", false, "continue running the script if an error occurs")
+	fVerbose := flag.Bool("v", false, "run tests verbosely")
+	flag.Var(&envVars, "e", "pass through environment variable to script (can appear multiple times)")
+	flag.Parse()
+
+	files := flag.Args()
 	if len(files) == 0 {
 		files = []string{"-"}
 	}

cmd/testscript/main_test.go

@@ -6,7 +6,6 @@ package main
 
 import (
 	"bytes"
-	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
@@ -19,9 +18,9 @@ import (
 )
 
 func TestMain(m *testing.M) {
-	os.Exit(testscript.RunMain(m, map[string]func() int{
-		"testscript": main1,
-	}))
+	testscript.Main(m, map[string]func(){
+		"testscript": main,
+	})
 }
 
 func TestScripts(t *testing.T) {

cmd/txtar-addmod/addmod.go

@@ -62,13 +62,9 @@ func fatalf(format string, args ...interface{}) {
 
 const goCmd = "go"
 
-func main() {
-	os.Exit(main1())
-}
-
 var allFiles = flag.Bool("all", false, "include all source files")
 
-func main1() int {
+func main() {
 	flag.Usage = usage
 	flag.Parse()
 	if flag.NArg() < 2 {
@@ -211,5 +207,5 @@ func main1() int {
 		}
 	}
 	os.RemoveAll(tmpdir)
-	return exitCode
+	os.Exit(exitCode)
 }

cmd/txtar-addmod/script_test.go

@@ -17,9 +17,9 @@ import (
 var proxyURL string
 
 func TestMain(m *testing.M) {
-	os.Exit(testscript.RunMain(gobinMain{m}, map[string]func() int{
-		"txtar-addmod": main1,
-	}))
+	testscript.Main(gobinMain{m}, map[string]func(){
+		"txtar-addmod": main,
+	})
 }
 
 type gobinMain struct {

cmd/txtar-c/savedir.go

@@ -14,7 +14,7 @@ package main
 
 import (
 	"bytes"
-	stdflag "flag"
+	"flag"
 	"fmt"
 	"log"
 	"os"
@@ -25,11 +25,10 @@ import (
 	"github.com/rogpeppe/go-internal/txtar"
 )
 
-var flag = stdflag.NewFlagSet(os.Args[0], stdflag.ContinueOnError)
-
 func usage() {
 	fmt.Fprintf(os.Stderr, "usage: txtar-c dir >saved.txtar\n")
 	flag.PrintDefaults()
+	os.Exit(2)
 }
 
 var (
@@ -38,17 +37,10 @@ var (
 )
 
 func main() {
-	os.Exit(main1())
-}
-
-func main1() int {
 	flag.Usage = usage
-	if flag.Parse(os.Args[1:]) != nil {
-		return 2
-	}
+	flag.Parse()
 	if flag.NArg() != 1 {
 		usage()
-		return 2
 	}
 
 	log.SetPrefix("txtar-c: ")
@@ -111,6 +103,4 @@ func main1() int {
 
 	data := txtar.Format(a)
 	os.Stdout.Write(data)
-
-	return 0
 }

cmd/txtar-c/script_test.go

@@ -5,16 +5,15 @@
 package main
 
 import (
-	"os"
 	"testing"
 
 	"github.com/rogpeppe/go-internal/testscript"
 )
 
 func TestMain(m *testing.M) {
-	os.Exit(testscript.RunMain(m, map[string]func() int{
-		"txtar-c": main1,
-	}))
+	testscript.Main(m, map[string]func(){
+		"txtar-c": main,
+	})
 }
 
 func TestScripts(t *testing.T) {

cmd/txtar-x/extract.go

@@ -29,18 +29,14 @@ var (
 func usage() {
 	fmt.Fprintf(os.Stderr, "usage: txtar-x [flags] [file]\n")
 	flag.PrintDefaults()
+	os.Exit(2)
 }
 
 func main() {
-	os.Exit(main1())
-}
-
-func main1() int {
 	flag.Usage = usage
 	flag.Parse()
 	if flag.NArg() > 1 {
 		usage()
-		return 2
 	}
 	log.SetPrefix("txtar-x: ")
 	log.SetFlags(0)
@@ -50,20 +46,19 @@ func main1() int {
 		data, err := io.ReadAll(os.Stdin)
 		if err != nil {
 			log.Printf("cannot read stdin: %v", err)
-			return 1
+			os.Exit(1)
 		}
 		a = txtar.Parse(data)
 	} else {
 		a1, err := txtar.ParseFile(flag.Arg(0))
 		if err != nil {
 			log.Print(err)
-			return 1
+			os.Exit(1)
 		}
 		a = a1
 	}
 	if err := txtar.Write(a, *extractDir); err != nil {
 		log.Print(err)
-		return 1
+		os.Exit(1)
 	}
-	return 0
 }

cmd/txtar-x/extract_test.go

@@ -13,9 +13,9 @@ import (
 )
 
 func TestMain(m *testing.M) {
-	os.Exit(testscript.RunMain(m, map[string]func() int{
-		"txtar-x": main1,
-	}))
+	testscript.Main(m, map[string]func(){
+		"txtar-x": main,
+	})
 }
 
 func TestScripts(t *testing.T) {

testscript/doc.go

@@ -24,14 +24,14 @@ To run a specific script foo.txtar or foo.txt, run
 where TestName is the name of the test that Run is called from.
 
 To define an executable command (or several) that can be run as part of the script,
-call RunMain with the functions that implement the command's functionality.
+call Main with the functions that implement the command's functionality.
 The command functions will be called in a separate process, so are
 free to mutate global variables without polluting the top level test binary.
 
 	func TestMain(m *testing.M) {
-		os.Exit(testscript.RunMain(m, map[string] func() int{
+		testscript.Main(m, map[string] func() {
 			"testscript": testscriptMain,
-		}))
+		})
 	}
 
 In general script files should have short names: a few words, not whole sentences.

testscript/exe.go

@@ -23,13 +23,11 @@ type TestingM interface {
 // Deprecated: this option is no longer used.
 func IgnoreMissedCoverage() {}
 
-// RunMain should be called within a TestMain function to allow
+// Main should be called within a TestMain function to allow
 // subcommands to be run in the testscript context.
 //
 // The commands map holds the set of command names, each
-// with an associated run function which should return the
-// code to pass to os.Exit. It's OK for a command function to
-// exit itself, but this may result in loss of coverage information.
+// with an associated run function which may call os.Exit.
 //
 // When Run is called, these commands are installed as regular commands in the shell
 // path, so can be invoked with "exec" or via any other command (for example a shell script).
@@ -38,9 +36,7 @@ func IgnoreMissedCoverage() {}
 // without "exec" - that is, "foo" will behave like "exec foo".
 // This can be disabled with Params.RequireExplicitExec to keep consistency
 // across test scripts, and to keep separate process executions explicit.
-//
-// This function returns an exit code to pass to os.Exit, after calling m.Run.
-func RunMain(m TestingM, commands map[string]func() int) (exitCode int) {
+func Main(m TestingM, commands map[string]func()) {
 	// Depending on os.Args[0], this is either the top-level execution of
 	// the test binary by "go test", or the execution of one of the provided
 	// commands via "foo" or "exec foo".
@@ -58,18 +54,18 @@ func RunMain(m TestingM, commands map[string]func() int) (exitCode int) {
 		tmpdir, err := os.MkdirTemp("", "testscript-main")
 		if err != nil {
 			log.Printf("could not set up temporary directory: %v", err)
-			return 2
+			os.Exit(2)
 		}
 		defer func() {
 			if err := os.RemoveAll(tmpdir); err != nil {
 				log.Printf("cannot delete temporary directory: %v", err)
-				exitCode = 2
+				os.Exit(2)
 			}
 		}()
 		bindir := filepath.Join(tmpdir, "bin")
 		if err := os.MkdirAll(bindir, 0o777); err != nil {
 			log.Printf("could not set up PATH binary directory: %v", err)
-			return 2
+			os.Exit(2)
 		}
 		os.Setenv("PATH", bindir+string(filepath.ListSeparator)+os.Getenv("PATH"))
 
@@ -86,7 +82,7 @@ func RunMain(m TestingM, commands map[string]func() int) (exitCode int) {
 			}
 			if err != nil {
 				log.Printf("could not set up %s in $PATH: %v", name, err)
-				return 2
+				os.Exit(2)
 			}
 			scriptCmds[name] = func(ts *TestScript, neg bool, args []string) {
 				if ts.params.RequireExplicitExec {
@@ -95,11 +91,26 @@ func RunMain(m TestingM, commands map[string]func() int) (exitCode int) {
 				ts.cmdExec(neg, append([]string{name}, args...))
 			}
 		}
-		return m.Run()
+		os.Exit(m.Run())
 	}
 	// The command being registered is being invoked, so run it, then exit.
 	os.Args[0] = cmdName
-	return mainf()
+	mainf()
+	os.Exit(0)
+}
+
+// Deprecated: use [Main], as the only reason for returning exit codes
+// was to collect full code coverage, which Go does automatically now:
+// https://go.dev/blog/integration-test-coverage
+func RunMain(m TestingM, commands map[string]func() int) (exitCode int) {
+	commands2 := make(map[string]func(), len(commands))
+	for name, fn := range commands {
+		commands2[name] = func() { os.Exit(fn()) }
+	}
+	Main(m, commands2)
+	// Main always calls os.Exit; we assume that all users of RunMain would have simply
+	// called os.Exit with the returned exitCode as well, following the documentation.
+	panic("unreachable")
 }
 
 // copyBinary makes a copy of a binary to a new location. It is used as part of

testscript/testscript.go

@@ -181,7 +181,7 @@ type Params struct {
 	// script.
 	UpdateScripts bool
 
-	// RequireExplicitExec requires that commands passed to RunMain must be used
+	// RequireExplicitExec requires that commands passed to [Main] must be used
 	// in test scripts via `exec cmd` and not simply `cmd`. This can help keep
 	// consistency across test scripts as well as keep separate process
 	// executions explicit.