From a1865df40a6f3361e66ec50e0e0bb85540cbe293 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Tue, 12 Dec 2023 10:21:58 +0000 Subject: [PATCH 1/4] Add integration tests for evaluate parse object This integration test will evaluate objects which uses the existing valueFromRemoteObject function in remote_object and not the latest parseConsoleRemoteObject function. In this change there are 3 commented out sub tests, 2 of which will be fixed, and 1 will remain commented out until a fix for the linked issue has been resolved. --- tests/remote_obj_test.go | 85 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/tests/remote_obj_test.go b/tests/remote_obj_test.go index 5992c7e33..be5ab9a68 100644 --- a/tests/remote_obj_test.go +++ b/tests/remote_obj_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/dop251/goja" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -106,3 +107,87 @@ func TestConsoleLogParse(t *testing.T) { }) } } + +func TestEvalRemoteObjectParse(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + eval string + want any + }{ + { + name: "number", eval: "1", want: 1, + }, + { + name: "string", eval: `"some string"`, want: "some string", + }, + { + name: "bool", eval: "true", want: true, + }, + { + name: "empty_array", eval: "[]", want: []any{}, + }, + { + name: "empty_object", eval: "{}", want: goja.Undefined(), + }, + { + name: "filled_object", eval: `{return {foo:"bar"};}`, want: map[string]any{"foo": "bar"}, + }, + { + name: "filled_array", eval: `{return ["foo","bar"];}`, want: []interface{}{0: "foo", 1: "bar"}, + }, + { + name: "filled_array", eval: `() => true`, want: `function()`, + }, + { + name: "empty", eval: "", want: "", + }, + { + name: "null", eval: "null", want: "null", + }, + { + name: "undefined", eval: "undefined", want: goja.Undefined(), + }, + // { + // // Commented out until fix applied + // name: "bigint", eval: `BigInt("2")`, want: "2n", + // }, + // { + // // Commented out until fix applied + // name: "unwrapped_bigint", eval: "3n", want: "3n", + // }, + { + name: "float", eval: "3.14", want: 3.14, + }, + { + name: "scientific_notation", eval: "123e-5", want: 0.00123, + }, + // { + // // This test is ignored until https://github.com/grafana/xk6-browser/issues/1132 + // // has been resolved. + // name: "partially_parsed", + // eval: "window", + // want: `{"document":"#document","location":"Location","name":"","self":"Window","window":"Window"}`, + // }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + tb := newTestBrowser(t, withFileServer()) + p := tb.NewPage(nil) + + var got any + if tt.eval == "" { + got = p.Evaluate(tb.toGojaValue(`() => ""`)) + } else { + got = p.Evaluate(tb.toGojaValue(fmt.Sprintf("() => %s", tt.eval))) + } + + assert.Equal(t, tb.toGojaValue(tt.want), got) + }) + } +} From 3fe89c0dd9f5cf02902c7849e21285db3439a195 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Tue, 12 Dec 2023 10:38:59 +0000 Subject: [PATCH 2/4] Fix BigInt parsing For an unknown reason BigInt can't be parsed and is being set as a UnserializableValue. To ensure that we parse it correctly to an int we need to catch this case when dealing with other UnserializableValue. --- common/remote_object.go | 11 +++++++++++ tests/remote_obj_test.go | 14 ++++++-------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/common/remote_object.go b/common/remote_object.go index fec6ec2af..ade675032 100644 --- a/common/remote_object.go +++ b/common/remote_object.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "math" + "regexp" "strconv" "strings" @@ -15,6 +16,8 @@ import ( "github.com/dop251/goja" ) +var bigIntR = regexp.MustCompile("^[0-9]*n$") + type objectOverflowError struct{} // Error returns the description of the overflow error. @@ -168,6 +171,14 @@ func parseRemoteObject(obj *cdpruntime.RemoteObject) (any, error) { return parseRemoteObjectValue(obj.Type, obj.Subtype, string(obj.Value), obj.Preview) } + if bigIntR.Match([]byte(obj.UnserializableValue)) { + n, err := strconv.ParseInt(strings.ReplaceAll(obj.UnserializableValue.String(), "n", ""), 10, 64) + if err != nil { + return nil, BigIntParseError{err} + } + return n, nil + } + switch obj.UnserializableValue.String() { case "-0": // To handle +0 divided by negative number return math.Float64frombits(0 | (1 << 63)), nil diff --git a/tests/remote_obj_test.go b/tests/remote_obj_test.go index be5ab9a68..0b6d92319 100644 --- a/tests/remote_obj_test.go +++ b/tests/remote_obj_test.go @@ -149,14 +149,12 @@ func TestEvalRemoteObjectParse(t *testing.T) { { name: "undefined", eval: "undefined", want: goja.Undefined(), }, - // { - // // Commented out until fix applied - // name: "bigint", eval: `BigInt("2")`, want: "2n", - // }, - // { - // // Commented out until fix applied - // name: "unwrapped_bigint", eval: "3n", want: "3n", - // }, + { + name: "bigint", eval: `BigInt("2")`, want: 2, + }, + { + name: "unwrapped_bigint", eval: "3n", want: 3, + }, { name: "float", eval: "3.14", want: 3.14, }, From a539d5bf78633600a7ae31256bc09c6006946e63 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Fri, 15 Dec 2023 09:27:15 +0000 Subject: [PATCH 3/4] Refactor variable names Resolves: https://github.com/grafana/xk6-browser/pull/1133#discussion_r1424931101 Resolves: https://github.com/grafana/xk6-browser/pull/1133#discussion_r1424931929 --- common/remote_object.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/common/remote_object.go b/common/remote_object.go index ade675032..73ceb84c0 100644 --- a/common/remote_object.go +++ b/common/remote_object.go @@ -16,7 +16,7 @@ import ( "github.com/dop251/goja" ) -var bigIntR = regexp.MustCompile("^[0-9]*n$") +var bigIntRegex = regexp.MustCompile("^[0-9]*n$") type objectOverflowError struct{} @@ -167,19 +167,21 @@ func parseExceptionDetails(exc *cdpruntime.ExceptionDetails) string { // parseRemoteObject is to be used by callers that require the string value // to be parsed to a Go type. func parseRemoteObject(obj *cdpruntime.RemoteObject) (any, error) { - if obj.UnserializableValue == "" { + uv := obj.UnserializableValue + + if uv == "" { return parseRemoteObjectValue(obj.Type, obj.Subtype, string(obj.Value), obj.Preview) } - if bigIntR.Match([]byte(obj.UnserializableValue)) { - n, err := strconv.ParseInt(strings.ReplaceAll(obj.UnserializableValue.String(), "n", ""), 10, 64) + if bigIntRegex.Match([]byte(uv)) { + n, err := strconv.ParseInt(strings.ReplaceAll(uv.String(), "n", ""), 10, 64) if err != nil { return nil, BigIntParseError{err} } return n, nil } - switch obj.UnserializableValue.String() { + switch uv.String() { case "-0": // To handle +0 divided by negative number return math.Float64frombits(0 | (1 << 63)), nil case "NaN": @@ -193,7 +195,7 @@ func parseRemoteObject(obj *cdpruntime.RemoteObject) (any, error) { // We should never get here, as previous switch statement should // be exhaustive and contain all possible unserializable values. // See: https://chromedevtools.github.io/devtools-protocol/tot/Runtime/#type-UnserializableValue - return nil, UnserializableValueError{obj.UnserializableValue} + return nil, UnserializableValueError{uv} } func valueFromRemoteObject(ctx context.Context, robj *cdpruntime.RemoteObject) (goja.Value, error) { From feafa26ef95c5cdf9ddc8ff90c9349098859fc23 Mon Sep 17 00:00:00 2001 From: Ankur Date: Fri, 15 Dec 2023 09:28:54 +0000 Subject: [PATCH 4/4] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: İnanç Gümüş --- tests/remote_obj_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/remote_obj_test.go b/tests/remote_obj_test.go index 0b6d92319..abbf20a0a 100644 --- a/tests/remote_obj_test.go +++ b/tests/remote_obj_test.go @@ -135,7 +135,7 @@ func TestEvalRemoteObjectParse(t *testing.T) { name: "filled_object", eval: `{return {foo:"bar"};}`, want: map[string]any{"foo": "bar"}, }, { - name: "filled_array", eval: `{return ["foo","bar"];}`, want: []interface{}{0: "foo", 1: "bar"}, + name: "filled_array", eval: `{return ["foo","bar"];}`, want: []any{0: "foo", 1: "bar"}, }, { name: "filled_array", eval: `() => true`, want: `function()`, @@ -161,6 +161,7 @@ func TestEvalRemoteObjectParse(t *testing.T) { { name: "scientific_notation", eval: "123e-5", want: 0.00123, }, + // TODO: // { // // This test is ignored until https://github.com/grafana/xk6-browser/issues/1132 // // has been resolved.