Minor readability improvements for e2e test refactor

tests/e2e/e2e.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	ginkgo "github.com/onsi/ginkgo/v2"
+
 	"github.com/stretchr/testify/require"
 
 	"github.com/ava-labs/avalanchego/tests/fixture"
@@ -51,7 +52,7 @@ type TestEnvironment struct {
 // Retrieve a random URI to naively attempt to spread API load across
 // nodes.
 func (te *TestEnvironment) GetRandomNodeURI() string {
-	r := rand.New(rand.NewSource(time.Now().Unix())) //nolint:gosec
+	r := rand.New(rand.NewSource(time.Now().Unix())) //#nosec G404
 	return te.URIs[r.Intn(len(te.URIs))]
 }
 

tests/e2e/e2e_test.go

@@ -12,7 +12,9 @@ import (
 	"testing"
 
 	ginkgo "github.com/onsi/ginkgo/v2"
+
 	"github.com/onsi/gomega"
+
 	"github.com/stretchr/testify/require"
 
 	"github.com/ava-labs/avalanchego/tests"

tests/e2e/p/permissionless_subnets.go

@@ -63,7 +63,7 @@ var _ = e2e.DescribePChain("[Permissionless Subnets]", func() {
 			ginkgo.By("retrieving the node ID of a primary network validator", func() {
 				pChainClient := platformvm.NewClient(nodeURI)
 				ctx, cancel := context.WithTimeout(context.Background(), e2e.DefaultTimeout)
-				validatorIDs, err := pChainClient.SampleValidators(ctx, ids.ID{}, 1)
+				validatorIDs, err := pChainClient.SampleValidators(ctx, constants.PrimaryNetworkID, 1)
 				cancel()
 				gomega.Expect(err).Should(gomega.BeNil())
 				gomega.Expect(validatorIDs).Should(gomega.HaveLen(1))

tests/e2e/p/workflow.go

@@ -94,8 +94,8 @@ var _ = e2e.DescribePChain("[Workflow]", func() {
 
 			// Use a random node ID to ensure that repeated test runs
 			// will succeed against a persistent network.
-			randomShortID, _ := ids.ToShortID(utils.RandomBytes(20))
-			validatorID := ids.NodeID(randomShortID)
+			validatorID, err := ids.ToNodeID(utils.RandomBytes(ids.NodeIDLen))
+			gomega.Expect(err).Should(gomega.BeNil())
 
 			vdr := &txs.Validator{
 				NodeID: validatorID,

tests/fixture/test_data_server.go

@@ -17,6 +17,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/ava-labs/avalanchego/utils"
 	"github.com/ava-labs/avalanchego/utils/crypto/secp256k1"
 )
 
@@ -37,15 +38,14 @@ type TestData struct {
 
 // http server allocating resources to tests potentially executing in parallel
 type testDataServer struct {
-	TestData
-
 	// Synchronizes access to test data
 	lock sync.Mutex
+	TestData
 }
 
 // Type used to marshal/unmarshal a set of test keys for transmission over http.
 type keysDocument struct {
-	Keys []*secp256k1.PrivateKey
+	Keys []*secp256k1.PrivateKey `json:"keys"`
 }
 
 func (s *testDataServer) allocateKeys(w http.ResponseWriter, r *http.Request) {
@@ -74,20 +74,21 @@ func (s *testDataServer) allocateKeys(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Allocate the requested number of keys
+	remainingKeys := len(s.FundedKeys) - keyCount
+	allocatedKeys := s.FundedKeys[remainingKeys:]
+
 	keysDoc := &keysDocument{
-		Keys: s.FundedKeys[len(s.FundedKeys)-keyCount:],
+		Keys: allocatedKeys,
 	}
 	if err := json.NewEncoder(w).Encode(keysDoc); err != nil {
-		http.Error(w, fmt.Sprintf("failed to encode test keys: %v", err), http.StatusInternalServerError)
+		msg := fmt.Sprintf("failed to encode test keys: %v", err)
+		http.Error(w, msg, http.StatusInternalServerError)
 		return
 	}
 
 	// Forget the allocated keys
-	for i := len(s.FundedKeys) - keyCount; i < len(s.FundedKeys); i++ {
-		// Ensure the key can be garbage collected
-		s.FundedKeys[i] = nil
-	}
-	s.FundedKeys = s.FundedKeys[:len(s.FundedKeys)-keyCount]
+	utils.ZeroSlice(allocatedKeys)
+	s.FundedKeys = s.FundedKeys[:remainingKeys]
 }
 
 // Serve test data via http to ensure allocation is synchronized even when
@@ -127,6 +128,7 @@ func AllocateFundedKeys(baseURI string, count int) ([]*secp256k1.PrivateKey, err
 	if count <= 0 {
 		return nil, errInvalidKeyCount
 	}
+
 	uri, err := url.Parse(baseURI)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse uri: %w", err)
@@ -139,13 +141,13 @@ func AllocateFundedKeys(baseURI string, count int) ([]*secp256k1.PrivateKey, err
 	if err != nil {
 		return nil, fmt.Errorf("failed to construct request: %w", err)
 	}
+
 	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("failed to request funded keys: %w", err)
 	}
-	if resp.Body != nil {
-		defer resp.Body.Close()
-	}
+	defer resp.Body.Close()
+
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read response for funded keys: %w", err)
@@ -156,6 +158,7 @@ func AllocateFundedKeys(baseURI string, count int) ([]*secp256k1.PrivateKey, err
 		}
 		return nil, fmt.Errorf("test data server returned unexpected status code %d: %v", resp.StatusCode, body)
 	}
+
 	keysDoc := &keysDocument{}
 	if err := json.Unmarshal(body, keysDoc); err != nil {
 		return nil, fmt.Errorf("failed to unmarshal funded keys: %w", err)

tests/fixture/test_data_server_test.go

@@ -18,11 +18,11 @@ func TestAllocateFundedKeys(t *testing.T) {
 	require := require.New(t)
 
 	factory := secp256k1.Factory{}
-	keys := []*secp256k1.PrivateKey{}
-	for i := 0; i < 5; i++ {
+	keys := make([]*secp256k1.PrivateKey, 5)
+	for i := range keys {
 		key, err := factory.NewPrivateKey()
 		require.NoError(err)
-		keys = append(keys, key)
+		keys[i] = key
 	}
 
 	uri, err := ServeTestData(TestData{
@@ -34,38 +34,44 @@ func TestAllocateFundedKeys(t *testing.T) {
 		name              string
 		count             int
 		expectedAddresses []ids.ShortID
-	}{{
-		name:  "single key",
-		count: 1,
-		expectedAddresses: []ids.ShortID{
-			keys[4].Address(),
+		expectedError     error
+	}{
+		{
+			name:  "single key",
+			count: 1,
+			expectedAddresses: []ids.ShortID{
+				keys[4].Address(),
+			},
+			expectedError: nil,
 		},
-	}, {
-		name:  "multiple keys",
-		count: 4,
-		expectedAddresses: []ids.ShortID{
-			keys[0].Address(),
-			keys[1].Address(),
-			keys[2].Address(),
-			keys[3].Address(),
+		{
+			name:  "multiple keys",
+			count: 4,
+			expectedAddresses: []ids.ShortID{
+				keys[0].Address(),
+				keys[1].Address(),
+				keys[2].Address(),
+				keys[3].Address(),
+			},
+			expectedError: nil,
 		},
-	}, {
-		name:  "insufficient keys available",
-		count: 1,
-	}}
+		{
+			name:              "insufficient keys available",
+			count:             1,
+			expectedAddresses: []ids.ShortID{},
+			expectedError:     errRequestedKeyCountExceedsAvailable,
+		},
+	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			keys, err := AllocateFundedKeys(uri, tc.count)
-			if tc.expectedAddresses == nil {
-				require.ErrorIs(err, errRequestedKeyCountExceedsAvailable)
-			} else {
-				require.NoError(err)
-				addresses := []ids.ShortID{}
-				for _, key := range keys {
-					addresses = append(addresses, key.Address())
-				}
-				require.Equal(tc.expectedAddresses, addresses)
+			require.ErrorIs(err, tc.expectedError)
+
+			addresses := make([]ids.ShortID, len(keys))
+			for i, key := range keys {
+				addresses[i] = key.Address()
 			}
+			require.Equal(tc.expectedAddresses, addresses)
 		})
 	}
 }

tests/http.go

@@ -27,7 +27,7 @@ func GetNodeMetrics(nodeURI string, metricNames ...string) (NodeMetrics, error)
 
 // GetNodesMetrics retrieves the specified metrics for the provided node URIs.
 func GetNodesMetrics(nodeURIs []string, metricNames ...string) (NodesMetrics, error) {
-	metrics := make(NodesMetrics)
+	metrics := make(NodesMetrics, len(nodeURIs))
 	for _, u := range nodeURIs {
 		var err error
 		metrics[u], err = GetNodeMetrics(u, metricNames...)

utils/zero.go

@@ -7,3 +7,13 @@ package utils
 func Zero[T any]() T {
 	return *new(T)
 }
+
+// ZeroSlice sets all values of the provided slice to the type's zero value.
+//
+// This can be useful to ensure that the garbage collector doesn't hold
+// references to values that are no longer desired.
+func ZeroSlice[T any](s []T) {
+	for i := range s {
+		s[i] = *new(T)
+	}
+}