Skip to content

Commit

Permalink
Unify duration configuration values
Browse files Browse the repository at this point in the history
  • Loading branch information
na-- committed Dec 1, 2020
1 parent 7d3809a commit ddee5dd
Show file tree
Hide file tree
Showing 10 changed files with 188 additions and 47 deletions.
2 changes: 2 additions & 0 deletions cmd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ func getNullInt64(flags *pflag.FlagSet, key string) null.Int {
}

func getNullDuration(flags *pflag.FlagSet, key string) types.NullDuration {
// TODO: use types.ParseExtendedDuration? not sure we should support
// unitless durations (i.e. milliseconds) here...
v, err := flags.GetDuration(key)
if err != nil {
panic(err)
Expand Down
29 changes: 28 additions & 1 deletion cmd/config_consolidation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,11 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase {
// Verify some CLI errors
{opts{cli: []string{"--blah", "blah"}}, exp{cliParseError: true}, nil},
{opts{cli: []string{"--duration", "blah"}}, exp{cliParseError: true}, nil},
{opts{cli: []string{"--duration", "1000"}}, exp{cliParseError: true}, nil}, // intentionally unsupported
{opts{cli: []string{"--iterations", "blah"}}, exp{cliParseError: true}, nil},
{opts{cli: []string{"--execution", ""}}, exp{cliParseError: true}, nil},
{opts{cli: []string{"--stage", "10:20s"}}, exp{cliReadError: true}, nil},
{opts{cli: []string{"--stage", "1000:20"}}, exp{cliReadError: true}, nil}, // intentionally unsupported
// Check if CLI shortcuts generate correct execution values
{opts{cli: []string{"--vus", "1", "--iterations", "5"}}, exp{}, verifySharedIters(I(1), I(5))},
{opts{cli: []string{"-u", "2", "-i", "6"}}, exp{}, verifySharedIters(I(2), I(6))},
Expand Down Expand Up @@ -249,6 +251,7 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase {
// Test if environment variable shortcuts are working as expected
{opts{env: []string{"K6_VUS=5", "K6_ITERATIONS=15"}}, exp{}, verifySharedIters(I(5), I(15))},
{opts{env: []string{"K6_VUS=10", "K6_DURATION=20s"}}, exp{}, verifyConstLoopingVUs(I(10), 20*time.Second)},
{opts{env: []string{"K6_VUS=10", "K6_DURATION=10000"}}, exp{}, verifyConstLoopingVUs(I(10), 10*time.Second)},
{
opts{env: []string{"K6_STAGES=2m30s:11,1h1m:100"}},
exp{},
Expand All @@ -259,6 +262,7 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase {
exp{},
verifyRampingVUs(null.NewInt(0, true), buildStages(100, 100, 30, 0)),
},
{opts{env: []string{"K6_STAGES=1000:100"}}, exp{consolidationError: true}, nil}, // intentionally unsupported
// Test if JSON configs work as expected
{opts{fs: defaultConfig(`{"iterations": 77, "vus": 7}`)}, exp{}, verifySharedIters(I(7), I(77))},
{opts{fs: defaultConfig(`wrong-json`)}, exp{consolidationError: true}, nil},
Expand All @@ -273,6 +277,12 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase {
cli: []string{"--config", "/my/config.file"},
}, exp{}, verifyConstLoopingVUs(I(8), 120*time.Second),
},
{
opts{
fs: getFS([]file{{"/my/config.file", `{"duration": 20000}`}}),
cli: []string{"--config", "/my/config.file"},
}, exp{}, verifyConstLoopingVUs(null.NewInt(1, false), 20*time.Second),
},
{
opts{
fs: defaultConfig(`{"stages": [{"duration": "20s", "target": 20}], "vus": 10}`),
Expand Down Expand Up @@ -332,7 +342,24 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase {
exp{},
verifySharedIters(I(12), I(25)),
},

{
opts{
fs: defaultConfig(`{"scenarios": { "foo": {
"executor": "constant-vus", "vus": 2, "duration": "1d",
"gracefulStop": "10000", "startTime": 1000.5
}}}`),
}, exp{}, func(t *testing.T, c Config) {
exec := c.Scenarios["foo"]
require.NotEmpty(t, exec)
require.IsType(t, executor.ConstantVUsConfig{}, exec)
clvc, ok := exec.(executor.ConstantVUsConfig)
require.True(t, ok)
assert.Equal(t, null.IntFrom(2), clvc.VUs)
assert.Equal(t, types.NullDurationFrom(24*time.Hour), clvc.Duration)
assert.Equal(t, types.NullDurationFrom(time.Second+500*time.Microsecond), clvc.StartTime)
assert.Equal(t, types.NullDurationFrom(10*time.Second), clvc.GracefulStop)
},
},
// TODO: test the externally controlled executor
// TODO: test execution-segment

Expand Down
29 changes: 7 additions & 22 deletions js/modules/k6/grpc/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import (
"github.com/loadimpact/k6/js/common"
"github.com/loadimpact/k6/lib"
"github.com/loadimpact/k6/lib/metrics"
"github.com/loadimpact/k6/lib/types"
"github.com/loadimpact/k6/stats"
)

Expand Down Expand Up @@ -220,17 +221,9 @@ func (c *Client) Connect(ctxPtr *context.Context, addr string, params map[string
isPlaintext, _ = v.(bool)
case "timeout":
var err error
switch t := v.(type) {
case string:
if timeout, err = time.ParseDuration(t); err != nil {
return false, fmt.Errorf("unable to parse %q: %v", k, err)
}
case int64:
timeout = time.Duration(t) * time.Millisecond
case float64:
timeout = time.Duration(t * float64(time.Millisecond))
default:
return false, fmt.Errorf("unable to use type %T as a timeout value", v)
timeout, err = types.GetDurationValue(v)
if err != nil {
return false, fmt.Errorf("invalid timeout value: %w", err)
}
default:
return false, fmt.Errorf("unknown connect param: %q", k)
Expand Down Expand Up @@ -359,17 +352,9 @@ func (c *Client) Invoke(ctxPtr *context.Context,
}
case "timeout":
var err error
switch t := v.(type) {
case string:
if timeout, err = time.ParseDuration(t); err != nil {
return nil, fmt.Errorf("unable to parse %q: %v", k, err)
}
case int64:
timeout = time.Duration(t) * time.Millisecond
case float64:
timeout = time.Duration(t * float64(time.Millisecond))
default:
return nil, fmt.Errorf("unable to use type %T as a timeout value", v)
timeout, err = types.GetDurationValue(v)
if err != nil {
return nil, fmt.Errorf("invalid timeout value: %w", err)
}
default:
return nil, fmt.Errorf("unknown param: %q", k)
Expand Down
6 changes: 3 additions & 3 deletions js/modules/k6/grpc/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func TestClient(t *testing.T) {
if !assert.Error(t, err) {
return
}
assert.Contains(t, err.Error(), "unable to parse \"timeout\"")
assert.Contains(t, err.Error(), "invalid duration")
})

t.Run("ConnectStringTimeout", func(t *testing.T) {
Expand Down Expand Up @@ -258,7 +258,7 @@ func TestClient(t *testing.T) {
if !assert.Error(t, err) {
return
}
assert.Contains(t, err.Error(), "unable to use type bool as a timeout value")
assert.Contains(t, err.Error(), "invalid timeout value: unable to use type bool as a duration value")
})

t.Run("InvokeInvalidTimeout", func(t *testing.T) {
Expand All @@ -268,7 +268,7 @@ func TestClient(t *testing.T) {
if !assert.Error(t, err) {
return
}
assert.Contains(t, err.Error(), " unable to parse \"timeout\"")
assert.Contains(t, err.Error(), "invalid duration")
})

t.Run("InvokeStringTimeout", func(t *testing.T) {
Expand Down
7 changes: 6 additions & 1 deletion js/modules/k6/http/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"github.com/loadimpact/k6/js/common"
"github.com/loadimpact/k6/lib"
"github.com/loadimpact/k6/lib/netext/httpext"
"github.com/loadimpact/k6/lib/types"
)

// ErrHTTPForbiddenInInitContext is used when a http requests was made in the init context
Expand Down Expand Up @@ -335,7 +336,11 @@ func (h *HTTP) parseRequest(
case "auth":
result.Auth = params.Get(k).String()
case "timeout":
result.Timeout = time.Duration(params.Get(k).ToFloat() * float64(time.Millisecond))
t, err := types.GetDurationValue(params.Get(k).Export())
if err != nil {
return nil, fmt.Errorf("invalid timeout value: %w", err)
}
result.Timeout = t
case "throw":
result.Throw = params.Get(k).ToBoolean()
case "responseType":
Expand Down
18 changes: 18 additions & 0 deletions js/modules/k6/http/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,24 @@ func TestRequestAndBatch(t *testing.T) {
assert.Contains(t, err.Error(), "context deadline exceeded")
assert.WithinDuration(t, startTime.Add(1*time.Second), endTime, 2*time.Second)

logEntry := hook.LastEntry()
assert.Nil(t, logEntry)
})
t.Run("10s", func(t *testing.T) {
hook := logtest.NewLocal(state.Logger)
defer hook.Reset()

startTime := time.Now()
_, err := common.RunString(rt, sr(`
http.get("HTTPBIN_URL/delay/10", {
timeout: "1s",
})
`))
endTime := time.Now()
require.Error(t, err)
assert.Contains(t, err.Error(), "context deadline exceeded")
assert.WithinDuration(t, startTime.Add(1*time.Second), endTime, 2*time.Second)

logEntry := hook.LastEntry()
assert.Nil(t, logEntry)
})
Expand Down
10 changes: 8 additions & 2 deletions js/modules/k6/ws/ws.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,10 @@ func (s *Socket) trackPong(pingID string) {
// timeout, which is in ms, has elapsed
func (s *Socket) SetTimeout(fn goja.Callable, timeoutMs float64) error {
// Starts a goroutine, blocks once on the timeout and pushes the callable
// back to the main loop through the scheduled channel
// back to the main loop through the scheduled channel.
//
// Intentionally not using the generic GetDurationValue() helper, since this
// API is meant to use ms, similar to the original SetTimeout() JS API.
d := time.Duration(timeoutMs * float64(time.Millisecond))
if d <= 0 {
return fmt.Errorf("setTimeout requires a >0 timeout parameter, received %.2f", timeoutMs)
Expand All @@ -416,7 +419,10 @@ func (s *Socket) SetTimeout(fn goja.Callable, timeoutMs float64) error {
// in ms
func (s *Socket) SetInterval(fn goja.Callable, intervalMs float64) error {
// Starts a goroutine, blocks forever on the ticker and pushes the callable
// back to the main loop through the scheduled channel
// back to the main loop through the scheduled channel.
//
// Intentionally not using the generic GetDurationValue() helper, since this
// API is meant to use ms, similar to the original SetInterval() JS API.
d := time.Duration(intervalMs * float64(time.Millisecond))
if d <= 0 {
return fmt.Errorf("setInterval requires a >0 timeout parameter, received %.2f", intervalMs)
Expand Down
2 changes: 1 addition & 1 deletion js/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ func parseTTL(ttlS string) (time.Duration, error) {
fallthrough
default:
var err error
ttl, err = types.ParseExtendedDurationMs(ttlS)
ttl, err = types.ParseExtendedDuration(ttlS)
if ttl < 0 || err != nil {
return ttl, fmt.Errorf("invalid DNS TTL: %s", ttlS)
}
Expand Down
77 changes: 62 additions & 15 deletions lib/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"bytes"
"encoding/json"
"fmt"
"math"
"reflect"
"strconv"
"strings"
Expand Down Expand Up @@ -97,6 +98,11 @@ func (d Duration) String() string {
// ParseExtendedDuration is a helper function that allows for string duration
// values containing days.
func ParseExtendedDuration(data string) (result time.Duration, err error) {
// Assume millisecond values if data is provided with no units
if t, errp := strconv.ParseFloat(data, 64); errp == nil {
return time.Duration(t * float64(time.Millisecond)), nil
}

dPos := strings.IndexByte(data, 'd')
if dPos < 0 {
return time.ParseDuration(data)
Expand All @@ -123,16 +129,6 @@ func ParseExtendedDuration(data string) (result time.Duration, err error) {
return time.Duration(days)*24*time.Hour + hours, nil
}

// ParseExtendedDurationMs wraps ParseExtendedDuration while assuming
// millisecond values if data is provided with no units.
// TODO: Merge this into ParseExtendedDuration once it's safe to do so globally.
func ParseExtendedDurationMs(data string) (result time.Duration, err error) {
if t, errp := strconv.ParseFloat(data, 32); errp == nil {
data = fmt.Sprintf("%.2fms", t)
}
return ParseExtendedDuration(data)
}

// UnmarshalText converts text data to Duration
func (d *Duration) UnmarshalText(data []byte) error {
v, err := ParseExtendedDuration(string(data))
Expand All @@ -157,12 +153,10 @@ func (d *Duration) UnmarshalJSON(data []byte) error {
}

*d = Duration(v)
} else if t, errp := strconv.ParseFloat(string(data), 64); errp == nil {
*d = Duration(t * float64(time.Millisecond))
} else {
var v time.Duration
if err := json.Unmarshal(data, &v); err != nil {
return err
}
*d = Duration(v)
return fmt.Errorf("'%s' is not a valid duration value", string(data))
}

return nil
Expand Down Expand Up @@ -233,3 +227,56 @@ func (d NullDuration) ValueOrZero() Duration {

return d.Duration
}

func getInt64(v interface{}) (int64, error) {
switch n := v.(type) {
case int:
return int64(n), nil
case int8:
return int64(n), nil
case int16:
return int64(n), nil
case int32:
return int64(n), nil
case int64:
return n, nil
case uint:
return int64(n), nil
case uint8:
return int64(n), nil
case uint16:
return int64(n), nil
case uint32:
return int64(n), nil
case uint64:
if n > math.MaxInt64 {
return 0, fmt.Errorf("%d is too big", n)
}
return int64(n), nil
default:
return 0, fmt.Errorf("unable to use type %T as a duration value", v)
}
}

// GetDurationValue is a helper function that can convert a lot of different
// types to time.Duration.
//
// TODO: move to a separate package and check for integer overflows?
func GetDurationValue(v interface{}) (time.Duration, error) {
switch d := v.(type) {
case time.Duration:
return d, nil
case string:
return ParseExtendedDuration(d)
case float32:
return time.Duration(float64(d) * float64(time.Millisecond)), nil
case float64:
return time.Duration(d * float64(time.Millisecond)), nil
default:
n, err := getInt64(v)
if err != nil {
return 0, err
}
return time.Duration(n) * time.Millisecond, nil
}
}

0 comments on commit ddee5dd

Please sign in to comment.