Close #18: allow additional arguments for chrome

[oneeman]

Resolves https://github.com/meteortesting/meteor-browser-tests/issues/18 .

It's a little more complicated than I would have liked because I figured there should be a way to specify multiple arguments, and the way that spawning processes from node works it's not equivalent to have, for example --no-sandbox --version as a single argument versus having --no-sandbox and --version as separate arguments.

I decided to split on whitespace, and support a way for users to escape spaces in the unlikely case that someone actually needs a space inside of a single argument. Open to suggestions for other ways to do it (or just to do less).

[SimonSimCity]

Just because %20 is typical for URL encoding, but foreign on the commandline - do you know if there's any way of escaping the space you want to use in the command? This would be more command-line like ... Or if the system internally not implodes the values by a space anyways - in which case we wouldn't even need to take this step 😅

[oneeman]

The system doesn't implode them. I verified it by comparing

child_process.spawn("ping", ["-c 2", "8.8.8.8"], {stdio: "inherit"})

and

child_process.spawn("ping", ["-c 2 8.8.8.8"], {stdio: "inherit"})

The first works and the second doesn't.

Typically, if you need to include spaces in a command-line argument, you either (a) wrap the argument in quotes or (b) escape them with a slash. The shell then takes care of parsing that for you, and the program itself doesn't have to do anything. That would certainly be the ideal behavior.

To comprehend wrapping in quotes, we'd need to either require another library (e.g. some csv parsing library) or copy/paste some code to do it, both of which I was hesitant about. But if I get approval for including some csv library that should make for a clean and robust solution.

To comprehend slash-escaping, we'd need to parse it ourselves. We could do something like /\\ /g, to unescape just \. It's not very robust (it would get wrong if we had for example foo\\ bar, where what's being escaped is the slash itself), though, and doing something more robust would make it less clean.

Another alternative would be to support JSON, e.g. TEST_CHROME_ARGS=["--foo", "--bar"], because then we could rely on JSON.parse. This seems like a decent solution.

[SimonSimCity]

Another way would be to replace every occurrence of %25 by % after replacing every occurrence of %20 by a space. This would be a partial version of decodeURIComponent but it would at least work out ...

But I like the JSON approach you have 😃It might also require some escaping, but it's at least more developer-friendly, or what is your opinion on this? I would like to have it as natural to use as possible ...

[oneeman]

Yeah, I think JSON would be fine. Some people might not like it, but nobody would really be hindered by it. I'll update the PR.

[oneeman]

Actually, I don't know. Exporting JSON is pretty annoying because strings need to be in quotes and the whole things needs to be in quotes because of the brackets.

export TEST_CHROME_ARGS='["--foo", "--bar"]'

I'm up for whatever, but none of the options seem very good.

Really, wanting to have a space in command-line arguments to chrome for your automated tests is an extreme edge case, so maybe it's not so bad to have that involve jumping through some hoops such as unintuitive escaping like %20.

[SimonSimCity]

Ok, let's leave the %20 in here and take it as is. I guess it's the most clean solution we can get ... at least it feels as ugly as the rest 😅