diff --git a/common/clickhousedb/tests.go b/common/clickhousedb/tests.go index 19875249..11d71de2 100644 --- a/common/clickhousedb/tests.go +++ b/common/clickhousedb/tests.go @@ -25,14 +25,7 @@ func SetupClickHouse(t *testing.T, r *reporter.Reporter) *Component { if err != nil { t.Fatalf("New() error:\n%+v", err) } - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } - t.Cleanup(func() { - if err := c.Stop(); err != nil { - t.Errorf("Stop() error:\n%+v", err) - } - }) + helpers.StartStop(t, c) return c } @@ -59,14 +52,6 @@ func NewMock(t *testing.T, r *reporter.Reporter) (*Component, *mocks.MockConn) { Close(). Return(nil) - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } - t.Cleanup(func() { - if err := c.Stop(); err != nil { - t.Errorf("Stop() error:\n%+v", err) - } - }) - + helpers.StartStop(t, c) return c, mock } diff --git a/common/daemon/root_test.go b/common/daemon/root_test.go index dfe98d40..8e5d9a7e 100644 --- a/common/daemon/root_test.go +++ b/common/daemon/root_test.go @@ -7,6 +7,7 @@ import ( "gopkg.in/tomb.v2" + "akvorado/common/helpers" "akvorado/common/reporter" ) @@ -16,7 +17,7 @@ func TestTerminate(t *testing.T) { if err != nil { t.Fatalf("New() error:\n%+v", err) } - c.Start() + helpers.StartStop(t, c) select { case <-c.Terminated(): @@ -75,7 +76,7 @@ func TestTombTracking(t *testing.T) { } c.Track(&tomb, "tomb") - c.Start() + helpers.StartStop(t, c) ch := make(chan bool) tomb.Go(func() error { diff --git a/common/helpers/tests.go b/common/helpers/tests.go index a5bd25b0..dc66d237 100644 --- a/common/helpers/tests.go +++ b/common/helpers/tests.go @@ -127,3 +127,27 @@ func CheckExternalService(t *testing.T, name string, dnsCandidates []string, por return server } + +// StartStop starts a component and stops it on cleanup. +func StartStop(t *testing.T, component interface{}) { + t.Helper() + if starterC, ok := component.(starter); ok { + if err := starterC.Start(); err != nil { + t.Fatalf("Start() error:\n%+v", err) + } + } + t.Cleanup(func() { + if stopperC, ok := component.(stopper); ok { + if err := stopperC.Stop(); err != nil { + t.Errorf("Stop() error:\n%+v", err) + } + } + }) +} + +type starter interface { + Start() error +} +type stopper interface { + Stop() error +} diff --git a/common/http/root_test.go b/common/http/root_test.go index 410b79b1..4f7c5138 100644 --- a/common/http/root_test.go +++ b/common/http/root_test.go @@ -4,7 +4,6 @@ import ( "fmt" "io/ioutil" netHTTP "net/http" - "runtime" "testing" "akvorado/common/helpers" @@ -17,15 +16,6 @@ import ( func TestHandler(t *testing.T) { r := reporter.NewMock(t) h := http.NewMock(t, r) - defer func() { - h.Stop() - runtime.Gosched() - resp, err := netHTTP.Get(fmt.Sprintf("http://%s/", h.Address)) - if err == nil { - t.Errorf("Still able to connect to expvar server after stop") - resp.Body.Close() - } - }() h.AddHandler("/test", netHTTP.HandlerFunc(func(w netHTTP.ResponseWriter, r *netHTTP.Request) { @@ -63,7 +53,6 @@ func TestHandler(t *testing.T) { func TestGinRouter(t *testing.T) { r := reporter.NewMock(t) h := http.NewMock(t, r) - defer h.Stop() h.GinRouter.GET("/api/v0/test", func(c *gin.Context) { c.JSON(netHTTP.StatusOK, gin.H{ @@ -89,7 +78,6 @@ func TestGinRouter(t *testing.T) { func TestGinRouterPanic(t *testing.T) { r := reporter.NewMock(t) h := http.NewMock(t, r) - defer h.Stop() h.GinRouter.GET("/api/v0/test", func(c *gin.Context) { panic("heeeelp") diff --git a/common/http/tests.go b/common/http/tests.go index d776d8d8..dd7bf959 100644 --- a/common/http/tests.go +++ b/common/http/tests.go @@ -6,6 +6,7 @@ import ( "testing" "akvorado/common/daemon" + "akvorado/common/helpers" "akvorado/common/reporter" ) @@ -19,8 +20,6 @@ func NewMock(t *testing.T, r *reporter.Reporter) *Component { if err != nil { t.Fatalf("New() error:\n%+v", err) } - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } + helpers.StartStop(t, c) return c } diff --git a/inlet/core/hydrate_test.go b/inlet/core/hydrate_test.go index 369acc39..596e544d 100644 --- a/inlet/core/hydrate_test.go +++ b/inlet/core/hydrate_test.go @@ -247,10 +247,7 @@ interfaceclassifiers: if err != nil { t.Fatalf("New() error:\n%+v", err) } - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } - defer c.Stop() + helpers.StartStop(t, c) // Inject twice since otherwise, we get a cache miss received := make(chan bool) diff --git a/inlet/core/root_test.go b/inlet/core/root_test.go index 3bd76beb..dbbf97ba 100644 --- a/inlet/core/root_test.go +++ b/inlet/core/root_test.go @@ -47,14 +47,7 @@ func TestCore(t *testing.T) { if err != nil { t.Fatalf("New() error:\n%+v", err) } - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } - defer func() { - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } - }() + helpers.StartStop(t, c) flowMessage := func(exporter string, in, out uint32) *flow.Message { return &flow.Message{ diff --git a/inlet/flow/root_test.go b/inlet/flow/root_test.go index 0037d683..f44071d6 100644 --- a/inlet/flow/root_test.go +++ b/inlet/flow/root_test.go @@ -29,11 +29,6 @@ func TestFlow(t *testing.T) { }, } c := NewMock(t, r, config) - defer func() { - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } - }() // Receive flows received := []*Message{} diff --git a/inlet/flow/tests.go b/inlet/flow/tests.go index c4dafeba..f180670e 100644 --- a/inlet/flow/tests.go +++ b/inlet/flow/tests.go @@ -6,6 +6,7 @@ import ( "testing" "akvorado/common/daemon" + "akvorado/common/helpers" "akvorado/common/http" "akvorado/common/reporter" "akvorado/inlet/flow/input/udp" @@ -33,9 +34,7 @@ func NewMock(t *testing.T, r *reporter.Reporter, config Configuration) *Componen if err != nil { t.Fatalf("New() error:\n%+v", err) } - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } + helpers.StartStop(t, c) return c } diff --git a/inlet/geoip/root_test.go b/inlet/geoip/root_test.go index 5392b49b..dee36cbf 100644 --- a/inlet/geoip/root_test.go +++ b/inlet/geoip/root_test.go @@ -46,15 +46,7 @@ func TestDatabaseRefresh(t *testing.T) { if err != nil { t.Fatalf("New() error:\n%+v", err) } - - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } - defer func() { - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } - }() + helpers.StartStop(t, c) // Check we did load both databases gotMetrics := r.GetMetrics("akvorado_inlet_geoip_db_") @@ -87,13 +79,7 @@ func TestStartWithoutDatabase(t *testing.T) { if err != nil { t.Fatalf("New() error:\n%+v", err) } - - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } + helpers.StartStop(t, c) } func TestStartWithMissingDatabase(t *testing.T) { diff --git a/inlet/geoip/tests.go b/inlet/geoip/tests.go index 9631e9cc..a1f2c036 100644 --- a/inlet/geoip/tests.go +++ b/inlet/geoip/tests.go @@ -9,6 +9,7 @@ import ( "testing" "akvorado/common/daemon" + "akvorado/common/helpers" "akvorado/common/reporter" ) @@ -27,8 +28,6 @@ func NewMock(t *testing.T, r *reporter.Reporter) *Component { if err != nil { t.Fatalf("New() error:\n%+s", err) } - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+s", err) - } + helpers.StartStop(t, c) return c } diff --git a/inlet/kafka/functional_test.go b/inlet/kafka/functional_test.go index 176a193f..42a5ae8c 100644 --- a/inlet/kafka/functional_test.go +++ b/inlet/kafka/functional_test.go @@ -32,14 +32,7 @@ func TestRealKafka(t *testing.T) { if err != nil { t.Fatalf("New() error:\n%+v", err) } - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } - defer func() { - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } - }() + helpers.StartStop(t, c) c.Send("127.0.0.1", []byte("hello world!")) c.Send("127.0.0.1", []byte("goodbye world!")) diff --git a/inlet/kafka/root_test.go b/inlet/kafka/root_test.go index 4d701b30..da05dae0 100644 --- a/inlet/kafka/root_test.go +++ b/inlet/kafka/root_test.go @@ -53,10 +53,6 @@ func TestKafka(t *testing.T) { if diff := helpers.Diff(gotMetrics, expectedMetrics); diff != "" { t.Fatalf("Metrics (-got, +want):\n%s", diff) } - - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } } func TestKafkaMetrics(t *testing.T) { diff --git a/inlet/kafka/tests.go b/inlet/kafka/tests.go index d89cdc5b..9dca0b65 100644 --- a/inlet/kafka/tests.go +++ b/inlet/kafka/tests.go @@ -9,6 +9,7 @@ import ( "github.com/Shopify/sarama/mocks" "akvorado/common/daemon" + "akvorado/common/helpers" "akvorado/common/reporter" ) @@ -27,9 +28,6 @@ func NewMock(t *testing.T, reporter *reporter.Reporter, configuration Configurat return mockProducer, nil } - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } - + helpers.StartStop(t, c) return c, mockProducer } diff --git a/inlet/snmp/root_test.go b/inlet/snmp/root_test.go index fcc070be..2cf4ad5e 100644 --- a/inlet/snmp/root_test.go +++ b/inlet/snmp/root_test.go @@ -26,11 +26,6 @@ func expectSNMPLookup(t *testing.T, c *Component, exporter string, ifIndex uint, func TestLookup(t *testing.T) { r := reporter.NewMock(t) c := NewMock(t, r, DefaultConfiguration(), Dependencies{Daemon: daemon.NewMock(t)}) - defer func() { - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } - }() expectSNMPLookup(t, c, "127.0.0.1", 765, answer{Err: ErrCacheMiss}) time.Sleep(30 * time.Millisecond) @@ -49,11 +44,6 @@ func TestSNMPCommunities(t *testing.T) { "127.0.0.2": "private", } c := NewMock(t, r, configuration, Dependencies{Daemon: daemon.NewMock(t)}) - defer func() { - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } - }() // Use "public" as a community. Should work. expectSNMPLookup(t, c, "127.0.0.1", 765, answer{Err: ErrCacheMiss}) @@ -75,30 +65,29 @@ func TestSNMPCommunities(t *testing.T) { } func TestComponentSaveLoad(t *testing.T) { - r := reporter.NewMock(t) configuration := DefaultConfiguration() configuration.CachePersistFile = filepath.Join(t.TempDir(), "cache") - c := NewMock(t, r, configuration, Dependencies{Daemon: daemon.NewMock(t)}) - expectSNMPLookup(t, c, "127.0.0.1", 765, answer{Err: ErrCacheMiss}) - time.Sleep(30 * time.Millisecond) - expectSNMPLookup(t, c, "127.0.0.1", 765, answer{ - ExporterName: "127_0_0_1", - Interface: Interface{Name: "Gi0/0/765", Description: "Interface 765", Speed: 1000}, - }) - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+c", err) - } + t.Run("save", func(t *testing.T) { + r := reporter.NewMock(t) + c := NewMock(t, r, configuration, Dependencies{Daemon: daemon.NewMock(t)}) - r = reporter.NewMock(t) - c = NewMock(t, r, configuration, Dependencies{Daemon: daemon.NewMock(t)}) - expectSNMPLookup(t, c, "127.0.0.1", 765, answer{ - ExporterName: "127_0_0_1", - Interface: Interface{Name: "Gi0/0/765", Description: "Interface 765", Speed: 1000}, + expectSNMPLookup(t, c, "127.0.0.1", 765, answer{Err: ErrCacheMiss}) + time.Sleep(30 * time.Millisecond) + expectSNMPLookup(t, c, "127.0.0.1", 765, answer{ + ExporterName: "127_0_0_1", + Interface: Interface{Name: "Gi0/0/765", Description: "Interface 765", Speed: 1000}, + }) + }) + + t.Run("load", func(t *testing.T) { + r := reporter.NewMock(t) + c := NewMock(t, r, configuration, Dependencies{Daemon: daemon.NewMock(t)}) + expectSNMPLookup(t, c, "127.0.0.1", 765, answer{ + ExporterName: "127_0_0_1", + Interface: Interface{Name: "Gi0/0/765", Description: "Interface 765", Speed: 1000}, + }) }) - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+c", err) - } } func TestAutoRefresh(t *testing.T) { @@ -131,11 +120,6 @@ func TestAutoRefresh(t *testing.T) { Interface: Interface{Name: "Gi0/0/765", Description: "Interface 765", Speed: 1000}, }) - // Stop and look at the cache - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } - gotMetrics := r.GetMetrics("akvorado_inlet_snmp_cache_") expectedMetrics := map[string]string{ `expired`: "0", @@ -185,10 +169,7 @@ func TestStartStopWithMultipleWorkers(t *testing.T) { r := reporter.NewMock(t) configuration := DefaultConfiguration() configuration.Workers = 5 - c := NewMock(t, r, configuration, Dependencies{Daemon: daemon.NewMock(t)}) - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } + NewMock(t, r, configuration, Dependencies{Daemon: daemon.NewMock(t)}) } type logCoalescePoller struct { @@ -201,27 +182,29 @@ func (fcp *logCoalescePoller) Poll(ctx context.Context, exporterIP string, _ uin } func TestCoalescing(t *testing.T) { - r := reporter.NewMock(t) - c := NewMock(t, r, DefaultConfiguration(), Dependencies{Daemon: daemon.NewMock(t)}) lcp := &logCoalescePoller{ received: []lookupRequest{}, } - c.poller = lcp + r := reporter.NewMock(t) + t.Run("run", func(t *testing.T) { + c := NewMock(t, r, DefaultConfiguration(), Dependencies{Daemon: daemon.NewMock(t)}) + c.poller = lcp - // Block dispatcher - blocker := make(chan bool) - c.dispatcherBChannel <- blocker + // Block dispatcher + blocker := make(chan bool) + c.dispatcherBChannel <- blocker - // Queue requests - expectSNMPLookup(t, c, "127.0.0.1", 766, answer{Err: ErrCacheMiss}) - expectSNMPLookup(t, c, "127.0.0.1", 767, answer{Err: ErrCacheMiss}) - expectSNMPLookup(t, c, "127.0.0.1", 768, answer{Err: ErrCacheMiss}) - expectSNMPLookup(t, c, "127.0.0.1", 769, answer{Err: ErrCacheMiss}) + // Queue requests + expectSNMPLookup(t, c, "127.0.0.1", 766, answer{Err: ErrCacheMiss}) + expectSNMPLookup(t, c, "127.0.0.1", 767, answer{Err: ErrCacheMiss}) + expectSNMPLookup(t, c, "127.0.0.1", 768, answer{Err: ErrCacheMiss}) + expectSNMPLookup(t, c, "127.0.0.1", 769, answer{Err: ErrCacheMiss}) - // Unblock - time.Sleep(20 * time.Millisecond) - close(blocker) - time.Sleep(20 * time.Millisecond) + // Unblock + time.Sleep(20 * time.Millisecond) + close(blocker) + time.Sleep(20 * time.Millisecond) + }) gotMetrics := r.GetMetrics("akvorado_inlet_snmp_poller_", "coalesced_count") expectedMetrics := map[string]string{ @@ -231,10 +214,6 @@ func TestCoalescing(t *testing.T) { t.Errorf("Metrics (-got, +want):\n%s", diff) } - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } - expectedAccepted := []lookupRequest{ {"127.0.0.1", []uint{766, 767, 768, 769}}, } @@ -264,11 +243,6 @@ func TestPollerBreaker(t *testing.T) { configuration := DefaultConfiguration() configuration.PollerCoalesce = 0 c := NewMock(t, r, configuration, Dependencies{Daemon: daemon.NewMock(t)}) - defer func() { - if err := c.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } - }() if tc.Poller != nil { c.poller = tc.Poller } diff --git a/inlet/snmp/tests.go b/inlet/snmp/tests.go index babdfb09..4a27ec81 100644 --- a/inlet/snmp/tests.go +++ b/inlet/snmp/tests.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "akvorado/common/helpers" "akvorado/common/reporter" ) @@ -48,8 +49,6 @@ func NewMock(t *testing.T, reporter *reporter.Reporter, configuration Configurat } // Change the poller to a fake one. c.poller = newMockPoller("public", c.sc.Put) - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } + helpers.StartStop(t, c) return c } diff --git a/orchestrator/clickhouse/functional_test.go b/orchestrator/clickhouse/functional_test.go index b4b1e513..3307dee0 100644 --- a/orchestrator/clickhouse/functional_test.go +++ b/orchestrator/clickhouse/functional_test.go @@ -17,70 +17,66 @@ func TestRealClickHouse(t *testing.T) { r := reporter.NewMock(t) chComponent := clickhousedb.SetupClickHouse(t, r) - configuration := DefaultConfiguration() - ch, err := New(r, configuration, Dependencies{ - Daemon: daemon.NewMock(t), - HTTP: http.NewMock(t, r), - ClickHouse: chComponent, - }) - if err != nil { - t.Fatalf("New() error:\n%+v", err) - } - if err := ch.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } - select { - case <-ch.migrationsDone: - case <-time.After(3 * time.Second): - t.Fatalf("Migrations not done") - } - - // Check with the ClickHouse client we have our tables - rows, err := chComponent.Query(context.Background(), "SHOW TABLES") - if err != nil { - t.Fatalf("Query() error:\n%+v", err) - } - got := []string{} - for rows.Next() { - var table string - if err := rows.Scan(&table); err != nil { - t.Fatalf("Scan() error:\n%+v", err) + t.Run("first time", func(t *testing.T) { + configuration := DefaultConfiguration() + ch, err := New(r, configuration, Dependencies{ + Daemon: daemon.NewMock(t), + HTTP: http.NewMock(t, r), + ClickHouse: chComponent, + }) + if err != nil { + t.Fatalf("New() error:\n%+v", err) } - if !strings.HasPrefix(table, ".") { - got = append(got, table) + helpers.StartStop(t, ch) + select { + case <-ch.migrationsDone: + case <-time.After(3 * time.Second): + t.Fatalf("Migrations not done") } - } - expected := []string{ - "asns", - "exporters", - "flows", - "flows_1_raw", - "flows_1_raw_consumer", - "protocols", - } - if diff := helpers.Diff(got, expected); diff != "" { - t.Fatalf("SHOW TABLES (-got, +want):\n%s", diff) - } - if err := ch.Stop(); err != nil { - t.Fatalf("Stop() error:\n%+v", err) - } - // Check we can run a second time - ch, err = New(r, configuration, Dependencies{ - Daemon: daemon.NewMock(t), - HTTP: http.NewMock(t, r), - ClickHouse: chComponent, + // Check with the ClickHouse client we have our tables + rows, err := chComponent.Query(context.Background(), "SHOW TABLES") + if err != nil { + t.Fatalf("Query() error:\n%+v", err) + } + got := []string{} + for rows.Next() { + var table string + if err := rows.Scan(&table); err != nil { + t.Fatalf("Scan() error:\n%+v", err) + } + if !strings.HasPrefix(table, ".") { + got = append(got, table) + } + } + expected := []string{ + "asns", + "exporters", + "flows", + "flows_1_raw", + "flows_1_raw_consumer", + "protocols", + } + if diff := helpers.Diff(got, expected); diff != "" { + t.Fatalf("SHOW TABLES (-got, +want):\n%s", diff) + } + }) + + t.Run("second time", func(t *testing.T) { + configuration := DefaultConfiguration() + ch, err := New(r, configuration, Dependencies{ + Daemon: daemon.NewMock(t), + HTTP: http.NewMock(t, r), + ClickHouse: chComponent, + }) + if err != nil { + t.Fatalf("New() error:\n%+v", err) + } + helpers.StartStop(t, ch) + select { + case <-ch.migrationsDone: + case <-time.After(3 * time.Second): + t.Fatalf("Migrations not done") + } }) - if err != nil { - t.Fatalf("New() error:\n%+v", err) - } - if err := ch.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } - select { - case <-ch.migrationsDone: - case <-time.After(3 * time.Second): - t.Fatalf("Migrations not done") - } - ch.Stop() } diff --git a/orchestrator/kafka/functional_test.go b/orchestrator/kafka/functional_test.go index aa369fbb..0a8a5ebc 100644 --- a/orchestrator/kafka/functional_test.go +++ b/orchestrator/kafka/functional_test.go @@ -66,9 +66,7 @@ func TestTopicCreation(t *testing.T) { if err != nil { t.Fatalf("New() error:\n%+v", err) } - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } + helpers.StartStop(t, c) adminClient, err := sarama.NewClusterAdminFromClient(client) if err != nil { @@ -111,9 +109,7 @@ func TestTopicMorePartitions(t *testing.T) { if err != nil { t.Fatalf("New() error:\n%+v", err) } - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } + helpers.StartStop(t, c) adminClient, err := sarama.NewClusterAdminFromClient(client) if err != nil { @@ -138,9 +134,7 @@ func TestTopicMorePartitions(t *testing.T) { if err != nil { t.Fatalf("New() error:\n%+v", err) } - if err := c.Start(); err != nil { - t.Fatalf("Start() error:\n%+v", err) - } + helpers.StartStop(t, c) topics, err = adminClient.ListTopics() if err != nil {