From 3f8d36d3c0da39061597eb0d2e93b8ae6dda536c Mon Sep 17 00:00:00 2001 From: John Eikenberry Date: Fri, 15 Jan 2021 17:49:53 -0800 Subject: [PATCH 1/3] add test for block_query_wait setting not taking --- config/config_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/config/config_test.go b/config/config_test.go index 6eb608a1b..9b00e0cff 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1798,6 +1798,18 @@ func TestConfig_Merge(t *testing.T) { BlockQueryWaitTime: TimeDuration(60 * time.Second), }, }, + { + "block_query_wait_nil", + &Config{ + BlockQueryWaitTime: TimeDuration(1 * time.Second), + }, + &Config{ + BlockQueryWaitTime: nil, + }, + &Config{ + BlockQueryWaitTime: TimeDuration(1 * time.Second), + }, + }, { "pid_file", &Config{ From 665dacbbd7b977b382dd0f014be526abaac6c645 Mon Sep 17 00:00:00 2001 From: John Eikenberry Date: Fri, 15 Jan 2021 17:51:01 -0800 Subject: [PATCH 2/3] fix issue with block_query_wait setting The block_query_wait field is a pointer type on the struct. Nil typed fields always need to be checked vs. nil in Merge() functions. I think this was missed as it was put below Once which is of type bool and doesn't need it (ie. followed bad pattern). --- config/config.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/config/config.go b/config/config.go index a4d31d5a9..cd20be814 100644 --- a/config/config.go +++ b/config/config.go @@ -222,9 +222,11 @@ func (c *Config) Merge(o *Config) *Config { r.Wait = r.Wait.Merge(o.Wait) } - r.Once = o.Once + if o.BlockQueryWaitTime != nil { + r.BlockQueryWaitTime = o.BlockQueryWaitTime + } - r.BlockQueryWaitTime = o.BlockQueryWaitTime + r.Once = o.Once return r } From 8ab9628fb643c435a77d1c8583674e0aff8fbc79 Mon Sep 17 00:00:00 2001 From: John Eikenberry Date: Fri, 15 Jan 2021 17:52:56 -0800 Subject: [PATCH 3/3] test cleanup Don't use default value in tests that compare values. When outputing test diffs, try harder to output values instead of pointers ints. Fix formatting of config's GoString() output. --- config/config.go | 14 ++++++++++---- config/config_test.go | 8 ++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/config/config.go b/config/config.go index cd20be814..7aa69eab4 100644 --- a/config/config.go +++ b/config/config.go @@ -417,8 +417,8 @@ func (c *Config) GoString() string { "Syslog:%#v, "+ "Templates:%#v, "+ "Vault:%#v, "+ - "Wait:%#v,"+ - "Once:%#v"+ + "Wait:%#v, "+ + "Once:%#v, "+ "BlockQueryWaitTime:%#v"+ "}", c.Consul, @@ -452,8 +452,14 @@ func (expected *Config) Diff(actual *Config) string { fo := va.Field(i) if !reflect.DeepEqual(fc.Interface(), fo.Interface()) { fmt.Fprintf(&b, "%s:\n", ct.Field(i).Name) - fmt.Fprintf(&b, "\texp: %#v\n", fc.Interface()) - fmt.Fprintf(&b, "\tact: %#v\n", fo.Interface()) + fi := fc.Interface() + if _, ok := fi.(fmt.GoStringer); ok { + fmt.Fprintf(&b, "\texp: %#v\n", fc.Interface()) + fmt.Fprintf(&b, "\tact: %#v\n", fo.Interface()) + } else { + fmt.Fprintf(&b, "\texp: %+v\n", fc.Interface()) + fmt.Fprintf(&b, "\tact: %+v\n", fo.Interface()) + } } } diff --git a/config/config_test.go b/config/config_test.go index 9b00e0cff..70ea9f44d 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -549,9 +549,9 @@ func TestParse(t *testing.T) { }, { "block_query_wait", - `block_query_wait = "60s"`, + `block_query_wait = "61s"`, &Config{ - BlockQueryWaitTime: TimeDuration(60 * time.Second), + BlockQueryWaitTime: TimeDuration(61 * time.Second), }, false, }, @@ -1792,10 +1792,10 @@ func TestConfig_Merge(t *testing.T) { BlockQueryWaitTime: TimeDuration(1 * time.Second), }, &Config{ - BlockQueryWaitTime: TimeDuration(60 * time.Second), + BlockQueryWaitTime: TimeDuration(61 * time.Second), }, &Config{ - BlockQueryWaitTime: TimeDuration(60 * time.Second), + BlockQueryWaitTime: TimeDuration(61 * time.Second), }, }, {