From 6a7c1c80bc8aa0d978d201433ea2acfcd0572d68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Wed, 20 Nov 2024 10:58:16 -0500 Subject: [PATCH 1/6] Add browser sobekEmptyString helper --- browser/helpers.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/browser/helpers.go b/browser/helpers.go index 10b80473e..4b79e14ba 100644 --- a/browser/helpers.go +++ b/browser/helpers.go @@ -3,6 +3,7 @@ package browser import ( "context" "errors" + "strings" "github.com/grafana/sobek" @@ -61,3 +62,8 @@ func exportArgs(gargs []sobek.Value) []any { func sobekValueExists(v sobek.Value) bool { return v != nil && !sobek.IsUndefined(v) && !sobek.IsNull(v) } + +// sobekEmptyString returns true if a given value is not nil or an empty string. +func sobekEmptyString(v sobek.Value) bool { + return !sobekValueExists(v) || strings.TrimSpace(v.String()) == "" +} From ff81c83bde685294ac1cf5c5c7526e422ba2b367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Wed, 20 Nov 2024 10:58:37 -0500 Subject: [PATCH 2/6] Fix jshandle evaluates for missing arg --- browser/js_handle_mapping.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/browser/js_handle_mapping.go b/browser/js_handle_mapping.go index db300a6d5..ad32bee8d 100644 --- a/browser/js_handle_mapping.go +++ b/browser/js_handle_mapping.go @@ -1,6 +1,8 @@ package browser import ( + "fmt" + "github.com/grafana/sobek" "github.com/grafana/xk6-browser/common" @@ -15,26 +17,32 @@ func mapJSHandle(vu moduleVU, jsh common.JSHandleAPI) mapping { }, "dispose": func() *sobek.Promise { return k6ext.Promise(vu.Context(), func() (any, error) { - return nil, jsh.Dispose() //nolint:wrapcheck + return nil, jsh.Dispose() }) }, - "evaluate": func(pageFunc sobek.Value, gargs ...sobek.Value) *sobek.Promise { + "evaluate": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) { + if sobekEmptyString(pageFunc) { + return nil, fmt.Errorf("evaluate requires a page function") + } return k6ext.Promise(vu.Context(), func() (any, error) { args := make([]any, 0, len(gargs)) for _, a := range gargs { args = append(args, exportArg(a)) } - return jsh.Evaluate(pageFunc.String(), args...) //nolint:wrapcheck - }) + return jsh.Evaluate(pageFunc.String(), args...) + }), nil }, - "evaluateHandle": func(pageFunc sobek.Value, gargs ...sobek.Value) *sobek.Promise { + "evaluateHandle": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) { + if sobekEmptyString(pageFunc) { + return nil, fmt.Errorf("evaluateHandle requires a page function") + } return k6ext.Promise(vu.Context(), func() (any, error) { h, err := jsh.EvaluateHandle(pageFunc.String(), exportArgs(gargs)...) if err != nil { return nil, err //nolint:wrapcheck } return mapJSHandle(vu, h), nil - }) + }), nil }, "getProperties": func() *sobek.Promise { return k6ext.Promise(vu.Context(), func() (any, error) { From 4e0606f5bce5601e2d7107dc047bb6386e15ed38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Wed, 20 Nov 2024 10:29:00 -0500 Subject: [PATCH 3/6] Fix page evaluates for missing arg --- browser/page_mapping.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/browser/page_mapping.go b/browser/page_mapping.go index 7c4650d6d..ce0e82fe7 100644 --- a/browser/page_mapping.go +++ b/browser/page_mapping.go @@ -80,19 +80,25 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop return nil, p.EmulateVisionDeficiency(typ) //nolint:wrapcheck }) }, - "evaluate": func(pageFunction sobek.Value, gargs ...sobek.Value) *sobek.Promise { + "evaluate": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) { + if sobekEmptyString(pageFunc) { + return nil, fmt.Errorf("evaluate requires a page function") + } return k6ext.Promise(vu.Context(), func() (any, error) { - return p.Evaluate(pageFunction.String(), exportArgs(gargs)...) //nolint:wrapcheck - }) + return p.Evaluate(pageFunc.String(), exportArgs(gargs)...) + }), nil }, - "evaluateHandle": func(pageFunc sobek.Value, gargs ...sobek.Value) *sobek.Promise { + "evaluateHandle": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) { + if sobekEmptyString(pageFunc) { + return nil, fmt.Errorf("evaluateHandle requires a page function") + } return k6ext.Promise(vu.Context(), func() (any, error) { jsh, err := p.EvaluateHandle(pageFunc.String(), exportArgs(gargs)...) if err != nil { return nil, err //nolint:wrapcheck } return mapJSHandle(vu, jsh), nil - }) + }), nil }, "fill": func(selector string, value string, opts sobek.Value) *sobek.Promise { return k6ext.Promise(vu.Context(), func() (any, error) { From b59832225f3513488550406b9315c45d9d2e87f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Wed, 20 Nov 2024 10:30:17 -0500 Subject: [PATCH 4/6] Fix frame evaluates for missing arg --- browser/frame_mapping.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/browser/frame_mapping.go b/browser/frame_mapping.go index d414c7dcb..967b9da61 100644 --- a/browser/frame_mapping.go +++ b/browser/frame_mapping.go @@ -59,19 +59,25 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping { //nolint:gocognit,cyclop return nil, f.DispatchEvent(selector, typ, exportArg(eventInit), popts) //nolint:wrapcheck }), nil }, - "evaluate": func(pageFunction sobek.Value, gargs ...sobek.Value) *sobek.Promise { + "evaluate": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) { + if sobekEmptyString(pageFunc) { + return nil, fmt.Errorf("evaluate requires a page function") + } return k6ext.Promise(vu.Context(), func() (any, error) { - return f.Evaluate(pageFunction.String(), exportArgs(gargs)...) //nolint:wrapcheck - }) + return f.Evaluate(pageFunc.String(), exportArgs(gargs)...) + }), nil }, - "evaluateHandle": func(pageFunction sobek.Value, gargs ...sobek.Value) *sobek.Promise { + "evaluateHandle": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) { + if sobekEmptyString(pageFunc) { + return nil, fmt.Errorf("evaluateHandle requires a page function") + } return k6ext.Promise(vu.Context(), func() (any, error) { - jsh, err := f.EvaluateHandle(pageFunction.String(), exportArgs(gargs)...) + jsh, err := f.EvaluateHandle(pageFunc.String(), exportArgs(gargs)...) if err != nil { return nil, err //nolint:wrapcheck } return mapJSHandle(vu, jsh), nil - }) + }), nil }, "fill": func(selector, value string, opts sobek.Value) *sobek.Promise { return k6ext.Promise(vu.Context(), func() (any, error) { From 55a04846dbb6b1d98c1868c2c47a4e4c0da5d389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Wed, 20 Nov 2024 11:00:10 -0500 Subject: [PATCH 5/6] Add TestPageEvaluateMappingError.void --- tests/page_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/page_test.go b/tests/page_test.go index 3bdba1b7a..3fc75de73 100644 --- a/tests/page_test.go +++ b/tests/page_test.go @@ -264,6 +264,11 @@ func TestPageEvaluateMappingError(t *testing.T) { script: "(6)", wantErr: "Given expression does not evaluate to a function", }, + { + name: "void", + script: "", + wantErr: "evaluate requires a page function", + }, } for _, tt := range tests { From b948275166a402325cec5d8b944d09ea246d87d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Mon, 25 Nov 2024 10:42:35 -0500 Subject: [PATCH 6/6] Add TestSobekEmptyString --- browser/helpers_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 browser/helpers_test.go diff --git a/browser/helpers_test.go b/browser/helpers_test.go new file mode 100644 index 000000000..da758924a --- /dev/null +++ b/browser/helpers_test.go @@ -0,0 +1,25 @@ +package browser + +import ( + "testing" + + "github.com/grafana/sobek" + "github.com/stretchr/testify/require" +) + +func TestSobekEmptyString(t *testing.T) { + // SobekEmpty string should return true if the argument + // is an empty string or not defined in the Sobek runtime. + rt := sobek.New() + require.NoError(t, rt.Set("sobekEmptyString", sobekEmptyString)) + for _, s := range []string{"() => true", "'() => false'"} { // not empty + v, err := rt.RunString(`sobekEmptyString(` + s + `)`) + require.NoError(t, err) + require.Falsef(t, v.ToBoolean(), "got: true, want: false for %q", s) + } + for _, s := range []string{"", " ", "null", "undefined"} { // empty + v, err := rt.RunString(`sobekEmptyString(` + s + `)`) + require.NoError(t, err) + require.Truef(t, v.ToBoolean(), "got: false, want: true for %q", s) + } +}