From ac5ac3161f2144f535c0fbd59494e9c2639a34bb Mon Sep 17 00:00:00 2001 From: Frederick Roth Date: Wed, 17 May 2017 00:25:30 +0200 Subject: [PATCH] Fixed inconsistency between HasIntField and IntField (#2813) --- plugins/inputs/aerospike/aerospike_test.go | 4 ++-- .../http_response/http_response_test.go | 4 ++-- plugins/inputs/memcached/memcached_test.go | 2 +- plugins/inputs/mongodb/mongodb_data_test.go | 4 ++-- plugins/inputs/mongodb/mongodb_server_test.go | 2 +- plugins/inputs/ping/ping_windows_test.go | 22 +++++++++---------- plugins/inputs/postgresql/postgresql_test.go | 2 +- .../postgresql_extensible_test.go | 2 +- plugins/inputs/powerdns/powerdns_test.go | 2 +- plugins/inputs/rabbitmq/rabbitmq_test.go | 4 ++-- .../inputs/rethinkdb/rethinkdb_data_test.go | 6 ++--- plugins/inputs/system/processes_test.go | 8 +++---- plugins/inputs/zookeeper/zookeeper_test.go | 2 +- testutil/accumulator.go | 18 +++++++++++++++ 14 files changed, 50 insertions(+), 32 deletions(-) diff --git a/plugins/inputs/aerospike/aerospike_test.go b/plugins/inputs/aerospike/aerospike_test.go index 2055da3bb..ac8c90d2b 100644 --- a/plugins/inputs/aerospike/aerospike_test.go +++ b/plugins/inputs/aerospike/aerospike_test.go @@ -24,7 +24,7 @@ func TestAerospikeStatistics(t *testing.T) { assert.True(t, acc.HasMeasurement("aerospike_node")) assert.True(t, acc.HasMeasurement("aerospike_namespace")) - assert.True(t, acc.HasIntField("aerospike_node", "batch_error")) + assert.True(t, acc.HasInt64Field("aerospike_node", "batch_error")) } func TestAerospikeStatisticsPartialErr(t *testing.T) { @@ -45,7 +45,7 @@ func TestAerospikeStatisticsPartialErr(t *testing.T) { assert.True(t, acc.HasMeasurement("aerospike_node")) assert.True(t, acc.HasMeasurement("aerospike_namespace")) - assert.True(t, acc.HasIntField("aerospike_node", "batch_error")) + assert.True(t, acc.HasInt64Field("aerospike_node", "batch_error")) } func TestAerospikeParseValue(t *testing.T) { diff --git a/plugins/inputs/http_response/http_response_test.go b/plugins/inputs/http_response/http_response_test.go index babb23dab..ee2390d2d 100644 --- a/plugins/inputs/http_response/http_response_test.go +++ b/plugins/inputs/http_response/http_response_test.go @@ -355,7 +355,7 @@ func TestTimeout(t *testing.T) { Address: ts.URL + "/twosecondnap", Body: "{ 'test': 'data'}", Method: "GET", - ResponseTimeout: internal.Duration{Duration: time.Millisecond}, + ResponseTimeout: internal.Duration{Duration: time.Second}, Headers: map[string]string{ "Content-Type": "application/json", }, @@ -363,7 +363,7 @@ func TestTimeout(t *testing.T) { } var acc testutil.Accumulator err := h.Gather(&acc) - require.NoError(t, err) + require.Error(t, err) ok := acc.HasIntField("http_response", "http_response_code") require.False(t, ok) diff --git a/plugins/inputs/memcached/memcached_test.go b/plugins/inputs/memcached/memcached_test.go index aae59dbc8..3c8a239f0 100644 --- a/plugins/inputs/memcached/memcached_test.go +++ b/plugins/inputs/memcached/memcached_test.go @@ -32,7 +32,7 @@ func TestMemcachedGeneratesMetrics(t *testing.T) { "bytes_read", "bytes_written", "threads", "conn_yields"} for _, metric := range intMetrics { - assert.True(t, acc.HasIntField("memcached", metric), metric) + assert.True(t, acc.HasInt64Field("memcached", metric), metric) } } diff --git a/plugins/inputs/mongodb/mongodb_data_test.go b/plugins/inputs/mongodb/mongodb_data_test.go index a74d9ed65..e2ecac49b 100644 --- a/plugins/inputs/mongodb/mongodb_data_test.go +++ b/plugins/inputs/mongodb/mongodb_data_test.go @@ -42,7 +42,7 @@ func TestAddNonReplStats(t *testing.T) { d.flush(&acc) for key, _ := range DefaultStats { - assert.True(t, acc.HasIntField("mongodb", key)) + assert.True(t, acc.HasInt64Field("mongodb", key)) } } @@ -63,7 +63,7 @@ func TestAddReplStats(t *testing.T) { d.flush(&acc) for key, _ := range MmapStats { - assert.True(t, acc.HasIntField("mongodb", key)) + assert.True(t, acc.HasInt64Field("mongodb", key)) } } diff --git a/plugins/inputs/mongodb/mongodb_server_test.go b/plugins/inputs/mongodb/mongodb_server_test.go index e9d1bae9e..c8ef5f240 100644 --- a/plugins/inputs/mongodb/mongodb_server_test.go +++ b/plugins/inputs/mongodb/mongodb_server_test.go @@ -36,6 +36,6 @@ func TestAddDefaultStats(t *testing.T) { require.NoError(t, err) for key, _ := range DefaultStats { - assert.True(t, acc.HasIntField("mongodb", key)) + assert.True(t, acc.HasInt64Field("mongodb", key)) } } diff --git a/plugins/inputs/ping/ping_windows_test.go b/plugins/inputs/ping/ping_windows_test.go index 70ff78f75..cb90b1686 100644 --- a/plugins/inputs/ping/ping_windows_test.go +++ b/plugins/inputs/ping/ping_windows_test.go @@ -217,19 +217,19 @@ func TestFatalPingGather(t *testing.T) { acc.GatherError(p.Gather) assert.True(t, acc.HasFloatField("ping", "errors"), "Fatal ping should have packet measurements") - assert.False(t, acc.HasIntField("ping", "packets_transmitted"), + assert.False(t, acc.HasInt64Field("ping", "packets_transmitted"), "Fatal ping should not have packet measurements") - assert.False(t, acc.HasIntField("ping", "packets_received"), + assert.False(t, acc.HasInt64Field("ping", "packets_received"), "Fatal ping should not have packet measurements") assert.False(t, acc.HasFloatField("ping", "percent_packet_loss"), "Fatal ping should not have packet measurements") assert.False(t, acc.HasFloatField("ping", "percent_reply_loss"), "Fatal ping should not have packet measurements") - assert.False(t, acc.HasIntField("ping", "average_response_ms"), + assert.False(t, acc.HasInt64Field("ping", "average_response_ms"), "Fatal ping should not have packet measurements") - assert.False(t, acc.HasIntField("ping", "maximum_response_ms"), + assert.False(t, acc.HasInt64Field("ping", "maximum_response_ms"), "Fatal ping should not have packet measurements") - assert.False(t, acc.HasIntField("ping", "minimum_response_ms"), + assert.False(t, acc.HasInt64Field("ping", "minimum_response_ms"), "Fatal ping should not have packet measurements") } @@ -273,11 +273,11 @@ func TestUnreachablePingGather(t *testing.T) { assert.False(t, acc.HasFloatField("ping", "errors"), "Fatal ping should not have packet measurements") - assert.False(t, acc.HasIntField("ping", "average_response_ms"), + assert.False(t, acc.HasInt64Field("ping", "average_response_ms"), "Fatal ping should not have packet measurements") - assert.False(t, acc.HasIntField("ping", "maximum_response_ms"), + assert.False(t, acc.HasInt64Field("ping", "maximum_response_ms"), "Fatal ping should not have packet measurements") - assert.False(t, acc.HasIntField("ping", "minimum_response_ms"), + assert.False(t, acc.HasInt64Field("ping", "minimum_response_ms"), "Fatal ping should not have packet measurements") } @@ -319,10 +319,10 @@ func TestTTLExpiredPingGather(t *testing.T) { assert.False(t, acc.HasFloatField("ping", "errors"), "Fatal ping should not have packet measurements") - assert.False(t, acc.HasIntField("ping", "average_response_ms"), + assert.False(t, acc.HasInt64Field("ping", "average_response_ms"), "Fatal ping should not have packet measurements") - assert.False(t, acc.HasIntField("ping", "maximum_response_ms"), + assert.False(t, acc.HasInt64Field("ping", "maximum_response_ms"), "Fatal ping should not have packet measurements") - assert.False(t, acc.HasIntField("ping", "minimum_response_ms"), + assert.False(t, acc.HasInt64Field("ping", "minimum_response_ms"), "Fatal ping should not have packet measurements") } diff --git a/plugins/inputs/postgresql/postgresql_test.go b/plugins/inputs/postgresql/postgresql_test.go index a0690961d..03c09936d 100644 --- a/plugins/inputs/postgresql/postgresql_test.go +++ b/plugins/inputs/postgresql/postgresql_test.go @@ -74,7 +74,7 @@ func TestPostgresqlGeneratesMetrics(t *testing.T) { for _, metric := range intMetrics { _, ok := availableColumns[metric] if ok { - assert.True(t, acc.HasIntField("postgresql", metric)) + assert.True(t, acc.HasInt64Field("postgresql", metric)) metricsCounted++ } } diff --git a/plugins/inputs/postgresql_extensible/postgresql_extensible_test.go b/plugins/inputs/postgresql_extensible/postgresql_extensible_test.go index 0b2785cad..1c59bffaa 100644 --- a/plugins/inputs/postgresql_extensible/postgresql_extensible_test.go +++ b/plugins/inputs/postgresql_extensible/postgresql_extensible_test.go @@ -69,7 +69,7 @@ func TestPostgresqlGeneratesMetrics(t *testing.T) { for _, metric := range intMetrics { _, ok := availableColumns[metric] if ok { - assert.True(t, acc.HasIntField("postgresql", metric)) + assert.True(t, acc.HasInt64Field("postgresql", metric)) metricsCounted++ } } diff --git a/plugins/inputs/powerdns/powerdns_test.go b/plugins/inputs/powerdns/powerdns_test.go index c2d4413e2..df36440de 100644 --- a/plugins/inputs/powerdns/powerdns_test.go +++ b/plugins/inputs/powerdns/powerdns_test.go @@ -105,7 +105,7 @@ func TestMemcachedGeneratesMetrics(t *testing.T) { "meta-cache-size", "qsize-q", "signature-cache-size", "sys-msec", "uptime", "user-msec"} for _, metric := range intMetrics { - assert.True(t, acc.HasIntField("powerdns", metric), metric) + assert.True(t, acc.HasInt64Field("powerdns", metric), metric) } } diff --git a/plugins/inputs/rabbitmq/rabbitmq_test.go b/plugins/inputs/rabbitmq/rabbitmq_test.go index 7ff0a5d57..3be0259bc 100644 --- a/plugins/inputs/rabbitmq/rabbitmq_test.go +++ b/plugins/inputs/rabbitmq/rabbitmq_test.go @@ -419,7 +419,7 @@ func TestRabbitMQGeneratesMetrics(t *testing.T) { } for _, metric := range intMetrics { - assert.True(t, acc.HasIntField("rabbitmq_overview", metric)) + assert.True(t, acc.HasInt64Field("rabbitmq_overview", metric)) } nodeIntMetrics := []string{ @@ -437,7 +437,7 @@ func TestRabbitMQGeneratesMetrics(t *testing.T) { } for _, metric := range nodeIntMetrics { - assert.True(t, acc.HasIntField("rabbitmq_node", metric)) + assert.True(t, acc.HasInt64Field("rabbitmq_node", metric)) } assert.True(t, acc.HasMeasurement("rabbitmq_queue")) diff --git a/plugins/inputs/rethinkdb/rethinkdb_data_test.go b/plugins/inputs/rethinkdb/rethinkdb_data_test.go index 6159016c0..ce1d963b9 100644 --- a/plugins/inputs/rethinkdb/rethinkdb_data_test.go +++ b/plugins/inputs/rethinkdb/rethinkdb_data_test.go @@ -36,7 +36,7 @@ func TestAddEngineStats(t *testing.T) { engine.AddEngineStats(keys, &acc, tags) for _, metric := range keys { - assert.True(t, acc.HasIntField("rethinkdb_engine", metric)) + assert.True(t, acc.HasInt64Field("rethinkdb_engine", metric)) } } @@ -67,7 +67,7 @@ func TestAddEngineStatsPartial(t *testing.T) { engine.AddEngineStats(keys, &acc, tags) for _, metric := range missing_keys { - assert.False(t, acc.HasIntField("rethinkdb", metric)) + assert.False(t, acc.HasInt64Field("rethinkdb", metric)) } } @@ -107,6 +107,6 @@ func TestAddStorageStats(t *testing.T) { storage.AddStats(&acc, tags) for _, metric := range keys { - assert.True(t, acc.HasIntField("rethinkdb", metric)) + assert.True(t, acc.HasInt64Field("rethinkdb", metric)) } } diff --git a/plugins/inputs/system/processes_test.go b/plugins/inputs/system/processes_test.go index eef52cd67..8a9b0fb6b 100644 --- a/plugins/inputs/system/processes_test.go +++ b/plugins/inputs/system/processes_test.go @@ -20,10 +20,10 @@ func TestProcesses(t *testing.T) { err := processes.Gather(&acc) require.NoError(t, err) - assert.True(t, acc.HasIntField("processes", "running")) - assert.True(t, acc.HasIntField("processes", "sleeping")) - assert.True(t, acc.HasIntField("processes", "stopped")) - assert.True(t, acc.HasIntField("processes", "total")) + assert.True(t, acc.HasInt64Field("processes", "running")) + assert.True(t, acc.HasInt64Field("processes", "sleeping")) + assert.True(t, acc.HasInt64Field("processes", "stopped")) + assert.True(t, acc.HasInt64Field("processes", "total")) total, ok := acc.Get("processes") require.True(t, ok) assert.True(t, total.Fields["total"].(int64) > 0) diff --git a/plugins/inputs/zookeeper/zookeeper_test.go b/plugins/inputs/zookeeper/zookeeper_test.go index e8bcc11d5..37cabbada 100644 --- a/plugins/inputs/zookeeper/zookeeper_test.go +++ b/plugins/inputs/zookeeper/zookeeper_test.go @@ -37,6 +37,6 @@ func TestZookeeperGeneratesMetrics(t *testing.T) { } for _, metric := range intMetrics { - assert.True(t, acc.HasIntField("zookeeper", metric), metric) + assert.True(t, acc.HasInt64Field("zookeeper", metric), metric) } } diff --git a/testutil/accumulator.go b/testutil/accumulator.go index c0f2caf22..e46476a8b 100644 --- a/testutil/accumulator.go +++ b/testutil/accumulator.go @@ -317,6 +317,24 @@ func (a *Accumulator) HasField(measurement string, field string) bool { // HasIntField returns true if the measurement has an Int value func (a *Accumulator) HasIntField(measurement string, field string) bool { + a.Lock() + defer a.Unlock() + for _, p := range a.Metrics { + if p.Measurement == measurement { + for fieldname, value := range p.Fields { + if fieldname == field { + _, ok := value.(int) + return ok + } + } + } + } + + return false +} + +// HasInt64Field returns true if the measurement has an Int64 value +func (a *Accumulator) HasInt64Field(measurement string, field string) bool { a.Lock() defer a.Unlock() for _, p := range a.Metrics {