Remove testutil.GetAvailablePort, apply exclusion logic to address as…

… well (open-telemetry#10871)

Signed-off-by: Bogdan Drutu <[email protected]>

exporter/zipkinexporter/zipkin_test.go

@@ -322,11 +322,11 @@ func TestZipkinExporter_roundtripProto(t *testing.T) {
  mzr.serializer = zipkin_proto3.SpanSerializer{}
 
  // Run the Zipkin receiver to "receive spans upload from a client application"
- port := testutil.GetAvailablePort(t)
+ addr := testutil.GetAvailableLocalAddress(t)
  recvCfg := &zipkinreceiver.Config{
  ReceiverSettings: config.NewReceiverSettings(config.NewComponentIDWithName("zipkin", "receiver")),
  HTTPServerSettings: confighttp.HTTPServerSettings{
- Endpoint: fmt.Sprintf(":%d", port),
+ Endpoint: addr,
  },
  }
  zi, err := zipkinreceiver.NewFactory().CreateTracesReceiver(context.Background(), componenttest.NewNopReceiverCreateSettings(), recvCfg, zexp)
@@ -337,7 +337,7 @@ func TestZipkinExporter_roundtripProto(t *testing.T) {
  t.Cleanup(func() { require.NoError(t, zi.Shutdown(context.Background())) })
 
  // Let the receiver receive "uploaded Zipkin spans from a Java client application"
- _, _ = http.Post(fmt.Sprintf("http:https://localhost:%d", port), "", strings.NewReader(zipkinSpansJSONJavaLibrary))
+ _, _ = http.Post("http:https://"+addr, "", strings.NewReader(zipkinSpansJSONJavaLibrary))
 
  // Use the mock zipkin reporter to ensure all expected spans in a single batch. Since Flush waits for
  // server response there is no need for further synchronization.

internal/common/testutil/testutil.go

@@ -18,10 +18,10 @@ import (
  "net"
  "os/exec"
  "runtime"
- "strconv"
  "strings"
  "testing"
 
+ "github.com/stretchr/testify/assert"
  "github.com/stretchr/testify/require"
 )
 
@@ -34,37 +34,22 @@ type portpair struct {
 // describing it. The port is available for opening when this function returns
 // provided that there is no race by some other code to grab the same port
 // immediately.
-func GetAvailableLocalAddress(t *testing.T) string {
- ln, err := net.Listen("tcp", "localhost:0")
- require.NoError(t, err, "Failed to get a free local port")
- // There is a possible race if something else takes this same port before
- // the test uses it, however, that is unlikely in practice.
- defer func() {
- require.NoError(t, ln.Close())
- }()
- return ln.Addr().String()
-}
-
-// GetAvailablePort finds an available local port and returns it. The port is
-// available for opening when this function returns provided that there is no
-// race by some other code to grab the same port immediately.
-func GetAvailablePort(t *testing.T) uint16 {
+func GetAvailableLocalAddress(t testing.TB) string {
  // Retry has been added for windows as net.Listen can return a port that is not actually available. Details can be
  // found in https://github.com/docker/for-win/issues/3171 but to summarize Hyper-V will reserve ranges of ports
  // which do not show up under the "netstat -ano" but can only be found by
  // "netsh interface ipv4 show excludedportrange protocol=tcp". We'll use []exclusions to hold those ranges and
  // retry if the port returned by GetAvailableLocalAddress falls in one of those them.
  var exclusions []portpair
  portFound := false
- var port string
- var err error
  if runtime.GOOS == "windows" {
  exclusions = getExclusionsList(t)
  }
 
+ var endpoint string
  for !portFound {
- endpoint := GetAvailableLocalAddress(t)
- _, port, err = net.SplitHostPort(endpoint)
+ endpoint = findAvailableAddress(t)
+ _, port, err := net.SplitHostPort(endpoint)
  require.NoError(t, err)
  portFound = true
  if runtime.GOOS == "windows" {
@@ -77,29 +62,37 @@ func GetAvailablePort(t *testing.T) uint16 {
  }
  }
 
- portInt, err := strconv.ParseUint(port, 10, 16)
- require.NoError(t, err)
+ return endpoint
+}
 
- return uint16(portInt)
+func findAvailableAddress(t testing.TB) string {
+ ln, err := net.Listen("tcp", "localhost:0")
+ require.NoError(t, err, "Failed to get a free local port")
+ // There is a possible race if something else takes this same port before
+ // the test uses it, however, that is unlikely in practice.
+ defer func() {
+ assert.NoError(t, ln.Close())
+ }()
+ return ln.Addr().String()
 }
 
 // Get excluded ports on Windows from the command: netsh interface ipv4 show excludedportrange protocol=tcp
-func getExclusionsList(t *testing.T) []portpair {
+func getExclusionsList(t testing.TB) []portpair {
  cmdTCP := exec.Command("netsh", "interface", "ipv4", "show", "excludedportrange", "protocol=tcp")
  outputTCP, errTCP := cmdTCP.CombinedOutput()
  require.NoError(t, errTCP)
- exclusions := createExclusionsList(string(outputTCP), t)
+ exclusions := createExclusionsList(t, string(outputTCP))
 
  cmdUDP := exec.Command("netsh", "interface", "ipv4", "show", "excludedportrange", "protocol=udp")
  outputUDP, errUDP := cmdUDP.CombinedOutput()
  require.NoError(t, errUDP)
- exclusions = append(exclusions, createExclusionsList(string(outputUDP), t)...)
+ exclusions = append(exclusions, createExclusionsList(t, string(outputUDP))...)
 
  return exclusions
 }
 
-func createExclusionsList(exclusionsText string, t *testing.T) []portpair {
- exclusions := []portpair{}
+func createExclusionsList(t testing.TB, exclusionsText string) []portpair {
+ var exclusions []portpair
 
  parts := strings.Split(exclusionsText, "--------")
  require.Equal(t, len(parts), 3)

internal/common/testutil/testutil_test.go

@@ -16,33 +16,24 @@ package testutil
 
 import (
  "net"
- "strconv"
  "testing"
 
  "github.com/stretchr/testify/require"
 )
 
 func TestGetAvailableLocalAddress(t *testing.T) {
- testEndpointAvailable(t, GetAvailableLocalAddress(t))
-}
-
-func TestGetAvailablePort(t *testing.T) {
- portStr := strconv.Itoa(int(GetAvailablePort(t)))
- require.NotEqual(t, "", portStr)
-
- testEndpointAvailable(t, "localhost:"+portStr)
-}
+ addr := GetAvailableLocalAddress(t)
 
-func testEndpointAvailable(t *testing.T, endpoint string) {
  // Endpoint should be free.
- ln0, err := net.Listen("tcp", endpoint)
+ ln0, err := net.Listen("tcp", addr)
  require.NoError(t, err)
  require.NotNil(t, ln0)
- defer ln0.Close()
+ t.Cleanup(func() {
+ require.NoError(t, ln0.Close())
+ })
 
- // Ensure that the endpoint wasn't something like ":0" by checking that a
- // second listener will fail.
- ln1, err := net.Listen("tcp", endpoint)
+ // Ensure that the endpoint wasn't something like ":0" by checking that a second listener will fail.
+ ln1, err := net.Listen("tcp", addr)
  require.Error(t, err)
  require.Nil(t, ln1)
 }
@@ -67,9 +58,9 @@ Start Port End Port
 
 * - Administered port exclusions.
 `
- exclusions := createExclusionsList(exclusionsText, t)
+ exclusions := createExclusionsList(t, exclusionsText)
  require.Equal(t, len(exclusions), 2)
 
- emptyExclusions := createExclusionsList(emptyExclusionsText, t)
+ emptyExclusions := createExclusionsList(t, emptyExclusionsText)
  require.Equal(t, len(emptyExclusions), 0)
 }

testbed/testbed/utils.go

@@ -15,11 +15,22 @@
 package testbed // import "github.com/open-telemetry/opentelemetry-collector-contrib/testbed/testbed"
 
 import (
+ "net"
+ "strconv"
  "testing"
 
+ "github.com/stretchr/testify/require"
+
  "github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil"
 )
 
 func GetAvailablePort(t *testing.T) int {
- return int(testutil.GetAvailablePort(t))
+ endpoint := testutil.GetAvailableLocalAddress(t)
+ _, port, err := net.SplitHostPort(endpoint)
+ require.NoError(t, err)
+
+ portInt, err := strconv.Atoi(port)
+ require.NoError(t, err)
+
+ return portInt
 }