Fix flaky TestMetricsNoAllocNoCover test (#2142)

libp2p · Feb 24, 2023 · cbcdd79 · cbcdd79
1 parent 100ae25
commit cbcdd79
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 20 deletions.
diff --git a/p2p/protocol/identify/id.go b/p2p/protocol/identify/id.go
@@ -81,12 +81,12 @@ type IDService interface {
 	io.Closer
 }
 
-type identifyPushSupport uint8
+type IdentifyPushSupport uint8
 
 const (
-	identifyPushSupportUnknown identifyPushSupport = iota
-	identifyPushSupported
-	identifyPushUnsupported
+	IdentifyPushSupportUnknown IdentifyPushSupport = iota
+	IdentifyPushSupported
+	IdentifyPushUnsupported
 )
 
 type entry struct {
@@ -96,7 +96,7 @@ type entry struct {
 
 	// PushSupport saves our knowledge about the peer's support of the Identify Push protocol.
 	// Before the identify request returns, we don't know yet if the peer supports Identify Push.
-	PushSupport identifyPushSupport
+	PushSupport IdentifyPushSupport
 	// Sequence is the sequence number of the last snapshot we sent to this peer.
 	Sequence uint64
 }
@@ -271,7 +271,7 @@ func (ids *idService) sendPushes(ctx context.Context) {
 	for c, e := range ids.conns {
 		// Push even if we don't know if push is supported.
 		// This will be only the case while the IdentifyWaitChan call is in flight.
-		if e.PushSupport == identifyPushSupported || e.PushSupport == identifyPushSupportUnknown {
+		if e.PushSupport == IdentifyPushSupported || e.PushSupport == IdentifyPushSupportUnknown {
 			conns = append(conns, c)
 		}
 	}
@@ -496,9 +496,9 @@ func (ids *idService) handleIdentifyResponse(s network.Stream, isPush bool) erro
 	}
 	sup, err := ids.Host.Peerstore().SupportsProtocols(c.RemotePeer(), IDPush)
 	if supportsIdentifyPush := err == nil && len(sup) > 0; supportsIdentifyPush {
-		e.PushSupport = identifyPushSupported
+		e.PushSupport = IdentifyPushSupported
 	} else {
-		e.PushSupport = identifyPushUnsupported
+		e.PushSupport = IdentifyPushUnsupported
 	}
 
 	if ids.metricsTracer != nil {

diff --git a/p2p/protocol/identify/metrics.go b/p2p/protocol/identify/metrics.go
@@ -95,7 +95,7 @@ type MetricsTracer interface {
 	TriggeredPushes(event any)
 
 	// ConnPushSupport counts peers by Push Support
-	ConnPushSupport(identifyPushSupport)
+	ConnPushSupport(IdentifyPushSupport)
 
 	// IdentifyReceived tracks metrics on receiving an identify response
 	IdentifyReceived(isPush bool, numProtocols int, numAddrs int)
@@ -146,7 +146,7 @@ func (t *metricsTracer) TriggeredPushes(ev any) {
 	pushesTriggered.WithLabelValues(*tags...).Inc()
 }
 
-func (t *metricsTracer) IncrementPushSupport(s identifyPushSupport) {
+func (t *metricsTracer) IncrementPushSupport(s IdentifyPushSupport) {
 	tags := metricshelper.GetStringSlice()
 	defer metricshelper.PutStringSlice(tags)
 
@@ -186,19 +186,19 @@ func (t *metricsTracer) IdentifyReceived(isPush bool, numProtocols int, numAddrs
 	numAddrsReceived.Observe(float64(numAddrs))
 }
 
-func (t *metricsTracer) ConnPushSupport(support identifyPushSupport) {
+func (t *metricsTracer) ConnPushSupport(support IdentifyPushSupport) {
 	tags := metricshelper.GetStringSlice()
 	defer metricshelper.PutStringSlice(tags)
 
 	*tags = append(*tags, getPushSupport(support))
 	connPushSupportTotal.WithLabelValues(*tags...).Inc()
 }
 
-func getPushSupport(s identifyPushSupport) string {
+func getPushSupport(s IdentifyPushSupport) string {
 	switch s {
-	case identifyPushSupported:
+	case IdentifyPushSupported:
 		return "supported"
-	case identifyPushUnsupported:
+	case IdentifyPushUnsupported:
 		return "not supported"
 	default:
 		return "unknown"

diff --git a/p2p/protocol/identify/metrics_test.go → ...entify/metrics_alloc_test/metrics_test.go b/p2p/protocol/identify/metrics_test.go → ...entify/metrics_alloc_test/metrics_test.go
@@ -1,12 +1,18 @@
 //go:build nocover
 
-package identify
+// These tests are in their own package to avoid transitively pulling in other
+// deps that may run background tasks in their init and thus allocate. Looking
+// at you
+// [go-libp2p-asn-util](https://github.com/libp2p/go-libp2p-asn-util/blob/master/asn.go#L14)
+
+package identify_alloc_test
 
 import (
 	"math/rand"
 	"testing"
 
 	"github.com/libp2p/go-libp2p/core/event"
+	"github.com/libp2p/go-libp2p/p2p/protocol/identify"
 )
 
 func TestMetricsNoAllocNoCover(t *testing.T) {
@@ -16,19 +22,20 @@ func TestMetricsNoAllocNoCover(t *testing.T) {
 		event.EvtNATDeviceTypeChanged{},
 	}
 
-	pushSupport := []identifyPushSupport{
-		identifyPushSupportUnknown,
-		identifyPushSupported,
-		identifyPushUnsupported,
+	pushSupport := []identify.IdentifyPushSupport{
+		identify.IdentifyPushSupportUnknown,
+		identify.IdentifyPushSupported,
+		identify.IdentifyPushUnsupported,
 	}
 
-	tr := NewMetricsTracer()
+	tr := identify.NewMetricsTracer()
 	tests := map[string]func(){
 		"TriggeredPushes":  func() { tr.TriggeredPushes(events[rand.Intn(len(events))]) },
 		"ConnPushSupport":  func() { tr.ConnPushSupport(pushSupport[rand.Intn(len(pushSupport))]) },
 		"IdentifyReceived": func() { tr.IdentifyReceived(rand.Intn(2) == 0, rand.Intn(20), rand.Intn(20)) },
 		"IdentifySent":     func() { tr.IdentifySent(rand.Intn(2) == 0, rand.Intn(20), rand.Intn(20)) },
 	}
+
 	for method, f := range tests {
 		allocs := testing.AllocsPerRun(1000, f)
 		if allocs > 0 {