replace flag.Parse with gentleParse (#34)

* replace flag.Parse with gentleParse

gentleParse does away with having to call out all the arguments that
will be passed to the wasm image.

Now the wasm image will be passed all the arguments that the tool
doesn't recognize itself.

Also there are unit test examples of the parsing.

* fix parsing's use of flag

The flag package's global obscurred the fact the last commit had mixed
instances of the FlagSet being used in the loop along with the
flag.CommandLine global. And when using it with the CommandLine FlagSet,
it may not matter. Only if this function is used somewhere else with a
different FlagSet would this problem have surfaced.

* Fix and cleanup gentleParse

Fixes the case of an unrecognized argument using "=".

Also fixes case where flag.Parse ate the "--" because it had recognized everything up
to that point; we want the "--" passed back to the caller.

Adds some tests for the two aspects just fixed.

Moved gentleParse to parse.go.

Renamed examples in parse_test.go to contain "Parse" so test runs
like 'go test -run Parse -v' are possible to hit just those tests.

* Cleanup parse_test.go

The unit tests for gentleParse are converted from Example functions
to a *testing.T framework where a list of args and a list of expected
result lines are given. They are still kept as separate tests to allow
running some subset when wanting to debug something.

main.go

@@ -24,6 +24,10 @@ import (
 	"github.com/chromedp/chromedp"
 )
 
+var (
+	cpuProfile *string
+)
+
 func main() {
 	// NOTE: Since `os.Exit` will cause the process to exit, this defer
 	// must be at the bottom of the defer stack to allow all other defer calls to
@@ -40,7 +44,7 @@ func main() {
 		logger.Fatal("Please pass a wasm file as a parameter")
 	}
 
-	initFlags()
+	cpuProfile = flag.String("test.cpuprofile", "", "")
 
 	wasmFile := os.Args[1]
 	ext := path.Ext(wasmFile)
@@ -54,13 +58,9 @@ func main() {
 		defer os.Remove(wasmFile)
 		os.Args[1] = wasmFile
 	}
-	// We create a copy of the args to pass to NewWASMServer, because flag.Parse needs the
-	// 2nd argument (the binary name) removed before being called.
-	// This is an effect of "go test" passing all the arguments _after_ the binary name.
-	argsCopy := append([]string(nil), os.Args...)
-	// Remove the 2nd argument.
-	os.Args = append(os.Args[:1], os.Args[2:]...)
-	flag.Parse()
+
+	passon := gentleParse(flag.CommandLine, os.Args[2:])
+	passon = append([]string{wasmFile}, passon...)
 
 	// Need to generate a random port every time for tests in parallel to run.
 	l, err := net.Listen("tcp", "localhost:")
@@ -77,7 +77,7 @@ func main() {
 	}
 
 	// Setup web server.
-	handler, err := NewWASMServer(wasmFile, filterCPUProfile(argsCopy[1:]), logger)
+	handler, err := NewWASMServer(wasmFile, passon, logger)
 	if err != nil {
 		logger.Fatal(err)
 	}
@@ -171,21 +171,6 @@ func main() {
 	<-done
 }
 
-// filterCPUProfile removes the cpuprofile argument if passed.
-// CPUProfile is taken from the chromedp driver.
-// So it is valid to pass such an argument, but the wasm binary will throw an error
-// since file i/o is not supported inside the browser.
-func filterCPUProfile(args []string) []string {
-	tmp := args[:0]
-	for _, x := range args {
-		if strings.Contains(x, "test.cpuprofile") {
-			continue
-		}
-		tmp = append(tmp, x)
-	}
-	return tmp
-}
-
 func copyFile(src, dst string) error {
 	srdFd, err := os.Open(src)
 	if err != nil {

parse.go

@@ -0,0 +1,67 @@
+package main
+
+import (
+	"flag"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"strings"
+)
+
+// gentleParse takes a flag.FlagSet, calls Parse to get its flags parsed,
+// and collects the arguments the FlagSet does not recognize, returning
+// the collected list.
+func gentleParse(flagset *flag.FlagSet, args []string) []string {
+	if len(args) == 0 {
+		return nil
+	}
+	const prefix = "flag provided but not defined: "
+
+	r := make([]string, 0, len(args))
+
+	flagset.Init(flagset.Name(), flag.ContinueOnError)
+	w := flagset.Output()
+	flagset.SetOutput(ioutil.Discard)
+
+	// Put back the flagset's output, the flagset's Usage might be called later.
+	defer flagset.SetOutput(w)
+
+	next := args
+
+	for len(next) > 0 {
+		if !strings.HasPrefix(next[0], "-") {
+			r, next = append(r, next[0]), next[1:]
+			continue
+		}
+		if err := flagset.Parse(next); err != nil {
+			if strings.HasPrefix(err.Error(), prefix) {
+				pull := strings.TrimPrefix(err.Error(), prefix)
+				for next[0] != pull && !(strings.HasPrefix(next[0], pull) && strings.HasPrefix(next[0], pull+"=")) {
+					next = next[1:]
+					if len(next) == 0 {
+						panic("odd: pull not found: " + pull)
+					}
+				}
+				r, next = append(r, next[0]), next[1:]
+				continue
+			}
+			fmt.Fprintf(w, "%s\n", err)
+			flagset.SetOutput(w)
+			flagset.Usage()
+			os.Exit(1)
+		}
+
+		// Check if the call to flagset.Parse ate a "--". If so, we're done
+		// and can return what's been built up on r along with the rest.
+		if len(next) > len(flagset.Args()) {
+			lastabsorbedpos := len(next) - len(flagset.Args()) - 1
+			lastabsorbed := next[lastabsorbedpos]
+			if lastabsorbed == "--" {
+				r = append(r, "--") // return the "--" too.
+				return append(r, flagset.Args()...)
+			}
+		}
+		next = flagset.Args()
+	}
+	return r
+}

parse_test.go

@@ -0,0 +1,222 @@
+package main
+
+import (
+	"flag"
+	"fmt"
+	"os"
+	"strings"
+	"testing"
+)
+
+// The formst of these tests for the gentleParse function
+// are generally that the args are presented to the testParse function
+// and the expected results are presented as separate lines to the
+// Expect result.
+
+func TestParseEmpty(t *testing.T) {
+	// Empty in, empty out.
+
+	testParse().Expect(t,
+		`cpuProfile: ""`,
+		`passon    : []`,
+	)
+}
+
+func TestParseOther(t *testing.T) {
+	// Empty in, empty out, with an extra `other` variable that has a default.
+
+	testParseOther().Expect(t,
+		`cpuProfile: ""`,
+		`other     : "default-other-value"`,
+		`passon    : []`,
+	)
+}
+
+func TestParseVerbose(t *testing.T) {
+	// One unrecognized in, same out.
+
+	testParse("-test.v").Expect(t,
+		`cpuProfile: ""`,
+		`passon    : ["-test.v"]`,
+	)
+}
+
+func TestParseCpu1(t *testing.T) {
+	// One unrecognized followed by ours.
+
+	testParse("-test.v", "-test.cpuprofile", "cpu1.out").Expect(t,
+		`cpuProfile: "cpu1.out"`,
+		`passon    : ["-test.v"]`,
+	)
+}
+
+func TestParseCpu2(t *testing.T) {
+	// Ours followed by one unrecognized.
+
+	testParse("-test.cpuprofile", "cpu2.out", "-test.v").Expect(t,
+		`cpuProfile: "cpu2.out"`,
+		`passon    : ["-test.v"]`,
+	)
+}
+
+func TestParseEqualCpu3(t *testing.T) {
+	// Ours followed by one unrecognized that uses "=".
+
+	testParse("-test.cpuprofile", "cpu3.out", "-test.v=true").Expect(t,
+		`cpuProfile: "cpu3.out"`,
+		`passon    : ["-test.v=true"]`,
+	)
+}
+
+func TestParseEqualCpu4(t *testing.T) {
+	// Swapping order from Cpu3 test, the unrecognized first, followed by ours.
+
+	testParse("-test.v=true", "-test.cpuprofile", "cpu4.out").Expect(t,
+		`cpuProfile: "cpu4.out"`,
+		`passon    : ["-test.v=true"]`,
+	)
+}
+
+func TestParseExtraBool1(t *testing.T) {
+	// Ours followed by two unrecognized.
+
+	testParse("-test.cpuprofile", "cpu.out", "-test.v", "-bool").Expect(t,
+		`cpuProfile: "cpu.out"`,
+		`passon    : ["-test.v" "-bool"]`,
+	)
+}
+
+func TestParseExtraBool2(t *testing.T) {
+	// Ours between two unrecognized.
+
+	testParse("-bool", "-test.cpuprofile", "cpu.out", "-test.v").Expect(t,
+		`cpuProfile: "cpu.out"`,
+		`passon    : ["-bool" "-test.v"]`,
+	)
+}
+
+func TestParseExtraStringNoDDash1(t *testing.T) {
+	// Ours pulled out from front.
+
+	testParse("-test.cpuprofile", "cpu.out", "-test.v", "-bool", "-string", "last").Expect(t,
+		`cpuProfile: "cpu.out"`,
+		`passon    : ["-test.v" "-bool" "-string" "last"]`,
+	)
+}
+
+func TestParseExtraStringNoDDash2(t *testing.T) {
+	// Ours pulled out from middle.
+
+	testParse("-string", "first", "-test.cpuprofile", "cpu.out", "-test.v", "-bool").Expect(t,
+		`cpuProfile: "cpu.out"`,
+		`passon    : ["-string" "first" "-test.v" "-bool"]`,
+	)
+}
+
+func TestParseDDash1ExtraString(t *testing.T) {
+	// Ours pulled out from front and the -- appears afterwards.
+
+	testParse("-test.cpuprofile", "cpu.out", "-test.v", "--", "-bool", "-string", "abc").Expect(t,
+		`cpuProfile: "cpu.out"`,
+		`passon    : ["-test.v" "--" "-bool" "-string" "abc"]`,
+	)
+}
+
+func TestParseDDash2ExtraString(t *testing.T) {
+	// Ours pulled out from front and the -- appears afterwards.
+
+	testParse("-test.cpuprofile", "cpu.out", "--", "-test.v", "-bool", "-string", "abc").Expect(t,
+		`cpuProfile: "cpu.out"`,
+		`passon    : ["--" "-test.v" "-bool" "-string" "abc"]`,
+	)
+}
+
+func TestParseDDash3UnprocessedProfile(t *testing.T) {
+	// Ours *not* pulled out because it appears after a --, just as "go test" would handle it.
+
+	testParse("--", "-test.cpuprofile", "cpu.other", "-test.v", "-bool", "-string", "abc").Expect(t,
+		`cpuProfile: ""`,
+		`passon    : ["--" "-test.cpuprofile" "cpu.other" "-test.v" "-bool" "-string" "abc"]`,
+	)
+}
+
+type testParseGot struct {
+	got []string
+}
+
+func makeParseGot(lines ...string) testParseGot {
+	return testParseGot{got: lines}
+}
+
+func (g testParseGot) failure(expect []string, format string, args ...interface{}) string {
+	buf := new(strings.Builder)
+	fmt.Fprintf(buf, format+"\n", args...)
+	fmt.Fprintf(buf, "  Got:\n")
+	for i := range g.got {
+		fmt.Fprintf(buf, "    %s\n", g.got[i])
+	}
+	fmt.Fprintf(buf, "  Expected:\n")
+	for i := range expect {
+		fmt.Fprintf(buf, "    %s\n", expect[i])
+	}
+	return buf.String()
+}
+
+func (g testParseGot) Expect(t testing.TB, expect ...string) {
+	if len(g.got) != len(expect) {
+		t.Helper()
+		t.Errorf("%s",
+			g.failure(expect, "got %d lines, expected %d", len(g.got), len(expect)))
+		return
+	}
+	for i := range g.got {
+		if g.got[i] != expect[i] {
+			t.Helper()
+			t.Errorf("%s",
+				g.failure(expect, "at least line %d of got and expected don't match", i+1))
+			return
+		}
+	}
+}
+
+func testParse(args ...string) testParseGot {
+	var (
+		cpuProfile string
+	)
+
+	flagset := flag.NewFlagSet("binname", flag.ExitOnError)
+	flagset.SetOutput(os.Stdout) // For Examples to catch as output.
+
+	flagset.StringVar(&cpuProfile, "test.cpuprofile", "", "")
+
+	passon := gentleParse(flagset, args)
+
+	return makeParseGot(
+		fmt.Sprintf("cpuProfile: %q", cpuProfile),
+		fmt.Sprintf("passon    : %q", passon),
+	)
+}
+
+// This one acts more like an example of how to perform a different type of test.
+// It was perhaps useful in early stages of building unit tests but then seems
+// to have gone unused except for the default, empty, case.
+func testParseOther(args ...string) testParseGot {
+	var (
+		cpuProfile string
+		other      string
+	)
+
+	flagset := flag.NewFlagSet("binname", flag.ExitOnError)
+	flagset.SetOutput(os.Stdout) // For Examples to catch as output.
+
+	flagset.StringVar(&cpuProfile, "test.cpuprofile", "", "")
+	flagset.StringVar(&other, "other", "default-other-value", "")
+
+	passon := gentleParse(flagset, args)
+
+	return makeParseGot(
+		fmt.Sprintf("cpuProfile: %q", cpuProfile),
+		fmt.Sprintf("other     : %q", other),
+		fmt.Sprintf("passon    : %q", passon),
+	)
+}