Skip to content

Commit

Permalink
Refactor timeout to time.Duration
Browse files Browse the repository at this point in the history
We should be working with time.Duration throughout the code base
instead of trying to work out what int64 means. This change aligns the
default timeout to this aim.
  • Loading branch information
ankur22 committed Sep 18, 2023
1 parent de293f2 commit b8e9be9
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 21 deletions.
2 changes: 1 addition & 1 deletion common/browser_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ func (b *BrowserContext) SetDefaultNavigationTimeout(timeout int64) {
func (b *BrowserContext) SetDefaultTimeout(timeout int64) {
b.logger.Debugf("BrowserContext:SetDefaultTimeout", "bctxid:%v timeout:%d", b.id, timeout)

b.timeoutSettings.setDefaultTimeout(timeout)
b.timeoutSettings.setDefaultTimeout(time.Duration(timeout) * time.Millisecond)
}

// SetExtraHTTPHeaders is not implemented.
Expand Down
2 changes: 1 addition & 1 deletion common/element_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func (h *ElementHandle) dblClick(p *Position, opts *MouseClickOptions) error {
}

func (h *ElementHandle) defaultTimeout() time.Duration {
return time.Duration(h.frame.manager.timeoutSettings.timeout()) * time.Millisecond
return h.frame.manager.timeoutSettings.timeout()
}

func (h *ElementHandle) dispatchEvent(_ context.Context, typ string, eventInit goja.Value) (any, error) {
Expand Down
4 changes: 2 additions & 2 deletions common/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func (f *Frame) detach() {
}

func (f *Frame) defaultTimeout() time.Duration {
return time.Duration(f.manager.timeoutSettings.timeout()) * time.Millisecond
return f.manager.timeoutSettings.timeout()
}

func (f *Frame) document() (*ElementHandle, error) {
Expand Down Expand Up @@ -1738,7 +1738,7 @@ func (f *Frame) WaitForNavigation(opts goja.Value) (api.Response, error) {
"fid:%s furl:%s", f.ID(), f.URL())

parsedOpts := NewFrameWaitForNavigationOptions(
time.Duration(f.manager.timeoutSettings.timeout()) * time.Millisecond)
f.manager.timeoutSettings.timeout())
if err := parsedOpts.Parse(f.ctx, opts); err != nil {
k6ext.Panic(f.ctx, "parsing wait for navigation options: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions common/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func (p *Page) closeWorker(sessionID target.SessionID) {
}

func (p *Page) defaultTimeout() time.Duration {
return time.Duration(p.timeoutSettings.timeout()) * time.Millisecond
return p.timeoutSettings.timeout()
}

func (p *Page) didClose() {
Expand Down Expand Up @@ -975,7 +975,7 @@ func (p *Page) SetDefaultNavigationTimeout(timeout int64) {
func (p *Page) SetDefaultTimeout(timeout int64) {
p.logger.Debugf("Page:SetDefaultTimeout", "sid:%v timeout:%d", p.sessionID(), timeout)

p.timeoutSettings.setDefaultTimeout(timeout)
p.timeoutSettings.setDefaultTimeout(time.Duration(timeout) * time.Millisecond)
}

// SetExtraHTTPHeaders sets default HTTP headers for page and whole frame hierarchy.
Expand Down
10 changes: 5 additions & 5 deletions common/timeout.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import "time"
// TimeoutSettings holds information on timeout settings.
type TimeoutSettings struct {
parent *TimeoutSettings
defaultTimeout *int64
defaultTimeout *time.Duration
defaultNavigationTimeout *time.Duration
}

Expand All @@ -19,7 +19,7 @@ func NewTimeoutSettings(parent *TimeoutSettings) *TimeoutSettings {
return t
}

func (t *TimeoutSettings) setDefaultTimeout(timeout int64) {
func (t *TimeoutSettings) setDefaultTimeout(timeout time.Duration) {
t.defaultTimeout = &timeout
}

Expand All @@ -32,20 +32,20 @@ func (t *TimeoutSettings) navigationTimeout() time.Duration {
return *t.defaultNavigationTimeout
}
if t.defaultTimeout != nil {
return time.Duration(*t.defaultTimeout * time.Hour.Milliseconds())
return *t.defaultTimeout
}
if t.parent != nil {
return t.parent.navigationTimeout()
}
return DefaultTimeout
}

func (t *TimeoutSettings) timeout() int64 {
func (t *TimeoutSettings) timeout() time.Duration {
if t.defaultTimeout != nil {
return *t.defaultTimeout
}
if t.parent != nil {
return t.parent.timeout()
}
return int64(DefaultTimeout.Milliseconds())
return DefaultTimeout
}
20 changes: 10 additions & 10 deletions common/timeout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ func testTimeoutSettingsSetDefaultTimeout(t *testing.T) {
t.Parallel()

ts := NewTimeoutSettings(nil)
ts.setDefaultTimeout(100)
assert.Equal(t, int64(100), *ts.defaultTimeout)
ts.setDefaultTimeout(time.Duration(100) * time.Millisecond)
assert.Equal(t, int64(100), ts.defaultTimeout.Milliseconds())
}

func testTimeoutSettingsSetDefaultNavigationTimeout(t *testing.T) {
Expand Down Expand Up @@ -112,11 +112,11 @@ func testTimeoutSettingsTimeout(t *testing.T) {
ts := NewTimeoutSettings(nil)

// Assert default timeout value is used
assert.Equal(t, int64(DefaultTimeout.Milliseconds()), ts.timeout())
assert.Equal(t, DefaultTimeout, ts.timeout())

// Assert custom default timeout is used
ts.setDefaultTimeout(100)
assert.Equal(t, int64(100), ts.timeout())
ts.setDefaultTimeout(time.Duration(100) * time.Millisecond)
assert.Equal(t, int64(100), ts.timeout().Milliseconds())
}

func testTimeoutSettingsTimeoutWithParent(t *testing.T) {
Expand All @@ -126,13 +126,13 @@ func testTimeoutSettingsTimeoutWithParent(t *testing.T) {
tsWithParent := NewTimeoutSettings(ts)

// Assert default timeout value is used
assert.Equal(t, int64(DefaultTimeout.Milliseconds()), tsWithParent.timeout())
assert.Equal(t, DefaultTimeout, tsWithParent.timeout())

// Assert custom default timeout from parent is used
ts.setDefaultTimeout(1000)
assert.Equal(t, int64(1000), tsWithParent.timeout())
ts.setDefaultTimeout(time.Duration(1000) * time.Millisecond)
assert.Equal(t, int64(1000), tsWithParent.timeout().Milliseconds())

// Assert custom default timeout is used (over parent)
tsWithParent.setDefaultTimeout(100)
assert.Equal(t, int64(100), tsWithParent.timeout())
tsWithParent.setDefaultTimeout(time.Duration(100) * time.Millisecond)
assert.Equal(t, int64(100), tsWithParent.timeout().Milliseconds())
}

0 comments on commit b8e9be9

Please sign in to comment.