test: add macOS and Windows tests (#8)

This is to make sure we stay cross-platform. Note that the Windows build and test is [a lot slower than Linux and Windows](actions/runner-images#7320). Because of that, I had to change the wait in one of the tests. Also, I'm using windows-2019, which is at least 2x faster than windows-latest: actions/runner-images#5166 On macOS, I ran into an issue where CloseRead on a connection that was already fully read causes an error, so I needed to update the test there too. It was a huge pain to figure out what was going on.
Jigsaw-Code · May 2, 2023 · b8f111a · b8f111a
1 parent c0cf642
commit b8f111a
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 68 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -11,21 +11,64 @@ permissions:  # added using https://github.com/step-security/secure-workflows
 
 jobs:
 
-  build:
-    name: Build
+  test_linux:
+    name: Linux
     runs-on: ubuntu-latest
     steps:
 
+    - name: Check out code into the Go module directory
+      uses: actions/checkout@v3
+
+    - name: Set up Go 1.20
+      uses: actions/setup-go@v4
+      with:
+        go-version-file: '${{ github.workspace }}/go.mod'
+
+    - name: Build
+      run: go build -v ./...
+
+    - name: Test
+      run: go test -v -race -bench '.' ./... -benchtime=100ms
+
+
+  test_macos:
+    name: macOS
+    runs-on: macos-latest
+    steps:
+
+    - name: Check out code into the Go module directory
+      uses: actions/checkout@v3
+
     - name: Set up Go 1.20
-      uses: actions/setup-go@v3
+      uses: actions/setup-go@v4
       with:
-        go-version: ^1.20
+        go-version-file: '${{ github.workspace }}/go.mod'
+
+    - name: Build
+      run: go build -v ./...
+
+    - name: Test
+      run: go test -v -race -bench '.' ./... -benchtime=100ms
+
+
+  test_windows:
+    name: Windows
+    # Use windows-2019, which is a lot faster than windows-2022:
+    # https://github.com/actions/runner-images/issues/5166
+    runs-on: windows-2019
+    steps:
 
     - name: Check out code into the Go module directory
       uses: actions/checkout@v3
 
+    - name: Set up Go 1.20
+      uses: actions/setup-go@v4
+      with:
+        go-version-file: '${{ github.workspace }}/go.mod'
+
     - name: Build
       run: go build -v ./...
 
     - name: Test
-      run: go test -v -race -bench=. ./... -benchtime=100ms
+      run: go test -v -race -bench '.' -benchtime=100ms ./...
+
diff --git a/transport/shadowsocks/stream_dialer_test.go b/transport/shadowsocks/stream_dialer_test.go
@@ -24,6 +24,7 @@ import (
 
 	"github.com/Jigsaw-Code/outline-internal-sdk/transport"
 	"github.com/shadowsocks/go-shadowsocks2/socks"
+	"github.com/stretchr/testify/require"
 )
 
 func TestStreamDialer_Dial(t *testing.T) {
@@ -70,42 +71,46 @@ func TestStreamDialer_DialNoPayload(t *testing.T) {
 func TestStreamDialer_DialFastClose(t *testing.T) {
 	// Set up a listener that verifies no data is sent.
 	listener, err := net.ListenTCP("tcp", &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0})
-	if err != nil {
-		t.Fatalf("ListenTCP failed: %v", err)
-	}
+	require.Nilf(t, err, "ListenTCP failed: %v", err)
+	defer listener.Close()
 
-	done := make(chan struct{})
+	var running sync.WaitGroup
+	running.Add(2)
+	// Server
 	go func() {
+		defer running.Done()
 		conn, err := listener.Accept()
-		if err != nil {
-			t.Error(err)
-		}
+		require.Nil(t, err)
+		defer conn.Close()
 		buf := make([]byte, 64)
 		n, err := conn.Read(buf)
 		if n > 0 || err != io.EOF {
 			t.Errorf("Expected EOF, got %v, %v", buf[:n], err)
 		}
-		listener.Close()
-		close(done)
 	}()
 
-	key := makeTestKey(t)
-	d, err := NewStreamDialer(&transport.TCPEndpoint{Address: listener.Addr().String()}, key)
-	if err != nil {
-		t.Fatalf("Failed to create StreamDialer: %v", err)
-	}
-	conn, err := d.Dial(context.Background(), testTargetAddr)
-	if err != nil {
-		t.Fatalf("StreamDialer.Dial failed: %v", err)
-	}
+	// Client
+	go func() {
+		defer running.Done()
+		key := makeTestKey(t)
+		proxyEndpoint := &transport.TCPEndpoint{Address: listener.Addr().String()}
+		d, err := NewStreamDialer(proxyEndpoint, key)
+		require.Nilf(t, err, "Failed to create StreamDialer: %v", err)
+		// Extend the wait to be safer.
+		d.ClientDataWait = 100 * time.Millisecond
+
+		conn, err := d.Dial(context.Background(), testTargetAddr)
+		require.Nilf(t, err, "StreamDialer.Dial failed: %v", err)
+
+		// Wait for less than 100 milliseconds to ensure that the target
+		// address is not sent.
+		time.Sleep(1 * time.Millisecond)
+		// Close the connection before the target address is sent.
+		conn.Close()
+	}()
 
-	// Wait for less than 10 milliseconds to ensure that the target
-	// address is not sent.
-	time.Sleep(1 * time.Millisecond)
-	// Close the connection before the target address is sent.
-	conn.Close()
 	// Wait for the listener to verify the close.
-	<-done
+	running.Wait()
 }
 
 func TestStreamDialer_TCPPrefix(t *testing.T) {

diff --git a/transport/stream_test.go b/transport/stream_test.go
@@ -23,61 +23,70 @@ import (
 	"testing"
 	"testing/iotest"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestNewTCPStreamDialerIPv4(t *testing.T) {
 	requestText := []byte("Request")
 	responseText := []byte("Response")
 
-	listener, err := net.ListenTCP("tcp", &net.TCPAddr{IP: net.IPv4(127, 0, 0, 10)})
-	require.Nil(t, err, "Failed to create TCP listener")
+	listener, err := net.ListenTCP("tcp", &net.TCPAddr{IP: net.IPv4(127, 0, 0, 1)})
+	require.Nilf(t, err, "Failed to create TCP listener: %v", err)
+	defer listener.Close()
 
 	var running sync.WaitGroup
-	running.Add(1)
+	running.Add(2)
+
+	// Server
 	go func() {
 		defer running.Done()
-		defer listener.Close()
 		clientConn, err := listener.AcceptTCP()
-		if err != nil {
-			t.Errorf("AcceptTCP failed: %v", err)
-			return
-		}
+		require.Nilf(t, err, "AcceptTCP failed: %v", err)
+
 		defer clientConn.Close()
-		if err = iotest.TestReader(clientConn, requestText); err != nil {
-			t.Errorf("Request read failed: %v", err)
-			return
-		}
-		if err = clientConn.CloseRead(); err != nil {
-			t.Errorf("CloseRead failed: %v", err)
-			return
-		}
-		if _, err = clientConn.Write(responseText); err != nil {
-			t.Errorf("Write failed: %v", err)
-			return
-		}
-		if err = clientConn.CloseWrite(); err != nil {
-			t.Errorf("CloseWrite failed: %v", err)
-			return
-		}
+		err = iotest.TestReader(clientConn, requestText)
+		assert.Nilf(t, err, "Request read failed: %v", err)
+
+		// This works on Linux, but on macOS it errors with "shutdown: socket is not connected" (syscall.ENOTCONN).
+		// It seems that on macOS you cannot call CloseRead() if you've already received a FIN and read all the data.
+		// TODO(fortuna): Consider wrapping StreamConns on macOS to make CloseRead a no-op if Read has returned io.EOF
+		// or WriteTo has been called.
+		// err = clientConn.CloseRead()
+		// assert.Nilf(t, err, "clientConn.CloseRead failed: %v", err)
+
+		_, err = clientConn.Write(responseText)
+		assert.Nilf(t, err, "Write failed: %v", err)
+
+		err = clientConn.CloseWrite()
+		assert.Nilf(t, err, "CloseWrite failed: %v", err)
 	}()
 
-	dialer := &TCPStreamDialer{}
-	dialer.Dialer.Control = func(network, address string, c syscall.RawConn) error {
-		require.Equal(t, "tcp4", network)
-		require.Equal(t, listener.Addr().String(), address)
-		return nil
-	}
-	serverConn, err := dialer.Dial(context.Background(), listener.Addr().String())
-	require.Nil(t, err, "Dial failed")
-	require.Equal(t, listener.Addr().String(), serverConn.RemoteAddr().String())
-	defer serverConn.Close()
+	// Client
+	go func() {
+		defer running.Done()
+		dialer := &TCPStreamDialer{}
+		dialer.Dialer.Control = func(network, address string, c syscall.RawConn) error {
+			require.Equal(t, "tcp4", network)
+			require.Equal(t, listener.Addr().String(), address)
+			return nil
+		}
+		serverConn, err := dialer.Dial(context.Background(), listener.Addr().String())
+		require.Nil(t, err, "Dial failed")
+		require.Equal(t, listener.Addr().String(), serverConn.RemoteAddr().String())
+		defer serverConn.Close()
 
-	serverConn.Write(requestText)
-	serverConn.CloseWrite()
+		n, err := serverConn.Write(requestText)
+		require.Nil(t, err)
+		require.Equal(t, 7, n)
+		assert.Nil(t, serverConn.CloseWrite())
 
-	require.Nil(t, iotest.TestReader(serverConn, responseText), "Response read failed")
-	serverConn.CloseRead()
+		err = iotest.TestReader(serverConn, responseText)
+		require.Nilf(t, err, "Response read failed: %v", err)
+		// See CloseRead comment on the server go-routine.
+		// err = serverConn.CloseRead()
+		// assert.Nilf(t, err, "serverConn.CloseRead failed: %v", err)
+	}()
 
 	running.Wait()
 }
@@ -104,7 +113,7 @@ func TestNewTCPStreamDialerAddress(t *testing.T) {
 }
 
 func TestDialStreamEndpointAddr(t *testing.T) {
-	listener, err := net.ListenTCP("tcp", &net.TCPAddr{IP: net.IPv4(127, 0, 0, 2)})
+	listener, err := net.ListenTCP("tcp", &net.TCPAddr{IP: net.IPv4(127, 0, 0, 1)})
 	require.Nil(t, err, "Failed to create TCP listener")
 	defer listener.Close()