As spotted in #162 by a contributor, if the context is done before the
Selector.run caller has received from the channel, the spawned goroutine
may leak if blocked on a send.
Turns out that these subtests are the only pair which cannot run in
parallel with each other. Undo that change and add a TODO. This should
fix the CI failures.
While at it, remove an unnecessary testAllocate line.
We know we're going to see an error, so don't log it too:
$ go test -run TestAllocatePortInUse
2019/02/21 17:11:34 ERROR: pool could not allocate runner ...
PASS
ok github.com/chromedp/chromedp 0.004s
The subtests were almost all marked as parallel, but that's not enough.
That only makes the subtests run in parallel with other subtests within
the same tests, not with any other test.
Since none of the tests make use of globals nor require the entire
program to themselves, properly run all the tests in parallel.
Speeds up 'go test' on my 8-core laptop from an average of ~130s to an
average of ~50s. Many tests hit timeouts and have sleeps, so we want to
avoid running those sequentially whenever possible.
It supports alternative names for Chrome such as chromium, as well as
extra names to look for like headless-shell.
Also swap the os.Getenv logic, so that we only do the exec.LookPath work
if the env var is unset.
tip is rather unstable, so we shouldn't block PRs if it happens to break
our build or tests.
While at it, run 'go mod tidy' with the latest tip version.
Before the fix, the added test would give a Pool.Allocate error like:
pool could not connect to 9000: timeout waiting for initial target
The actual underlying error, which can only be seen if one inspects
chrome's stderr, is that it failed to bind to the debugging protocol
port if it was already in use.
This is of course an issue with the environment that chromedp is being
run under, since it was given a port range that wasn't available.
However, the confusing error can lead to developers wasting their time
instead of spotting the error quickly.
Unfortunately, there doesn't seem to be a way to have Chrome exit
immediately if it can't bind to the given port. So, instead of relying
on it, check if the current process can bind to the port first.
Add a test too, where we grab the first port in the pool range, and
check that we get an error that's not confusing.
Fixes#253.
There's no need to put the error variables in a larger scope, nor define
them earlier than necessary. If anything, it makes the code harder to
follow, such as figuring out when nil errors are returned.
