using -- to signify end of options
FrankReh opened this issue · 8 comments
I've found supporting -- as an end of options argument useful in getting the remaining arguments passed to the wasm binary.
Perhaps you would consider implementing this too or looking at a PR I could provide. It is not quite as straight forward as letting the flag.Parse catch the --, but close.
Yes, that's definitely an option although it slightly breaks the clean abstraction of just running "go test". So far, the only config knob that I have added is WASM_HEADLESS and deliberately steered clear of adding any additional knobs to keep things minimal. What additional options did you have in mind to add?
"go test" allows arguments it doesn't recognize and it does recognize "--". It doesn't prefix the ones it doesn't recognize with "test." nor does it prefix any it might recognize after the "--".
When one sends the test a bad argument, it is not go that is complaining, it is the binary. I often pass my binaries under test extra arguments - some in the form of flags, others in the form of plain arguments that are picked up from os.Args or flag.Args().
"go test" allows "--" but doesn't require it. (It is a way to get -v passed to your binary if you want it.)
func TestSample(t *testing.T) {
t.Logf("os.Args looks like %s", os.Args)
t.Logf("flag.Args() looks like %s", flag.Args())
}$ go test -v -- -x -v
=== RUN TestSample
map_test.go:126: os.Args looks like [/var/folders/2t/sdgt1zts68s4n9_f1tx670pw0000gn/T/go-build2924304216/b001/play.test -test.paniconexit0 -test.timeout=10m0s -test.v=true -- -x -v]
map_test.go:127: flag.Args() looks like [-x -v]
--- PASS: TestSample (0.00s)
PASS
ok testingroot/play 0.206swasmbrowsertest could be an even better abstraction of running "go test", making it more minimal, if it behaved the same way.
My approach suggested above was my attempt at getting the biggest bang for the buck, support extra arguments but require
the use of "--". But there is a better way, that would make it even more like "go test".
Don't use flag.Parse, so don't even use the flag package. Parse the os.Args, pull out -cpuprofile, and if it doesn't come with an "=" sign, pull out the next argument and treat it as the filename. Then the flags.go file can go away.
And since you asked, I have added two other options for my own testing framework, one for specifying a different path for the JavaScript glue file and one for avoiding the close of the window when not running headless. With a small tweak, my version can still be a replacement for go_js_wasm_exec and also a replacement for node. In my environment, the JavaScript glue file comes from different places, as I'm often making changes to it - and I'm using it against Tinygo JavaScript glue files too so being able to specify the filename on the command line was important.
A bit long winded. Sorry. Let me say this package is super useful. I had found it years ago and then forgotten about it because I wasn't using wasm day to day. Being able to use it in place of node for automating wasm tests has already shown me a useful stack dump that node was unable to generate - also being able to enter the window's console and look at the go instance is useful. And having such a test environment where the DOM and worker threads are available to run wasm as well will be very handy. If you don't want to change something you don't consider broken for your use cases, I wouldn't blame you. It is already super if you keep up with the chromedp standards as they and chrome evolve.
Actually, I dug into the flag package and found a ContinueOnError mode and with a quick loop, the flag package lets us have our cake and eat it too. This tool can define only the flags it wants and there's a way to pass the rest on to the next step.
// Setup the flag.CommandLine so that we catch the arguments it doesn't recognize
// and pass them on to the wasm image.
flag.CommandLine.Init(os.Args[0], flag.ContinueOnError)
flag.CommandLine.SetOutput(ioutil.Discard)
passon := []string{os.Args[1]} // The wasm image name
next := os.Args[2:] // Where to have Parse start
for len(next) > 0 {
if next[0] == "--" {
passon = append(passon, next...) // include the "--" for the wasm image, it's what "go test" does.
break
}
err := flag.CommandLine.Parse(next)
if err == nil {
passon = append(passon, flag.Args()...)
break
}
if strings.HasPrefix(err.Error(), "flag provided but not defined: ") {
passon = append(passon, next[0])
next = next[1:]
continue
}
w := os.Stderr
fmt.Fprintf(w, "%s\n", err)
flag.CommandLine.SetOutput(w)
flag.Usage()
os.Exit(1)
}It works for me.
Ok, this sounds interesting. Getting rid of flags.go would be nice. But coming to the flags, you mentioned
And since you asked, I have added two other options for my own testing framework, one for specifying a different path for the JavaScript glue file and one for avoiding the close of the window when not running headless.
Were you suggesting these options to be added to wasmbrowsertest itself?
I have added them to my version of wasmbrowsertest. I'm likely to continue to make mods to my copy as I use it more and more. I mentioned them because they could be useful to others so I could offer them as a second PR if you wanted to take a look but I could understand if you wanted to keep it as knob-free as possible for purely a go_js_wasm_exec replacement. The hack I use for determining whether the command has one filename arg as go_js_wasm_exec is given, or two, as node is given, is whether the first argument ends in ".js".
And I didn't say it yet, but that this project can do cpu profiling within Chrome and then massage the results to conform to the go pprof format is amazing. So much more enlightening than node for my use cases. I started using that aspect yesterday as well.
but I could understand if you wanted to keep it as knob-free as possible for purely a go_js_wasm_exec replacement.
Yeah, I'd like to keep it minimal. But I am interested your solution to remove flags.go. If you are interested in sending a PR for that, that'll be very appreciated!
And I didn't say it yet, but that this project can do cpu profiling within Chrome and then massage the results to conform to the go pprof format is amazing.
Thanks! There might be some small bugs in that, so please do feel free to let me know if you find any.