fixup: respond to review feedback #3

Author: maru-ava

tests/e2e/README.md

@@ -71,8 +71,8 @@ ginkgo -v ./tests/e2e -- \
     --use-persistent-network \
     --network-dir=/path/to/network
 
-# Also possible to set the AVALANCHEGO_PATH env var instead of supplying --avalanchego-path
-# Also possible to set the TESTNETCTL_NETWORK_DIR env var instead of supplying --network-dir
+# It is also possible to set the AVALANCHEGO_PATH env var instead of supplying --avalanchego-path
+# and to set TESTNETCTL_NETWORK_DIR instead of supplying --network-dir.
 ```
 
 See the testnet fixture [README](../fixture/testnet/README.md) for more details.

tests/e2e/x/transfer/virtuous.go

@@ -50,7 +50,7 @@ var _ = e2e.DescribeXChainSerial("[Virtuous Transfer Tx AVAX]", func() {
 			// test avoids the case of a previous test having initiated block
 			// processing but not having completed it.
 			gomega.Eventually(func() bool {
-				allNodeMetrics, err := tests.GetMetricsForNodes(rpcEps, metricBlksProcessing)
+				allNodeMetrics, err := tests.GetNodesMetrics(rpcEps, metricBlksProcessing)
 				gomega.Expect(err).Should(gomega.BeNil())
 				for _, metrics := range allNodeMetrics {
 					if metrics[metricBlksProcessing] > 0 {
@@ -110,7 +110,7 @@ var _ = e2e.DescribeXChainSerial("[Virtuous Transfer Tx AVAX]", func() {
 					)
 				}
 
-				metricsBeforeTx, err := tests.GetMetricsForNodes(rpcEps, allMetrics...)
+				metricsBeforeTx, err := tests.GetNodesMetrics(rpcEps, allMetrics...)
 				gomega.Expect(err).Should(gomega.BeNil())
 				for _, uri := range rpcEps {
 					tests.Outf("{{green}}metrics at %q:{{/}} %v\n", uri, metricsBeforeTx[uri])
@@ -256,7 +256,7 @@ RECEIVER  NEW BALANCE (AFTER) : %21d AVAX
 					gomega.Expect(err).Should(gomega.BeNil())
 					gomega.Expect(status).Should(gomega.Equal(choices.Accepted))
 
-					mm, err := tests.GetMetricsForNode(u, allMetrics...)
+					mm, err := tests.GetNodeMetrics(u, allMetrics...)
 					gomega.Expect(err).Should(gomega.BeNil())
 
 					prev := metricsBeforeTx[u]

tests/fixture/test_data_server.go

@@ -11,6 +11,7 @@ import (
 	"io"
 	"net"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 	"sync"
@@ -39,7 +40,7 @@ type testDataServer struct {
 	TestData
 
 	// Synchronizes access to test data
-	mutex sync.Mutex
+	lock sync.Mutex
 }
 
 // Type used to marshal/unmarshal a set of test keys for transmission over http.
@@ -63,8 +64,8 @@ func (s *testDataServer) allocateKeys(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Ensure a key will be allocated at most once
-	s.mutex.Lock()
-	defer s.mutex.Unlock()
+	s.lock.Lock()
+	defer s.lock.Unlock()
 
 	// Only fulfill requests for available keys
 	if keyCount > len(s.FundedKeys) {
@@ -111,6 +112,7 @@ func ServeTestData(testData TestData) (string, error) {
 	}
 
 	go func() {
+		// Serve always returns a non-nil error and closes l.
 		if err := httpServer.Serve(listener); err != http.ErrServerClosed {
 			panic(fmt.Sprintf("unexpected error closing test data server: %v", err))
 		}
@@ -121,12 +123,19 @@ func ServeTestData(testData TestData) (string, error) {
 
 // Retrieve the specified number of funded test keys from the provided URI. A given
 // key is allocated at most once during the life of the test data server.
-func AllocateFundedKeys(uri string, count int) ([]*secp256k1.PrivateKey, error) {
+func AllocateFundedKeys(baseURI string, count int) ([]*secp256k1.PrivateKey, error) {
 	if count <= 0 {
 		return nil, errInvalidKeyCount
 	}
-	query := fmt.Sprintf("%s%s?%s=%d", uri, allocateKeysPath, keyCountParameterName, count)
-	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, query, nil)
+	uri, err := url.Parse(baseURI)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse uri: %w", err)
+	}
+	uri.RawQuery = url.Values{
+		keyCountParameterName: {strconv.Itoa(count)},
+	}.Encode()
+	uri.Path = allocateKeysPath
+	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, uri.String(), nil)
 	if err != nil {
 		return nil, fmt.Errorf("failed to construct request: %w", err)
 	}

tests/http.go

@@ -19,18 +19,18 @@ type NodeMetrics map[string]float64
 // URI -> "metric name" -> "metric value"
 type NodesMetrics map[string]NodeMetrics
 
-// GetMetricsForNode retrieves the specified metrics the provided node URI.
-func GetMetricsForNode(nodeURI string, metricNames ...string) (NodeMetrics, error) {
+// GetNodeMetrics retrieves the specified metrics the provided node URI.
+func GetNodeMetrics(nodeURI string, metricNames ...string) (NodeMetrics, error) {
 	uri := nodeURI + "/ext/metrics"
 	return GetMetricsValue(uri, metricNames...)
 }
 
-// GetMetricsForNodes retrieves the specified metrics for the provided node URIs.
-func GetMetricsForNodes(nodeURIs []string, metricNames ...string) (NodesMetrics, error) {
+// GetNodesMetrics retrieves the specified metrics for the provided node URIs.
+func GetNodesMetrics(nodeURIs []string, metricNames ...string) (NodesMetrics, error) {
 	metrics := make(NodesMetrics)
 	for _, u := range nodeURIs {
 		var err error
-		metrics[u], err = GetMetricsForNode(u, metricNames...)
+		metrics[u], err = GetNodeMetrics(u, metricNames...)
 		if err != nil {
 			return nil, fmt.Errorf("failed to retrieve metrics for %s: %w", u, err)
 		}