From cd3f2275015f5d2beed875fc6fd44a82e36f70bf Mon Sep 17 00:00:00 2001 From: ankur22 Date: Wed, 19 Oct 2022 16:47:28 +0100 Subject: [PATCH 1/6] Update WaitUntil struct tag When we test using WaitUntil, it is transformed from a go struct into a goja value. If we don't provide a struct tag, goja will transform the name to snake case (wait_until). Adding the js struct tag fixes this. --- common/frame_options.go | 6 +++--- common/page_options.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/frame_options.go b/common/frame_options.go index 9adf76163..db473aefc 100644 --- a/common/frame_options.go +++ b/common/frame_options.go @@ -39,7 +39,7 @@ type FrameFillOptions struct { type FrameGotoOptions struct { Referer string `json:"referer"` Timeout time.Duration `json:"timeout"` - WaitUntil LifecycleEvent `json:"waitUntil"` + WaitUntil LifecycleEvent `json:"waitUntil" js:"waitUntil"` } type FrameHoverOptions struct { @@ -95,7 +95,7 @@ type FrameSelectOptionOptions struct { type FrameSetContentOptions struct { Timeout time.Duration `json:"timeout"` - WaitUntil LifecycleEvent `json:"waitUntil"` + WaitUntil LifecycleEvent `json:"waitUntil" js:"waitUntil"` } type FrameTapOptions struct { @@ -130,7 +130,7 @@ type FrameWaitForLoadStateOptions struct { type FrameWaitForNavigationOptions struct { URL string `json:"url"` - WaitUntil LifecycleEvent `json:"waitUntil"` + WaitUntil LifecycleEvent `json:"waitUntil" js:"waitUntil"` Timeout time.Duration `json:"timeout"` } diff --git a/common/page_options.go b/common/page_options.go index d7d522546..cca1bdfe2 100644 --- a/common/page_options.go +++ b/common/page_options.go @@ -19,7 +19,7 @@ type PageEmulateMediaOptions struct { } type PageReloadOptions struct { - WaitUntil LifecycleEvent `json:"waitUntil"` + WaitUntil LifecycleEvent `json:"waitUntil" js:"waitUntil"` Timeout time.Duration `json:"timeout"` } From d6c3e9f4577cc66729a2a43f0c2eedc000bbdc79 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Wed, 19 Oct 2022 16:53:25 +0100 Subject: [PATCH 2/6] Fix networkIdle waitUntil option for load When a site that is navigated to emits a DOMContentLoaded well before load, this is a signal that we're waiting on assets to load over the network. In such a case (e.g. navigating to google.com) when we goto with waitUntil = networkIdle, due to how we handle such lifecycle events, we end up in a situation where the networkIdle event is ignored. This change ensure that we action on these events too. Partly fixes: https://github.com/orgs/grafana/projects/80/views/14 --- common/frame_session.go | 2 ++ tests/frame_test.go | 61 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/common/frame_session.go b/common/frame_session.go index cbcccf629..080e64939 100644 --- a/common/frame_session.go +++ b/common/frame_session.go @@ -705,6 +705,8 @@ func (fs *FrameSession) onPageLifecycle(event *cdppage.EventLifecycleEvent) { fs.manager.frameLifecycleEvent(event.FrameID, LifecycleEventLoad) case "DOMContentLoaded": fs.manager.frameLifecycleEvent(event.FrameID, LifecycleEventDOMContentLoad) + case "networkIdle": + fs.manager.frameLifecycleEvent(event.FrameID, LifecycleEventNetworkIdle) } eventToMetric := map[string]*k6metrics.Metric{ diff --git a/tests/frame_test.go b/tests/frame_test.go index 7741feaf7..5c7c44026 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -1,9 +1,15 @@ package tests import ( + "fmt" + "net/http" "testing" + "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/grafana/xk6-browser/common" ) func TestFramePress(t *testing.T) { @@ -21,3 +27,58 @@ func TestFramePress(t *testing.T) { require.Equal(t, "AbC", f.InputValue("#text1", nil)) } + +func TestLifecycleNetworkIdle(t *testing.T) { + t.Parallel() + + t.Run("doesn't timeout waiting for networkIdle", func(t *testing.T) { + t.Parallel() + + tb := newTestBrowser(t, withHTTPServer()) + p := tb.NewPage(nil) + + tb.withHandler("/home", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprintf(w, ` + + + +
Waiting...
+ + + + + `) + }) + tb.withHandler("/ping.js", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprintf(w, ` + var serverMsgOutput = document.getElementById("serverMsg"); + serverMsgOutput.innerText = "ping.js loaded from server"; + `) + }) + + var resolved, rejected bool + err := tb.await(func() error { + opts := tb.toGojaValue(common.FrameGotoOptions{ + WaitUntil: common.LifecycleEventNetworkIdle, + Timeout: 30 * time.Second, + }) + tb.promise(p.Goto(tb.URL("/home"), opts)).then( + func() { + result := p.TextContent("#serverMsg", nil) + assert.EqualValues(t, "ping.js loaded from server", result) + + resolved = true + }, + func() { + rejected = true + }, + ) + + return nil + }) + require.NoError(t, err) + + assert.True(t, resolved) + assert.False(t, rejected) + }) +} From 1a81b0f7046b7106118b22494a890e35312948f2 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Wed, 19 Oct 2022 17:07:25 +0100 Subject: [PATCH 3/6] Fix clearLifecycle for goto When we navigate to a page, lifecycle events were being stored. If a new navigation occurred, the previous events would be used and fired off to the internal handles. This is incorrect as it means that WaitUntil will fail to wait for the correct event after the initial navigation to a page. Partly fixes: https://github.com/orgs/grafana/projects/80/views/14 --- common/frame.go | 6 +-- tests/frame_test.go | 91 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 5 deletions(-) diff --git a/common/frame.go b/common/frame.go index 8e5f4a1a0..7f5bceec0 100644 --- a/common/frame.go +++ b/common/frame.go @@ -152,11 +152,7 @@ func (f *Frame) clearLifecycle() { // clear lifecycle events f.lifecycleEventsMu.Lock() - { - for e := range f.lifecycleEvents { - f.lifecycleEvents[e] = false - } - } + f.lifecycleEvents = make(map[LifecycleEvent]bool) f.lifecycleEventsMu.Unlock() f.page.frameManager.MainFrame().recalculateLifecycle() diff --git a/tests/frame_test.go b/tests/frame_test.go index 5c7c44026..eaf2cd4b9 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -3,6 +3,7 @@ package tests import ( "fmt" "net/http" + "sync" "testing" "time" @@ -81,4 +82,94 @@ func TestLifecycleNetworkIdle(t *testing.T) { assert.True(t, resolved) assert.False(t, rejected) }) + + t.Run("doesn't unblock wait for networkIdle too early", func(t *testing.T) { + t.Parallel() + + tb := newTestBrowser(t, withHTTPServer()) + p := tb.NewPage(nil) + + counterMu := sync.RWMutex{} + var counter int + + tb.withHandler("/home", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprintf(w, ` + + + +
Waiting...
+
Waiting...
+ + + + + + `) + }) + ch := make(chan bool) + tb.withHandler("/ping", func(w http.ResponseWriter, _ *http.Request) { + <-ch + + counterMu.Lock() + defer counterMu.Unlock() + + time.Sleep(time.Millisecond * 50) + + counter++ + fmt.Fprintf(w, "pong %d", counter) + }) + tb.withHandler("/ping.js", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprintf(w, ` + var serverMsgOutput = document.getElementById("serverMsg"); + serverMsgOutput.innerText = "ping.js loaded from server"; + `) + + close(ch) + }) + + var resolved, rejected bool + err := tb.await(func() error { + opts := tb.toGojaValue(common.FrameGotoOptions{ + WaitUntil: common.LifecycleEventNetworkIdle, + Timeout: 30 * time.Second, + }) + tb.promise(p.Goto(tb.URL("/home"), opts)).then( + func() { + result := p.TextContent("#prolongNetworkIdleLoad", nil) + assert.EqualValues(t, "Waiting... pong 4 - for loop complete", result) + + result = p.TextContent("#serverMsg", nil) + assert.EqualValues(t, "ping.js loaded from server", result) + + resolved = true + }, + func() { + rejected = true + }, + ) + + return nil + }) + require.NoError(t, err) + + assert.True(t, resolved) + assert.False(t, rejected) + }) } From 4a5581687fdb9d6500a8e8568ebff4ada9ee1105 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Wed, 19 Oct 2022 17:17:51 +0100 Subject: [PATCH 4/6] Fix WaitUntil for long running network requests Some websites may have many network requests which are performed after the page has fully loaded and we receive a FrameStoppedLoading. For now we want to wait for the networkIdle event, and in the future if we need, we should add a timer for very chatty websites where we may never see a networkIdle event. Partly fixes: https://github.com/orgs/grafana/projects/80/views/14 --- common/frame.go | 16 +++++---- common/frame_session.go | 4 +-- tests/frame_test.go | 74 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 8 deletions(-) diff --git a/common/frame.go b/common/frame.go index 7f5bceec0..0dc7620f8 100644 --- a/common/frame.go +++ b/common/frame.go @@ -458,12 +458,16 @@ func (f *Frame) onLoadingStarted() { func (f *Frame) onLoadingStopped() { f.log.Debugf("Frame:onLoadingStopped", "fid:%s furl:%q", f.ID(), f.URL()) - f.lifecycleEventsMu.Lock() - defer f.lifecycleEventsMu.Unlock() - - f.lifecycleEvents[LifecycleEventDOMContentLoad] = true - f.lifecycleEvents[LifecycleEventLoad] = true - f.lifecycleEvents[LifecycleEventNetworkIdle] = true + // TODO: We should start a timer here and allow the user + // to set how long to wait until after onLoadingStopped + // has occurred. The reason we may want a timeout here + // are for websites where they perform many network + // requests so it may take a long time for us to see + // a networkIdle event or we may never see one if the + // website never stops performing network requests. + // NOTE: This is a different timeout to networkIdleTimer, + // which only works once there are no more network + // requests and we don't see a networkIdle event. } func (f *Frame) position() *Position { diff --git a/common/frame_session.go b/common/frame_session.go index 080e64939..32eac456d 100644 --- a/common/frame_session.go +++ b/common/frame_session.go @@ -475,8 +475,8 @@ func (fs *FrameSession) handleFrameTree(frameTree *cdppage.FrameTree) { func (fs *FrameSession) navigateFrame(frame *Frame, url, referrer string) (string, error) { fs.logger.Debugf("FrameSession:navigateFrame", - "sid:%v tid:%v url:%q referrer:%q", - fs.session.ID(), fs.targetID, url, referrer) + "sid:%v fid:%s tid:%v url:%q referrer:%q", + fs.session.ID(), frame.ID(), fs.targetID, url, referrer) action := cdppage.Navigate(url).WithReferrer(referrer).WithFrameID(cdp.FrameID(frame.ID())) _, documentID, errorText, err := action.Do(cdp.WithExecutor(fs.ctx, fs.session)) diff --git a/tests/frame_test.go b/tests/frame_test.go index eaf2cd4b9..f10365aed 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -172,4 +172,78 @@ func TestLifecycleNetworkIdle(t *testing.T) { assert.True(t, resolved) assert.False(t, rejected) }) + + t.Run("doesn't unblock wait on networkIdle early when load and domcontentloaded complete at once", func(t *testing.T) { + t.Parallel() + + tb := newTestBrowser(t, withHTTPServer()) + p := tb.NewPage(nil) + + counterMu := sync.RWMutex{} + var counter int + + tb.withHandler("/home", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprintf(w, ` + + + +
Waiting...
+ + + + + `) + }) + tb.withHandler("/ping", func(w http.ResponseWriter, _ *http.Request) { + counterMu.Lock() + defer counterMu.Unlock() + + time.Sleep(time.Millisecond * 50) + + counter++ + fmt.Fprintf(w, "pong %d", counter) + }) + + var resolved, rejected bool + err := tb.await(func() error { + opts := tb.toGojaValue(common.FrameGotoOptions{ + WaitUntil: common.LifecycleEventNetworkIdle, + Timeout: 30 * time.Second, + }) + tb.promise(p.Goto(tb.URL("/home"), opts)).then( + func() { + result := p.TextContent("#prolongNetworkIdleLoad", nil) + assert.EqualValues(t, "Waiting... pong 10 - for loop complete", result) + + resolved = true + }, + func() { + rejected = true + }, + ) + + return nil + }) + require.NoError(t, err) + + assert.True(t, resolved) + assert.False(t, rejected) + }) } From abdd521a67b8f7d0fd3ee67f3068065947132be1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Mon, 24 Oct 2022 14:34:46 +0300 Subject: [PATCH 5/6] Add assertHome helper to TestLifecycleNetworkIdle --- tests/frame_test.go | 111 +++++++++++++++----------------------------- 1 file changed, 37 insertions(+), 74 deletions(-) diff --git a/tests/frame_test.go b/tests/frame_test.go index f10365aed..8516938fe 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/grafana/xk6-browser/api" "github.com/grafana/xk6-browser/common" ) @@ -32,6 +33,31 @@ func TestFramePress(t *testing.T) { func TestLifecycleNetworkIdle(t *testing.T) { t.Parallel() + assertHome := func(tb *testBrowser, p api.Page, check func()) { + var resolved, rejected bool + err := tb.await(func() error { + opts := tb.toGojaValue(common.FrameGotoOptions{ + WaitUntil: common.LifecycleEventNetworkIdle, + Timeout: 30 * time.Second, + }) + tb.promise(p.Goto(tb.URL("/home"), opts)).then( + func() { + check() + resolved = true + }, + func() { + rejected = true + }, + ) + + return nil + }) + require.NoError(t, err) + + assert.True(t, resolved) + assert.False(t, rejected) + } + t.Run("doesn't timeout waiting for networkIdle", func(t *testing.T) { t.Parallel() @@ -56,31 +82,10 @@ func TestLifecycleNetworkIdle(t *testing.T) { serverMsgOutput.innerText = "ping.js loaded from server"; `) }) - - var resolved, rejected bool - err := tb.await(func() error { - opts := tb.toGojaValue(common.FrameGotoOptions{ - WaitUntil: common.LifecycleEventNetworkIdle, - Timeout: 30 * time.Second, - }) - tb.promise(p.Goto(tb.URL("/home"), opts)).then( - func() { - result := p.TextContent("#serverMsg", nil) - assert.EqualValues(t, "ping.js loaded from server", result) - - resolved = true - }, - func() { - rejected = true - }, - ) - - return nil + assertHome(tb, p, func() { + result := p.TextContent("#serverMsg", nil) + assert.EqualValues(t, "ping.js loaded from server", result) }) - require.NoError(t, err) - - assert.True(t, resolved) - assert.False(t, rejected) }) t.Run("doesn't unblock wait for networkIdle too early", func(t *testing.T) { @@ -143,34 +148,13 @@ func TestLifecycleNetworkIdle(t *testing.T) { close(ch) }) + assertHome(tb, p, func() { + result := p.TextContent("#prolongNetworkIdleLoad", nil) + assert.EqualValues(t, "Waiting... pong 4 - for loop complete", result) - var resolved, rejected bool - err := tb.await(func() error { - opts := tb.toGojaValue(common.FrameGotoOptions{ - WaitUntil: common.LifecycleEventNetworkIdle, - Timeout: 30 * time.Second, - }) - tb.promise(p.Goto(tb.URL("/home"), opts)).then( - func() { - result := p.TextContent("#prolongNetworkIdleLoad", nil) - assert.EqualValues(t, "Waiting... pong 4 - for loop complete", result) - - result = p.TextContent("#serverMsg", nil) - assert.EqualValues(t, "ping.js loaded from server", result) - - resolved = true - }, - func() { - rejected = true - }, - ) - - return nil + result = p.TextContent("#serverMsg", nil) + assert.EqualValues(t, "ping.js loaded from server", result) }) - require.NoError(t, err) - - assert.True(t, resolved) - assert.False(t, rejected) }) t.Run("doesn't unblock wait on networkIdle early when load and domcontentloaded complete at once", func(t *testing.T) { @@ -220,30 +204,9 @@ func TestLifecycleNetworkIdle(t *testing.T) { counter++ fmt.Fprintf(w, "pong %d", counter) }) - - var resolved, rejected bool - err := tb.await(func() error { - opts := tb.toGojaValue(common.FrameGotoOptions{ - WaitUntil: common.LifecycleEventNetworkIdle, - Timeout: 30 * time.Second, - }) - tb.promise(p.Goto(tb.URL("/home"), opts)).then( - func() { - result := p.TextContent("#prolongNetworkIdleLoad", nil) - assert.EqualValues(t, "Waiting... pong 10 - for loop complete", result) - - resolved = true - }, - func() { - rejected = true - }, - ) - - return nil + assertHome(tb, p, func() { + result := p.TextContent("#prolongNetworkIdleLoad", nil) + assert.EqualValues(t, "Waiting... pong 10 - for loop complete", result) }) - require.NoError(t, err) - - assert.True(t, resolved) - assert.False(t, rejected) }) } From 657bd49daba77cc25d9afd9bcc1a8c5b76fcf27b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Mon, 24 Oct 2022 14:47:08 +0300 Subject: [PATCH 6/6] Move pages to static in TestLifecycleNetworkIdle --- tests/frame_test.go | 88 ++++----------------- tests/static/prolonged_network_idle.html | 30 +++++++ tests/static/prolonged_network_idle_10.html | 28 +++++++ 3 files changed, 75 insertions(+), 71 deletions(-) create mode 100644 tests/static/prolonged_network_idle.html create mode 100644 tests/static/prolonged_network_idle_10.html diff --git a/tests/frame_test.go b/tests/frame_test.go index 8516938fe..ce123ab82 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -63,25 +63,25 @@ func TestLifecycleNetworkIdle(t *testing.T) { tb := newTestBrowser(t, withHTTPServer()) p := tb.NewPage(nil) - tb.withHandler("/home", func(w http.ResponseWriter, _ *http.Request) { fmt.Fprintf(w, `
Waiting...
- `) }) + tb.withHandler("/ping.js", func(w http.ResponseWriter, _ *http.Request) { fmt.Fprintf(w, ` var serverMsgOutput = document.getElementById("serverMsg"); serverMsgOutput.innerText = "ping.js loaded from server"; `) }) + assertHome(tb, p, func() { result := p.TextContent("#serverMsg", nil) assert.EqualValues(t, "ping.js loaded from server", result) @@ -91,47 +91,18 @@ func TestLifecycleNetworkIdle(t *testing.T) { t.Run("doesn't unblock wait for networkIdle too early", func(t *testing.T) { t.Parallel() - tb := newTestBrowser(t, withHTTPServer()) + tb := newTestBrowser(t, withFileServer()) p := tb.NewPage(nil) - - counterMu := sync.RWMutex{} - var counter int - - tb.withHandler("/home", func(w http.ResponseWriter, _ *http.Request) { - fmt.Fprintf(w, ` - - - -
Waiting...
-
Waiting...
- - - - - - `) + tb.withHandler("/home", func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, tb.staticURL("prolonged_network_idle.html"), http.StatusMovedPermanently) }) + + var counter int64 ch := make(chan bool) tb.withHandler("/ping", func(w http.ResponseWriter, _ *http.Request) { <-ch + var counterMu sync.Mutex counterMu.Lock() defer counterMu.Unlock() @@ -140,14 +111,15 @@ func TestLifecycleNetworkIdle(t *testing.T) { counter++ fmt.Fprintf(w, "pong %d", counter) }) + tb.withHandler("/ping.js", func(w http.ResponseWriter, _ *http.Request) { fmt.Fprintf(w, ` var serverMsgOutput = document.getElementById("serverMsg"); serverMsgOutput.innerText = "ping.js loaded from server"; `) - close(ch) }) + assertHome(tb, p, func() { result := p.TextContent("#prolongNetworkIdleLoad", nil) assert.EqualValues(t, "Waiting... pong 4 - for loop complete", result) @@ -160,41 +132,14 @@ func TestLifecycleNetworkIdle(t *testing.T) { t.Run("doesn't unblock wait on networkIdle early when load and domcontentloaded complete at once", func(t *testing.T) { t.Parallel() - tb := newTestBrowser(t, withHTTPServer()) + tb := newTestBrowser(t, withFileServer()) p := tb.NewPage(nil) - - counterMu := sync.RWMutex{} - var counter int - - tb.withHandler("/home", func(w http.ResponseWriter, _ *http.Request) { - fmt.Fprintf(w, ` - - - -
Waiting...
- - - - - `) + tb.withHandler("/home", func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, tb.staticURL("prolonged_network_idle_10.html"), http.StatusMovedPermanently) }) + + var counterMu sync.Mutex + var counter int64 tb.withHandler("/ping", func(w http.ResponseWriter, _ *http.Request) { counterMu.Lock() defer counterMu.Unlock() @@ -204,6 +149,7 @@ func TestLifecycleNetworkIdle(t *testing.T) { counter++ fmt.Fprintf(w, "pong %d", counter) }) + assertHome(tb, p, func() { result := p.TextContent("#prolongNetworkIdleLoad", nil) assert.EqualValues(t, "Waiting... pong 10 - for loop complete", result) diff --git a/tests/static/prolonged_network_idle.html b/tests/static/prolonged_network_idle.html new file mode 100644 index 000000000..9e8046105 --- /dev/null +++ b/tests/static/prolonged_network_idle.html @@ -0,0 +1,30 @@ + + + + + +
Waiting...
+
Waiting...
+ + + + + + \ No newline at end of file diff --git a/tests/static/prolonged_network_idle_10.html b/tests/static/prolonged_network_idle_10.html new file mode 100644 index 000000000..1006ee750 --- /dev/null +++ b/tests/static/prolonged_network_idle_10.html @@ -0,0 +1,28 @@ + + + + + +
Waiting...
+ + + + + \ No newline at end of file