fix(pkg/board/remote): adb kill all port use wrong command (#147)

* fix(pkg/board/remote): adb kill all port use wrong command

* add adb forward test

* embed the script in then dockerfile

* Revert "embed the script in then dockerfile"

This reverts commit b7481d3.

Author: lucarin91

adbd.Dockerfile

@@ -1,7 +1,7 @@
 FROM debian:trixie
 
 RUN apt-get update \
-    && apt-get install -y --no-install-recommends adbd file ssh sudo \
+    && apt-get install -y --no-install-recommends adbd file ssh sudo netcat-traditional \
     && apt-get clean \
     && rm -rf /var/lib/apt/lists/*
 
@@ -10,7 +10,9 @@ RUN useradd -m --create-home --shell /bin/bash --user-group --groups sudo arduin
     mkdir /home/arduino/ArduinoApps && \
     chown -R arduino:arduino /home/arduino/ArduinoApps
 
+ADD scripts/pong-server.sh /usr/local/bin/pong-server.sh
+
 WORKDIR /home/arduino
 EXPOSE 22
 
-CMD ["/bin/sh", "-c", "/usr/sbin/sshd -D & su arduino -c adbd"]
+CMD ["/bin/sh", "-c", "/usr/sbin/sshd -D & su arduino -c adbd & pong-server.sh"]

pkg/board/remote/adb/adb.go

@@ -132,7 +132,7 @@ func (a *ADBConnection) Forward(ctx context.Context, localPort int, remotePort i
 }
 
 func (a *ADBConnection) ForwardKillAll(ctx context.Context) error {
-	cmd, err := paths.NewProcess(nil, a.adbPath, "-s", a.host, "killforward-all")
+	cmd, err := paths.NewProcess(nil, a.adbPath, "-s", a.host, "forward", "--remove-all")
 	if err != nil {
 		return err
 	}

pkg/board/remote/remote_test.go

@@ -16,19 +16,16 @@
 package remote_test
 
 import (
-	"context"
 	"fmt"
+	"net"
 
 	"io"
-	"os/exec"
-	"strconv"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/arduino/arduino-app-cli/cmd/feedback"
 	"github.com/arduino/arduino-app-cli/internal/testtools"
 	"github.com/arduino/arduino-app-cli/pkg/board/remote"
 	"github.com/arduino/arduino-app-cli/pkg/board/remote/adb"
@@ -123,7 +120,7 @@ func TestRemoteFS(t *testing.T) {
 	}
 }
 
-func TestSSHShell(t *testing.T) {
+func TestRemoteShell(t *testing.T) {
 	name, adbPort, sshPort := testtools.StartAdbDContainer(t)
 	t.Cleanup(func() { testtools.StopAdbDContainer(t, name) })
 
@@ -187,83 +184,66 @@ func TestSSHShell(t *testing.T) {
 
 }
 
-func TestSSHForwarder(t *testing.T) {
-	name, _, sshPort := testtools.StartAdbDContainer(t)
+func TestRemoteForwarder(t *testing.T) {
+	name, adbPort, sshPort := testtools.StartAdbDContainer(t)
 	t.Cleanup(func() { testtools.StopAdbDContainer(t, name) })
 
-	conn, err := ssh.FromHost("arduino", "arduino", fmt.Sprintf("%s:%s", "localhost", sshPort))
-	require.NoError(t, err)
-
-	t.Run("Forward ADB", func(t *testing.T) {
-		ctx, cancel := context.WithCancel(t.Context())
-		defer cancel()
-
-		forwardPort, err := ports.GetAvailable()
-		require.NoError(t, err)
-
-		err = conn.Forward(ctx, forwardPort, 5555)
-		if err != nil {
-			t.Errorf("Forward failed: %v", err)
-		}
-		if forwardPort <= 0 || forwardPort > 65535 {
-			t.Fatalf("invalid port: %d", forwardPort)
-		}
-		adb_forwarded_endpoint := fmt.Sprintf("localhost:%s", strconv.Itoa(forwardPort))
+	const pongServerPort = 9999
+
+	remotes := []struct {
+		name        string
+		conn        remote.Forwarder
+		forwardPort int
+	}{
+		{
+			name: "adb",
+			conn: func() remote.Forwarder {
+				conn, err := adb.FromHost("localhost:"+adbPort, "")
+				require.NoError(t, err)
+				return conn
+			}(),
+			forwardPort: func() int {
+				port, err := ports.GetAvailable()
+				require.NoError(t, err)
+				return port
+			}(),
+		},
+		{
+			name: "ssh",
+			conn: func() remote.Forwarder {
+				conn, err := ssh.FromHost("arduino", "arduino", "127.0.0.1:"+sshPort)
+				require.NoError(t, err)
+				return conn
+			}(),
+		},
+
+		// We are skipping the local forwarder test, which is just an no op in this case.
+	}
 
-		out, err := exec.Command("adb", "connect", adb_forwarded_endpoint).CombinedOutput()
-		require.NoError(t, err, "adb connect output: %q", out)
+	for _, remote := range remotes {
+		t.Run(remote.name, func(t *testing.T) {
+			forwardPort, err := ports.GetAvailable()
+			require.NoError(t, err)
 
-		cmd := exec.Command("adb", "-s", adb_forwarded_endpoint, "shell", "echo", "Hello, World!")
-		out, err = cmd.CombinedOutput()
-		require.NoError(t, err, "command output: %q", out)
-		feedback.Printf("Command output:\n%s\n", string(out))
-		require.NotNil(t, string(out))
-	})
-}
+			err = remote.conn.Forward(t.Context(), forwardPort, pongServerPort)
+			assert.NoError(t, err)
 
-func TestSSHKillForwarder(t *testing.T) {
-	name, _, sshPort := testtools.StartAdbDContainer(t)
-	t.Cleanup(func() { testtools.StopAdbDContainer(t, name) })
+			conn, err := net.Dial("tcp", fmt.Sprintf("localhost:%d", forwardPort))
+			require.NoError(t, err)
 
-	conn, err := ssh.FromHost("arduino", "arduino", fmt.Sprintf("%s:%s", "localhost", sshPort))
-	require.NoError(t, err)
+			buf := [128]byte{}
+			n, err := conn.Read(buf[:])
+			require.NoError(t, err)
+			require.Equal(t, "pong", string(buf[:n]))
 
-	t.Run("KillAllForwards", func(t *testing.T) {
-		ctx, cancel := context.WithCancel(t.Context())
-		defer cancel()
+			err = conn.Close()
+			require.NoError(t, err)
 
-		forwardPort, err := ports.GetAvailable()
-		require.NoError(t, err)
+			err = remote.conn.ForwardKillAll(t.Context())
+			assert.NoError(t, err)
 
-		err = conn.Forward(ctx, forwardPort, 5555)
-		if err != nil {
-			t.Errorf("Forward failed: %v", err)
-		}
-		if forwardPort <= 0 || forwardPort > 65535 {
-			t.Fatalf("invalid port: %d", forwardPort)
-		}
-		adb_forwarded_endpoint := fmt.Sprintf("localhost:%s", strconv.Itoa(forwardPort))
-
-		out, err := exec.Command("adb", "connect", adb_forwarded_endpoint).CombinedOutput()
-		require.NoError(t, err, "adb connect output: %q", out)
-
-		cmd := exec.Command("adb", "-s", adb_forwarded_endpoint, "shell", "echo", "Hello, World!")
-		out, err = cmd.CombinedOutput()
-		require.NoError(t, err, "command output: %q", out)
-		feedback.Printf("Command output:\n%s\n", string(out))
-		require.NotNil(t, string(out))
-
-		err = conn.ForwardKillAll(t.Context())
-		require.NoError(t, err)
-		out, err = exec.Command("adb", "disconnect", adb_forwarded_endpoint).CombinedOutput()
-		require.NoError(t, err, "adb disconnect output: %q", out)
-
-		out, err = exec.Command("adb", "connect", adb_forwarded_endpoint).CombinedOutput()
-		require.NoError(t, err, "adb connect output: %q", out)
-
-		cmd = exec.Command("adb", "-s", adb_forwarded_endpoint, "shell", "echo", "Hello, World!")
-		out, err = cmd.CombinedOutput()
-		require.Error(t, err, "command output: %q", out)
-		feedback.Printf("Command output:\n%s\n", string(out))
-	})
+			_, err = net.Dial("tcp", fmt.Sprintf("localhost:%d", forwardPort))
+			require.Error(t, err)
+		})
+	}
 }

scripts/pong-server.sh

@@ -0,0 +1,7 @@
+#!/bin/sh
+
+PORT=9999
+
+while true; do
+  echo -n "pong" | nc -l -p $PORT
+done