We were doing this for extra open tabs, since NewContext was taking care
of detaching and closing the respective target.
And cancelling an entire allocator also properly waited for all the
resources, such as processes and temporary directories, to be cleaned
up.
However, this didn't work for a browser's first context; cancelling it
should wait for that one browser's resources to be cleaned up, but that
wasn't implemented. Do that, fixing the TODO in TestExecAllocator.
It's Run that actually starts a Browser, not NewContext. If the browser
is closed or crashes, the browser handler will fail to read from the
websocket, and its goroutines will stop.
However, the target handler's goroutines may not stop. The browser
handler uses a separate cancel function to stop itself when encountering
a websocket error, so that doesn't propagate to the original context
children, like the target handler.
To fix this, make it so that NewContext can keep the cancel function
around, for Run to use it in this scenario. And add a test case that
tests this very edge case, which used to time out before the fix.
Fixes#289.
For all contexts except the first browser context, as in that case the
allocator and browser handler already take care of shutting down the
process and goroutines, respectively.
Fixes#293.
Chrome already starts with a blank page, so use that for the first
target context instead of creating a new tab.
Add the first version of the Targets API, which is useful to test this
feature.
Fixes#291.
This way, the simple examples and tests don't need to do that
separately. Practically all users will want this cleanup work to be
synchronous, and practically all Go APIs are synchronous by default, so
this makes chromedp easier to use.
"test" as part of the name is redundant, and spaces aren't recommended.
Add a padding for two digits, so that tests with more than a handful of
subtests still print in a nice way.
Many consecutive calls to Run can be collapsed into a single call. While
at it, make the error handling style more consistent. Overall removes 70
lines of repetitive code.
This fixes the data race uncovered by the recent refactor to run all
tests as tabs under the same browser.
The problem was that a write on the pages map could be done from the
goroutine calling NewContext to create a new map, while other goroutines
could similarly read or write the same map.
Instead of adding a lock around the map, make one of the Browser's
goroutines be the sole user of the map. To make that extra obvious and
avoid potential races in the future, declare the map inside the
goroutine's scope.
For some reason, this makes the Attributes tests flakier than before.
For now, add short sleeps; we can investigate that separately, now that
the data races are gone.
The reason that t.Parallel broke the tests was because the parent test
must finish before the parallel subtests can start. So, we'd be closing
the httptest server and removing the tmpfile before any of the parallel
subtests even began.
We could refactor this to make the subtests parallel, but they're only
two, and the parent is already parallel, so it's not worth the effort.
See the added comment.
Remove the log option lines from testAllocate; right now, we don't have
these options for Target, and Target doesn't log much anyway. We can
always revisit this in the future.
While at it, simplify some code.
To fix a potential temporary goroutine leak; see the added comment. The
leaked goroutine would eventually stop once the timer is fired, but
until then, the goroutine could be unnecessarily left around.
This can simplify some common use cases, like running a few actions
directly, or running no actions at all. It's also an almost entirely
backwards compatible change, as all Run call sites should continue to
compile and work.
Leave Tasks, as it can still be useful for functions to return complex
sequences of actions as a single Action.
t.Parallel effectively fires off a goroutine, so we can't use the test
range variable directly. That can result in different subtests using the
same test case data, causing sporadic failures, or some test cases
rarely being actually tested.
This was uncovered while stress-testing the test suite for an unrelated
refactor.
While at it, one test case in TestMouseClickNode was incorrect.
contextmenu fires on a right click, so ModifierNone won't fire it. This
wasn't caught before, as this test case was almost never ran. After the
data race fix, the test case failed consistently, before being fixed.
That way, we avoid the racy map access via Browser.executorForTarget. If
a context is attached to a target, the Target field must be non-nil.
The Browser.pages map is still racy, since multiple tabs can be created
concurrently; we'll fix this other data race in another commit.
This vastly speeds up 'go test' on my laptop from ~10s to ~3s, as we
save a lot of time spinning up new Chrome browser processes.
In practice, each tab is a separate process anyway, but there's a lot of
added overhead if we're firing up the entire browser, particularly with
an empty user data dir.
This makes 'go test' racy now, as Browser doesn't support creating tabs
concurrently right now. Follow-up commits will fix that, with the help
of 'go test -race' after this commit.
We broke this in the refactor because of a nil pointer dereference, but
we didn't catch that as none of the tests loaded a page with an iframe.
That is, a page with multiple frames.
Add such a test, and fix the bug by creating an almost-empty frame when
we start receiving events about a new frame before it's navigated to.
Using a smaller viewport speeds up both tests, and lets us know what
dimensions to expect in TestCaptureScreenshot.
For TestScreenshot, we can know what dimensions to expect in advance, as
we have the images in testdata.
'go test -run Screenshot' goes from ~0.9s to ~0.5s on my machine.
Finally, don't run ExampleTitle as part of 'go test', as it's slow.
The Default folder is created asynchronously to Chrome starting to
listen on the debugging protocol port. So we can't expect it to exist.
Instead, base the example on DevToolsActivePort, which we can rely on.
First, collapse Browser.Start with NewBrowser. There's no reason to
split them up.
Second, unexport Browser.userDataDir, since it's only needed for a test.
It's also a bad precedent, as only the ExecAllocator will control the
user data directory.
Third, export Context.Browser, since we were already exporting
Context.Allocator.
Finally, remove the Executor interface, a duplicate of cdp.Executor.
All it did was wait on the entire allocator, which is confusing. From
the user's perspective, this wait method should instead wait for the
resources for its own browser, and not any other browsers sharing the
same allocator.
We haven't decided how to integrate that into our API, so simply replace
it with Allocator.Wait.
Now that they're both faster and more reliable than before the refactor.
On my laptop, 'go test' now consistently takes ~10s, and I haven't found
any flakes in the past couple of days.
The navigate sleeps can be replaced by appropriate wait actions.
Some other tests don't need any sleeps at all. This might be because
work is done synchronously now; I haven't been able to get test flakes
after hundreds of test runs with flags like -parallel=32 -count=200.
We hadn't noticed a few uncaught exceptions being received from the
browser, because the events were ignored. Start printing them via the
error logger.
The ones we were getting were caused by testAllocate running Navigate
actions when the path argument was empty. Navigating to "testdata/"
causes JS exceptions, as it's not a valid page.
Instead, leave the new target pointing at a blank document.
Use a single websocket connection per browser, removing the need for an
extra websocket connection per target.
This is thanks to the Target.sendMessageToTarget command to send
messages to each target, and the Target.receivedMessageFromTarget event
to receive messages back.
The browser handles activity via a single worker goroutine, and the same
technique is used for each target. This means that commands and events
are dealt with in order, and we can do away with some complexity like
mutexes and extra go statements.
First, we want all of the functionality in a single package; this means
collapsing whatever is useful into the root chromedp package.
The runner package is being replaced by the Allocator interface, with a
default implementation which starts browser processes.
The client package doesn't really have a place in the new design. The
context, allocator, and browser types will handle the connection with
each browser.
Finally, the new API is context-based, hence the addition of context.go.
The tests have been modified to build and run against the new API.
This isn't strictly necessary, as one can always build with the earlier
cdproto version specified in go.mod. However, many people still install
chromedp in GOPATH via 'go get -u', so this workaround makes life easier
for a lot of developers.
Fixes#285.
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.
