From 1d32ee477b763962e2bc8f2ef86f240285845325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Miri=C4=87?= Date: Fri, 22 Apr 2022 19:21:31 +0200 Subject: [PATCH 1/5] Add lifecycle event validation to WaitForLoadState --- common/frame.go | 9 ++---- common/types.go | 43 +++++++++++++++++++++++++++ common/types_test.go | 69 ++++++++++++++++++++++++++++++++++++++++++++ tests/page_test.go | 23 +++++++++++++-- 4 files changed, 136 insertions(+), 8 deletions(-) diff --git a/common/frame.go b/common/frame.go index 6518d6c50..f82927a86 100644 --- a/common/frame.go +++ b/common/frame.go @@ -1583,12 +1583,9 @@ func (f *Frame) WaitForLoadState(state string, opts goja.Value) { k6Throw(f.ctx, "cannot parse waitForLoadState options: %v", err) } - waitUntil := LifecycleEventLoad - switch state { - case "domcontentloaded": - waitUntil = LifecycleEventDOMContentLoad - case "networkidle": - waitUntil = LifecycleEventNetworkIdle + var waitUntil LifecycleEvent + if err = waitUntil.UnmarshalText([]byte(state)); err != nil { + k6Throw(f.ctx, "waitForLoadState error: %v", err) } if f.hasLifecycleEventFired(waitUntil) { diff --git a/common/types.go b/common/types.go index fbddd2713..a5e206c8d 100644 --- a/common/types.go +++ b/common/types.go @@ -26,6 +26,8 @@ import ( "encoding/json" "fmt" "math" + "sort" + "strings" "github.com/dop251/goja" @@ -278,6 +280,47 @@ func (l *LifecycleEvent) UnmarshalJSON(b []byte) error { return nil } +// MarshalText returns the string representation of the enum value. +// It returns an error if the enum value is invalid. +func (l *LifecycleEvent) MarshalText() ([]byte, error) { + if l == nil { + return []byte(""), nil + } + var ( + ok bool + s string + ) + if s, ok = lifecycleEventToString[*l]; !ok { + return nil, fmt.Errorf("invalid lifecycle event: %v", int(*l)) + } + + return []byte(s), nil +} + +// UnmarshalText unmarshals a text representation to the enum value. +// It returns an error if given a wrong value. +func (l *LifecycleEvent) UnmarshalText(text []byte) error { + var ( + ok bool + val = string(text) + ) + + if *l, ok = lifecycleEventToID[val]; !ok { + valid := make([]string, 0, len(lifecycleEventToID)) + for k := range lifecycleEventToID { + valid = append(valid, k) + } + sort.Slice(valid, func(i, j int) bool { + return lifecycleEventToID[valid[j]] > lifecycleEventToID[valid[i]] + }) + return fmt.Errorf( + "invalid lifecycle event: %q; must be one of: %s", + val, strings.Join(valid, ", ")) + } + + return nil +} + type MediaType string const ( diff --git a/common/types_test.go b/common/types_test.go index 950fc0c9f..3e47c2570 100644 --- a/common/types_test.go +++ b/common/types_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // See: Issue #183 for details. @@ -60,3 +61,71 @@ func TestViewportCalculateInset(t *testing.T) { }) } } + +func TestLifecycleEventMarshalText(t *testing.T) { + t.Parallel() + + t.Run("ok/nil", func(t *testing.T) { + t.Parallel() + + var evt *LifecycleEvent + m, err := evt.MarshalText() + require.NoError(t, err) + assert.Equal(t, []byte(""), m) + }) + + t.Run("err/invalid", func(t *testing.T) { + t.Parallel() + + evt := LifecycleEvent(-1) + _, err := evt.MarshalText() + require.EqualError(t, err, "invalid lifecycle event: -1") + }) +} + +func TestLifecycleEventMarshalTextRound(t *testing.T) { + t.Parallel() + + evt := LifecycleEventLoad + m, err := evt.MarshalText() + require.NoError(t, err) + assert.Equal(t, []byte("load"), m) + + var evt2 LifecycleEvent + err = evt2.UnmarshalText(m) + require.NoError(t, err) + assert.Equal(t, evt, evt2) +} + +func TestLifecycleEventUnmarshalText(t *testing.T) { + t.Parallel() + + t.Run("ok", func(t *testing.T) { + t.Parallel() + + var evt LifecycleEvent + err := evt.UnmarshalText([]byte("load")) + require.NoError(t, err) + assert.Equal(t, LifecycleEventLoad, evt) + }) + + t.Run("err/invalid", func(t *testing.T) { + t.Parallel() + + var evt LifecycleEvent + err := evt.UnmarshalText([]byte("none")) + require.EqualError(t, err, + `invalid lifecycle event: "none"; `+ + `must be one of: load, domcontentloaded, networkidle`) + }) + + t.Run("err/invalid_empty", func(t *testing.T) { + t.Parallel() + + var evt LifecycleEvent + err := evt.UnmarshalText([]byte("")) + require.EqualError(t, err, + `invalid lifecycle event: ""; `+ + `must be one of: load, domcontentloaded, networkidle`) + }) +} diff --git a/tests/page_test.go b/tests/page_test.go index 4043a08a7..95f5ac4de 100644 --- a/tests/page_test.go +++ b/tests/page_test.go @@ -629,6 +629,26 @@ func TestPageWaitForFunction(t *testing.T) { }) } +func TestPageWaitForLoadState(t *testing.T) { + t.Parallel() + + // TODO: Add happy path tests once WaitForLoadState is not racy. + + t.Run("err_wrong_event", func(t *testing.T) { + t.Parallel() + + defer func() { + assertPanicErrorContains(t, recover(), + `invalid lifecycle event: "none"; `+ + `must be one of: load, domcontentloaded, networkidle`) + }() + + p := newTestBrowser(t).NewPage(nil) + p.WaitForLoadState("none", nil) + t.Error("did not panic") + }) +} + // See: The issue #187 for details. func TestPageWaitForNavigationShouldNotPanic(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) @@ -645,8 +665,7 @@ func assertPanicErrorContains(t *testing.T, err interface{}, expErrMsg string) { require.IsType(t, &goja.Object{}, err) gotObj, _ := err.(*goja.Object) got := gotObj.Export() - expErr := fmt.Errorf("%w", errors.New(expErrMsg)) - require.IsType(t, expErr, got) + expErr := errors.New(expErrMsg) gotErr, ok := got.(error) require.True(t, ok) assert.Contains(t, gotErr.Error(), expErr.Error()) From cbac7094c2176c8b68dfb8f7097866ca55497c89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Miri=C4=87?= Date: Mon, 25 Apr 2022 17:20:34 +0200 Subject: [PATCH 2/5] Validate FrameWaitForNavigation WaitUntil using UnmarshalText --- common/frame_options.go | 6 ++--- common/frame_options_test.go | 47 ++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 common/frame_options_test.go diff --git a/common/frame_options.go b/common/frame_options.go index 6957ed6a3..a04889206 100644 --- a/common/frame_options.go +++ b/common/frame_options.go @@ -678,10 +678,8 @@ func (o *FrameWaitForNavigationOptions) Parse(ctx context.Context, opts goja.Val o.Timeout = time.Duration(opts.Get(k).ToInteger()) * time.Millisecond case "waitUntil": lifeCycle := opts.Get(k).String() - if l, ok := lifecycleEventToID[lifeCycle]; ok { - o.WaitUntil = l - } else { - return fmt.Errorf("%q is not a valid lifecycle", lifeCycle) + if err := o.WaitUntil.UnmarshalText([]byte(lifeCycle)); err != nil { + return fmt.Errorf("error parsing waitForNavigation options: %w", err) } } } diff --git a/common/frame_options_test.go b/common/frame_options_test.go new file mode 100644 index 000000000..0320c3e6d --- /dev/null +++ b/common/frame_options_test.go @@ -0,0 +1,47 @@ +package common + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFrameWaitForNavigationOptionsParse(t *testing.T) { + t.Parallel() + + t.Run("ok", func(t *testing.T) { + t.Parallel() + + mockVU := newMockVU(t) + opts := mockVU.RuntimeField.ToValue(map[string]interface{}{ + "url": "https://example.com/", + "timeout": "1000", + "waitUntil": "networkidle", + }) + navOpts := NewFrameWaitForNavigationOptions(0) + err := navOpts.Parse(mockVU.CtxField, opts) + require.NoError(t, err) + + assert.Equal(t, "https://example.com/", navOpts.URL) + assert.Equal(t, time.Second, navOpts.Timeout) + assert.Equal(t, LifecycleEventNetworkIdle, navOpts.WaitUntil) + }) + + t.Run("err/invalid_waitUntil", func(t *testing.T) { + t.Parallel() + + mockVU := newMockVU(t) + opts := mockVU.RuntimeField.ToValue(map[string]interface{}{ + "waitUntil": "none", + }) + navOpts := NewFrameWaitForNavigationOptions(0) + err := navOpts.Parse(mockVU.CtxField, opts) + + assert.EqualError(t, err, + `error parsing waitForNavigation options: `+ + `invalid lifecycle event: "none"; must be one of: `+ + `load, domcontentloaded, networkidle`) + }) +} From 707c3b9df93fd0e5bd3ff679bfbd2aa6782d5711 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Miri=C4=87?= Date: Mon, 25 Apr 2022 17:27:22 +0200 Subject: [PATCH 3/5] Validate FrameGoto WaitUntil using UnmarshalText --- common/frame_options.go | 6 ++---- common/frame_options_test.go | 37 ++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/common/frame_options.go b/common/frame_options.go index a04889206..89bf8230c 100644 --- a/common/frame_options.go +++ b/common/frame_options.go @@ -297,10 +297,8 @@ func (o *FrameGotoOptions) Parse(ctx context.Context, opts goja.Value) error { o.Timeout = time.Duration(opts.Get(k).ToInteger()) * time.Millisecond case "waitUntil": lifeCycle := opts.Get(k).String() - if l, ok := lifecycleEventToID[lifeCycle]; ok { - o.WaitUntil = l - } else { - return fmt.Errorf("%q is not a valid lifecycle", lifeCycle) + if err := o.WaitUntil.UnmarshalText([]byte(lifeCycle)); err != nil { + return fmt.Errorf("error parsing goto options: %w", err) } } } diff --git a/common/frame_options_test.go b/common/frame_options_test.go index 0320c3e6d..ca230ef33 100644 --- a/common/frame_options_test.go +++ b/common/frame_options_test.go @@ -8,6 +8,43 @@ import ( "github.com/stretchr/testify/require" ) +func TestFrameGotoOptionsParse(t *testing.T) { + t.Parallel() + + t.Run("ok", func(t *testing.T) { + t.Parallel() + + mockVU := newMockVU(t) + opts := mockVU.RuntimeField.ToValue(map[string]interface{}{ + "timeout": "1000", + "waitUntil": "networkidle", + }) + gotoOpts := NewFrameGotoOptions("https://example.com/", 0) + err := gotoOpts.Parse(mockVU.CtxField, opts) + require.NoError(t, err) + + assert.Equal(t, "https://example.com/", gotoOpts.Referer) + assert.Equal(t, time.Second, gotoOpts.Timeout) + assert.Equal(t, LifecycleEventNetworkIdle, gotoOpts.WaitUntil) + }) + + t.Run("err/invalid_waitUntil", func(t *testing.T) { + t.Parallel() + + mockVU := newMockVU(t) + opts := mockVU.RuntimeField.ToValue(map[string]interface{}{ + "waitUntil": "none", + }) + navOpts := NewFrameGotoOptions("", 0) + err := navOpts.Parse(mockVU.CtxField, opts) + + assert.EqualError(t, err, + `error parsing goto options: `+ + `invalid lifecycle event: "none"; must be one of: `+ + `load, domcontentloaded, networkidle`) + }) +} + func TestFrameWaitForNavigationOptionsParse(t *testing.T) { t.Parallel() From 62fa217c3a13560479fbd4b50b6bd25120cc303f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Miri=C4=87?= Date: Mon, 25 Apr 2022 17:35:40 +0200 Subject: [PATCH 4/5] Validate FrameSetContent WaitUntil using UnmarshalText --- common/frame_options.go | 6 ++---- common/frame_options_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/common/frame_options.go b/common/frame_options.go index 89bf8230c..fb07063c4 100644 --- a/common/frame_options.go +++ b/common/frame_options.go @@ -502,10 +502,8 @@ func (o *FrameSetContentOptions) Parse(ctx context.Context, opts goja.Value) err o.Timeout = time.Duration(opts.Get(k).ToInteger()) * time.Millisecond case "waitUntil": lifeCycle := opts.Get(k).String() - if l, ok := lifecycleEventToID[lifeCycle]; ok { - o.WaitUntil = l - } else { - return fmt.Errorf("%q is not a valid lifecycle", lifeCycle) + if err := o.WaitUntil.UnmarshalText([]byte(lifeCycle)); err != nil { + return fmt.Errorf("error parsing setContent options: %w", err) } } } diff --git a/common/frame_options_test.go b/common/frame_options_test.go index ca230ef33..f02d16c2d 100644 --- a/common/frame_options_test.go +++ b/common/frame_options_test.go @@ -45,6 +45,41 @@ func TestFrameGotoOptionsParse(t *testing.T) { }) } +func TestFrameSetContentOptionsParse(t *testing.T) { + t.Parallel() + + t.Run("ok", func(t *testing.T) { + t.Parallel() + + mockVU := newMockVU(t) + opts := mockVU.RuntimeField.ToValue(map[string]interface{}{ + "waitUntil": "networkidle", + }) + scOpts := NewFrameSetContentOptions(30 * time.Second) + err := scOpts.Parse(mockVU.CtxField, opts) + require.NoError(t, err) + + assert.Equal(t, 30*time.Second, scOpts.Timeout) + assert.Equal(t, LifecycleEventNetworkIdle, scOpts.WaitUntil) + }) + + t.Run("err/invalid_waitUntil", func(t *testing.T) { + t.Parallel() + + mockVU := newMockVU(t) + opts := mockVU.RuntimeField.ToValue(map[string]interface{}{ + "waitUntil": "none", + }) + navOpts := NewFrameSetContentOptions(0) + err := navOpts.Parse(mockVU.CtxField, opts) + + assert.EqualError(t, err, + `error parsing setContent options: `+ + `invalid lifecycle event: "none"; must be one of: `+ + `load, domcontentloaded, networkidle`) + }) +} + func TestFrameWaitForNavigationOptionsParse(t *testing.T) { t.Parallel() From a526dd99914f2946e401c476a3f5063a50587286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Miri=C4=87?= Date: Tue, 26 Apr 2022 10:17:57 +0200 Subject: [PATCH 5/5] Fix defaulting to load in WaitForLoadState --- common/frame.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/common/frame.go b/common/frame.go index f82927a86..52f785ca5 100644 --- a/common/frame.go +++ b/common/frame.go @@ -1583,9 +1583,11 @@ func (f *Frame) WaitForLoadState(state string, opts goja.Value) { k6Throw(f.ctx, "cannot parse waitForLoadState options: %v", err) } - var waitUntil LifecycleEvent - if err = waitUntil.UnmarshalText([]byte(state)); err != nil { - k6Throw(f.ctx, "waitForLoadState error: %v", err) + waitUntil := LifecycleEventLoad + if state != "" { + if err = waitUntil.UnmarshalText([]byte(state)); err != nil { + k6Throw(f.ctx, "waitForLoadState error: %v", err) + } } if f.hasLifecycleEventFired(waitUntil) {