Skip to content
This repository was archived by the owner on Jan 30, 2025. It is now read-only.

Improve GetAttribute null attribute detection #1370

Merged
merged 4 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion browser/element_handle_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,14 @@ func mapElementHandle(vu moduleVU, eh *common.ElementHandle) mapping { //nolint:
},
"getAttribute": func(name string) *goja.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
return eh.GetAttribute(name) //nolint:wrapcheck
s, ok, err := eh.GetAttribute(name)
if err != nil {
return nil, err //nolint:wrapcheck
}
if !ok {
return nil, nil
}
return s, nil
})
},
"hover": func(opts goja.Value) *goja.Promise {
Expand Down
9 changes: 8 additions & 1 deletion browser/frame_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,14 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping { //nolint:gocognit,cyclop
},
"getAttribute": func(selector, name string, opts goja.Value) *goja.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
return f.GetAttribute(selector, name, opts) //nolint:wrapcheck
s, ok, err := f.GetAttribute(selector, name, opts)
if err != nil {
return nil, err //nolint:wrapcheck
}
if !ok {
return nil, nil
}
return s, nil
})
},
"goto": func(url string, opts goja.Value) (*goja.Promise, error) {
Expand Down
9 changes: 8 additions & 1 deletion browser/locator_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,14 @@ func mapLocator(vu moduleVU, lo *common.Locator) mapping { //nolint:funlen
},
"getAttribute": func(name string, opts goja.Value) *goja.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
return lo.GetAttribute(name, opts) //nolint:wrapcheck
s, ok, err := lo.GetAttribute(name, opts)
if err != nil {
return nil, err //nolint:wrapcheck
}
if !ok {
return nil, nil
}
return s, nil
})
},
"innerHTML": func(opts goja.Value) *goja.Promise {
Expand Down
8 changes: 4 additions & 4 deletions browser/mapping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ type pageAPI interface {
Fill(selector string, value string, opts goja.Value) error
Focus(selector string, opts goja.Value) error
Frames() []*common.Frame
GetAttribute(selector string, name string, opts goja.Value) (any, error)
GetAttribute(selector string, name string, opts goja.Value) (string, bool, error)
GetKeyboard() *common.Keyboard
GetMouse() *common.Mouse
GetTouchscreen() *common.Touchscreen
Expand Down Expand Up @@ -380,7 +380,7 @@ type frameAPI interface {
Fill(selector string, value string, opts goja.Value) error
Focus(selector string, opts goja.Value) error
FrameElement() (*common.ElementHandle, error)
GetAttribute(selector string, name string, opts goja.Value) (any, error)
GetAttribute(selector string, name string, opts goja.Value) (string, bool, error)
Goto(url string, opts goja.Value) (*common.Response, error)
Hover(selector string, opts goja.Value) error
InnerHTML(selector string, opts goja.Value) (string, error)
Expand Down Expand Up @@ -430,7 +430,7 @@ type elementHandleAPI interface {
DispatchEvent(typ string, props goja.Value) error
Fill(value string, opts goja.Value) error
Focus() error
GetAttribute(name string) (any, error)
GetAttribute(name string) (string, bool, error)
Hover(opts goja.Value) error
InnerHTML() (string, error)
InnerText() (string, error)
Expand Down Expand Up @@ -512,7 +512,7 @@ type locatorAPI interface {
IsHidden(opts goja.Value) (bool, error)
Fill(value string, opts goja.Value) error
Focus(opts goja.Value) error
GetAttribute(name string, opts goja.Value) (any, error)
GetAttribute(name string, opts goja.Value) (string, bool, error)
InnerHTML(opts goja.Value) (string, error)
InnerText(opts goja.Value) (string, error)
TextContent(opts goja.Value) (string, error)
Expand Down
9 changes: 8 additions & 1 deletion browser/page_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,14 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
},
"getAttribute": func(selector string, name string, opts goja.Value) *goja.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
return p.GetAttribute(selector, name, opts) //nolint:wrapcheck
s, ok, err := p.GetAttribute(selector, name, opts)
if err != nil {
return nil, err //nolint:wrapcheck
}
if !ok {
return nil, nil
}
return s, nil
})
},
"goto": func(url string, opts goja.Value) (*goja.Promise, error) {
Expand Down
19 changes: 14 additions & 5 deletions common/element_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,8 @@ func (h *ElementHandle) Focus() error {
}

// GetAttribute retrieves the value of specified element attribute.
func (h *ElementHandle) GetAttribute(name string) (any, error) {
// The second return value is true if the attribute exists, and false otherwise.
func (h *ElementHandle) GetAttribute(name string) (string, bool, error) {
getAttribute := func(apiCtx context.Context, handle *ElementHandle) (any, error) {
return handle.getAttribute(apiCtx, name)
}
Expand All @@ -851,12 +852,20 @@ func (h *ElementHandle) GetAttribute(name string) (any, error) {

v, err := call(h.ctx, getAttributeAction, opts.Timeout)
if err != nil {
return nil, fmt.Errorf("getting attribute %q of element: %w", name, err)
return "", false, fmt.Errorf("getting attribute %q of element: %w", name, err)
}
if v == nil {
return "", false, nil
}
s, ok := v.(string)
if !ok {
return "", false, fmt.Errorf(
"getting attribute %q of element: unexpected type %T (expecting string)",
name, v,
)
}

applySlowMo(h.ctx)

return v, nil
return s, true, nil
}

// Hover scrolls element into view and hovers over its center point.
Expand Down
24 changes: 16 additions & 8 deletions common/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -960,22 +960,23 @@ func (f *Frame) FrameElement() (*ElementHandle, error) {
}

// GetAttribute of the first element found that matches the selector.
func (f *Frame) GetAttribute(selector, name string, opts goja.Value) (any, error) {
// The second return value is true if the attribute exists, and false otherwise.
func (f *Frame) GetAttribute(selector, name string, opts goja.Value) (string, bool, error) {
f.log.Debugf("Frame:GetAttribute", "fid:%s furl:%q sel:%q name:%s", f.ID(), f.URL(), selector, name)

popts := NewFrameBaseOptions(f.defaultTimeout())
if err := popts.Parse(f.ctx, opts); err != nil {
return nil, fmt.Errorf("parsing get attribute options: %w", err)
return "", false, fmt.Errorf("parsing get attribute options: %w", err)
}
v, err := f.getAttribute(selector, name, popts)
s, ok, err := f.getAttribute(selector, name, popts)
if err != nil {
return nil, fmt.Errorf("getting attribute %q of %q: %w", name, selector, err)
return "", false, fmt.Errorf("getting attribute %q of %q: %w", name, selector, err)
}

return v, nil
return s, ok, nil
}

func (f *Frame) getAttribute(selector, name string, opts *FrameBaseOptions) (any, error) {
func (f *Frame) getAttribute(selector, name string, opts *FrameBaseOptions) (string, bool, error) {
getAttribute := func(apiCtx context.Context, handle *ElementHandle) (any, error) {
return handle.getAttribute(apiCtx, name)
}
Expand All @@ -985,10 +986,17 @@ func (f *Frame) getAttribute(selector, name string, opts *FrameBaseOptions) (any
)
v, err := call(f.ctx, act, opts.Timeout)
if err != nil {
return "", errorFromDOMError(err)
return "", false, errorFromDOMError(err)
}
if v == nil {
return "", false, nil
}
s, ok := v.(string)
if !ok {
return "", false, fmt.Errorf("unexpected type %T (expecting string)", v)
}

return v, nil
return s, true, nil
}

// Referrer returns the referrer of the frame from the network manager
Expand Down
13 changes: 7 additions & 6 deletions common/locator.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,25 +322,26 @@ func (l *Locator) focus(opts *FrameBaseOptions) error {
}

// GetAttribute of the element using locator's selector with strict mode on.
func (l *Locator) GetAttribute(name string, opts goja.Value) (any, error) {
// The second return value is true if the attribute exists, and false otherwise.
func (l *Locator) GetAttribute(name string, opts goja.Value) (string, bool, error) {
l.log.Debugf(
"Locator:GetAttribute", "fid:%s furl:%q sel:%q name:%q opts:%+v",
l.frame.ID(), l.frame.URL(), l.selector, name, opts,
)

copts := NewFrameBaseOptions(l.frame.defaultTimeout())
if err := copts.Parse(l.ctx, opts); err != nil {
return nil, fmt.Errorf("parsing get attribute options: %w", err)
return "", false, fmt.Errorf("parsing get attribute options: %w", err)
}
v, err := l.getAttribute(name, copts)
s, ok, err := l.getAttribute(name, copts)
if err != nil {
return nil, fmt.Errorf("getting attribute %q of %q: %w", name, l.selector, err)
return "", false, fmt.Errorf("getting attribute %q of %q: %w", name, l.selector, err)
}

return v, nil
return s, ok, nil
}

func (l *Locator) getAttribute(name string, opts *FrameBaseOptions) (any, error) {
func (l *Locator) getAttribute(name string, opts *FrameBaseOptions) (string, bool, error) {
opts.Strict = true
return l.frame.getAttribute(l.selector, name, opts)
}
Expand Down
3 changes: 2 additions & 1 deletion common/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,8 @@ func (p *Page) Frames() []*Frame {
}

// GetAttribute returns the attribute value of the element matching the provided selector.
func (p *Page) GetAttribute(selector string, name string, opts goja.Value) (any, error) {
// The second return value is true if the attribute exists, and false otherwise.
func (p *Page) GetAttribute(selector string, name string, opts goja.Value) (string, bool, error) {
p.logger.Debugf("Page:GetAttribute", "sid:%v selector:%s name:%s",
p.sessionID(), selector, name)

Expand Down
43 changes: 36 additions & 7 deletions tests/element_handle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,20 +248,49 @@ func TestElementHandleNonClickable(t *testing.T) {
func TestElementHandleGetAttribute(t *testing.T) {
t.Parallel()

const want = "https://somewhere"
p := newTestBrowser(t).NewPage(nil)
err := p.SetContent(`<a id="el" href="null">Something</a>`, nil)
require.NoError(t, err)

el, err := p.Query("#el")
require.NoError(t, err)

got, ok, err := el.GetAttribute("href")
require.NoError(t, err)
require.True(t, ok)
assert.Equal(t, "null", got)
}

func TestElementHandleGetAttributeMissing(t *testing.T) {
t.Parallel()

p := newTestBrowser(t).NewPage(nil)
err := p.SetContent(`
<a id="dark-mode-toggle-X" href="https://somewhere">Dark</a>
`, nil)
err := p.SetContent(`<a id="el">Something</a>`, nil)
require.NoError(t, err)

el, err := p.Query("#el")
require.NoError(t, err)

got, ok, err := el.GetAttribute("missing")
require.NoError(t, err)
require.False(t, ok)
assert.Equal(t, "", got)
}

func TestElementHandleGetAttributeEmpty(t *testing.T) {
t.Parallel()

p := newTestBrowser(t).NewPage(nil)
err := p.SetContent(`<a id="el" empty>Something</a>`, nil)
require.NoError(t, err)

el, err := p.Query("#dark-mode-toggle-X")
el, err := p.Query("#el")
require.NoError(t, err)

got, err := el.GetAttribute("href")
got, ok, err := el.GetAttribute("empty")
require.NoError(t, err)
assert.Equal(t, want, got)
require.True(t, ok)
assert.Equal(t, "", got)
}

func TestElementHandleInputValue(t *testing.T) {
Expand Down
39 changes: 39 additions & 0 deletions tests/frame_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,42 @@ func TestFrameTitle(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "Some title", p.MainFrame().Title())
}

func TestFrameGetAttribute(t *testing.T) {
t.Parallel()

p := newTestBrowser(t).NewPage(nil)
err := p.SetContent(`<a id="el" href="null">Something</a>`, nil)
require.NoError(t, err)

got, ok, err := p.Frames()[0].GetAttribute("#el", "href", nil)
require.NoError(t, err)
require.True(t, ok)
assert.Equal(t, "null", got)
}

func TestFrameGetAttributeMissing(t *testing.T) {
t.Parallel()

p := newTestBrowser(t).NewPage(nil)
err := p.SetContent(`<a id="el">Something</a>`, nil)
require.NoError(t, err)

got, ok, err := p.Frames()[0].GetAttribute("#el", "missing", nil)
require.NoError(t, err)
require.False(t, ok)
assert.Equal(t, "", got)
}

func TestFrameGetAttributeEmpty(t *testing.T) {
t.Parallel()

p := newTestBrowser(t).NewPage(nil)
err := p.SetContent(`<a id="el" empty>Something</a>`, nil)
require.NoError(t, err)

got, ok, err := p.Frames()[0].GetAttribute("#el", "empty", nil)
require.NoError(t, err)
require.True(t, ok)
assert.Equal(t, "", got)
}
5 changes: 3 additions & 2 deletions tests/locator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,10 @@ func TestLocator(t *testing.T) {
{
"GetAttribute", func(_ *testBrowser, p *common.Page) {
l := p.Locator("#inputText", nil)
v, err := l.GetAttribute("value", nil)
v, ok, err := l.GetAttribute("value", nil)
require.NoError(t, err)
require.NotNil(t, v)
require.True(t, ok)
require.Equal(t, "something", v)
},
},
Expand Down Expand Up @@ -366,7 +367,7 @@ func TestLocator(t *testing.T) {
},
{
"GetAttribute", func(l *common.Locator, tb *testBrowser) error {
_, err := l.GetAttribute("value", timeout(tb))
_, _, err := l.GetAttribute("value", timeout(tb))
return err
},
},
Expand Down
39 changes: 39 additions & 0 deletions tests/page_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1806,3 +1806,42 @@ func TestPageTargetBlank(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "you clicked!", got)
}

func TestPageGetAttribute(t *testing.T) {
t.Parallel()

p := newTestBrowser(t).NewPage(nil)
err := p.SetContent(`<a id="el" href="null">Something</a>`, nil)
require.NoError(t, err)

got, ok, err := p.GetAttribute("#el", "href", nil)
require.NoError(t, err)
require.True(t, ok)
assert.Equal(t, "null", got)
}

func TestPageGetAttributeMissing(t *testing.T) {
t.Parallel()

p := newTestBrowser(t).NewPage(nil)
err := p.SetContent(`<a id="el">Something</a>`, nil)
require.NoError(t, err)

got, ok, err := p.GetAttribute("#el", "missing", nil)
require.NoError(t, err)
require.False(t, ok)
assert.Equal(t, "", got)
}

func TestPageGetAttributeEmpty(t *testing.T) {
t.Parallel()

p := newTestBrowser(t).NewPage(nil)
err := p.SetContent(`<a id="el" empty>Something</a>`, nil)
require.NoError(t, err)

got, ok, err := p.GetAttribute("#el", "empty", nil)
require.NoError(t, err)
require.True(t, ok)
assert.Equal(t, "", got)
}
Loading