Skip to content

Commit

Permalink
Fix unmap internal methods (#783)
Browse files Browse the repository at this point in the history
We shouldn't expose some of the methods as they are internal to the
extension, have no meaning for the users, and Playwright doesn't support
them. The refactoring to the test should be needed to be done with these
together to not to break the tests.
  • Loading branch information
inancgumus committed Feb 21, 2023
1 parent 26d0b83 commit c1a3c20
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 7 deletions.
3 changes: 0 additions & 3 deletions browser/mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ func mapJSHandle(vu moduleVU, jsh api.JSHandle) mapping {
return rt.ToValue(m).ToObject(rt)
},
"jsonValue": jsh.JSONValue,
"objectID": jsh.ObjectID,
}
}

Expand Down Expand Up @@ -315,8 +314,6 @@ func mapFrame(vu moduleVU, f api.Frame) mapping {
"isEnabled": f.IsEnabled,
"isHidden": f.IsHidden,
"isVisible": f.IsVisible,
"iD": f.ID,
"loaderID": f.LoaderID,
"locator": f.Locator,
"name": f.Name,
"page": func() *goja.Object {
Expand Down
22 changes: 18 additions & 4 deletions browser/mapping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ func customMappings() map[string]string {
"Page.getKeyboard": "keyboard",
"Page.getMouse": "mouse",
"Page.getTouchscreen": "touchscreen",
// internal methods
"ElementHandle.objectID": "",
"Frame.id": "",
"Frame.loaderID": "",
"JSHandle.objectID": "",
}
}

Expand Down Expand Up @@ -67,19 +72,27 @@ func TestMappings(t *testing.T) {
mapped = tt.mapp()
)
for i := 0; i < typ.NumMethod(); i++ {
method := typ.Method(i)
var (
method = typ.Method(i)
typName = typ.Name()
)
require.NotNil(t, method)

// goja uses methods that starts with lowercase.
// so we need to convert the first letter to lowercase.
m := toFirstLetterLower(method.Name)

cm, cmok := isCustomMapping(customMappings, typ.Name(), m)
cm, cmok := isCustomMapping(customMappings, typName, m)
// if the method is a custom mapping, it should not be
// mapped to the module. so we should not find it in
// the mapped methods.
if _, ok := mapped[m]; cmok && ok {
t.Errorf("method %s should not be mapped", m)
t.Errorf("method %s should not be mapped for %s", m, typName)
}
// a custom mapping with an empty string means that
// the method should not exist on the API.
if cmok && cm == "" {
continue
}
// change the method name if it is mapped to a custom
// method. these custom methods are not exist on our
Expand All @@ -88,7 +101,7 @@ func TestMappings(t *testing.T) {
m = cm
}
if _, ok := mapped[m]; !ok {
t.Errorf("method %s not found", m)
t.Errorf("method %s for %s not found", m, typName)
}
}
}
Expand Down Expand Up @@ -173,6 +186,7 @@ func toFirstLetterLower(s string) string {
// Instead of loading up an acronyms list, just do this.
// Good enough for our purposes.
special := map[string]string{
"ID": "id",
"JSON": "json",
"JSONValue": "jsonValue",
"URL": "url",
Expand Down

0 comments on commit c1a3c20

Please sign in to comment.