commons_go/chromedp
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

error in Run if passed an allocator context

Browse Source 
The correct way to use Run is after NewContext. Using NewExecAllocator
only, which also gives a context, almost worked. This confused some
users into thinking it was a bug in chromedp.

Instead, error immediately with an invalid context error.

Fixes #299.
This commit is contained in:
Daniel Martí 2019-04-14 19:56:09 +09:00
parent 92a77355f6
commit ac47d6ba0e
2 changed files with 26 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
allocate_test.go
View File

@ -9,13 +9,13 @@ import (
func TestExecAllocator(t *testing.T) { func TestExecAllocator(t *testing.T) {
t.Parallel() t.Parallel()
poolCtx, poolCancel := NewExecAllocator(context.Background(), allocOpts...) allocCtx, cancel := NewExecAllocator(context.Background(), allocOpts...)
defer poolCancel() defer cancel()
// TODO: test that multiple child contexts are run in different // TODO: test that multiple child contexts are run in different
// processes and browsers. // processes and browsers.
taskCtx, cancel := NewContext(poolCtx) taskCtx, cancel := NewContext(allocCtx)
defer cancel() defer cancel()
want := "insert" want := "insert"
@ -41,22 +41,36 @@ func TestExecAllocator(t *testing.T) {
func TestExecAllocatorCancelParent(t *testing.T) { func TestExecAllocatorCancelParent(t *testing.T) {
t.Parallel() t.Parallel()
poolCtx, poolCancel := NewExecAllocator(context.Background(), allocOpts...) allocCtx, allocCancel := NewExecAllocator(context.Background(), allocOpts...)
defer poolCancel() defer allocCancel()
// TODO: test that multiple child contexts are run in different // TODO: test that multiple child contexts are run in different
// processes and browsers. // processes and browsers.
taskCtx, _ := NewContext(poolCtx) taskCtx, _ := NewContext(allocCtx)
if err := Run(taskCtx); err != nil { if err := Run(taskCtx); err != nil {
t.Fatal(err) t.Fatal(err)
} }
// Canceling the pool context should stop all browsers too. // Canceling the pool context should stop all browsers too.
poolCancel() allocCancel()
tempDir := FromContext(taskCtx).Browser.userDataDir tempDir := FromContext(taskCtx).Browser.userDataDir
if _, err := os.Lstat(tempDir); !os.IsNotExist(err) { if _, err := os.Lstat(tempDir); !os.IsNotExist(err) {
t.Fatalf("temporary user data dir %q not deleted", tempDir) t.Fatalf("temporary user data dir %q not deleted", tempDir)
} }
} }
func TestSkipNewContext(t *testing.T) {
ctx, cancel := NewExecAllocator(context.Background(), allocOpts...)
defer cancel()
// Using the allocator context directly (without calling NewContext)
// should be an immediate error.
err := Run(ctx, Navigate(testdataDir+"/form.html"))
want := ErrInvalidContext
if err != want {
t.Fatalf("want error to be %q, got %q", want, err)
}
}

6
chromedp.go
View File

@ -154,7 +154,11 @@ func Cancel(ctx context.Context) error {
// via one of the allocator constructors like NewExecAllocator. // via one of the allocator constructors like NewExecAllocator.
func Run(ctx context.Context, actions ...Action) error { func Run(ctx context.Context, actions ...Action) error {
c := FromContext(ctx) c := FromContext(ctx)
if c == nil || c.Allocator == nil { // If c is nil, it's not a chromedp context.
// If c.Allocator is nil, NewContext wasn't used properly.
// If c.cancel is nil, Run is being called directly with an allocator
// context.
if c == nil || c.Allocator == nil || c.cancel == nil {
return ErrInvalidContext return ErrInvalidContext
} }
if c.Browser == nil { if c.Browser == nil {