From 126713648e3a9ef2a3275c4aef1c0cc7e9844272 Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Mon, 3 Feb 2025 20:20:07 +0100
Subject: [PATCH] cli/connhelper/ssh: TestParseURL: various improvements

- use designated example domains as example value
- swap "expected" and "actual" values in assertions
- add doc / name for each test
- add test-cases for remote commands
- also test the Spec that's produced, not just the args
- merge two test-cases that could be combined

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 cli/connhelper/ssh/ssh_test.go | 95 +++++++++++++++++++++++++++-------
 1 file changed, 77 insertions(+), 18 deletions(-)

diff --git a/cli/connhelper/ssh/ssh_test.go b/cli/connhelper/ssh/ssh_test.go
index 4b6d2c341d..cefe45eda7 100644
--- a/cli/connhelper/ssh/ssh_test.go
+++ b/cli/connhelper/ssh/ssh_test.go
@@ -9,59 +9,118 @@ import (
 
 func TestParseURL(t *testing.T) {
 	testCases := []struct {
+		doc           string
 		url           string
+		remoteCommand []string
 		expectedArgs  []string
 		expectedError string
+		expectedSpec  Spec
 	}{
 		{
-			url: "ssh://foo",
+			doc: "bare ssh URL",
+			url: "ssh://example.com",
 			expectedArgs: []string{
-				"--", "foo",
+				"--", "example.com",
+			},
+			expectedSpec: Spec{
+				Host: "example.com",
 			},
 		},
 		{
-			url: "ssh://me@foo:10022",
+			doc: "bare ssh URL and remote command",
+			url: "ssh://example.com",
+			remoteCommand: []string{
+				"docker", "system", "dial-stdio",
+			},
+			expectedArgs: []string{
+				"--", "example.com",
+				"docker", "system", "dial-stdio",
+			},
+			expectedSpec: Spec{
+				Host: "example.com",
+			},
+		},
+		{
+			doc: "ssh URL with path and remote command and flag",
+			url: "ssh://example.com/var/run/docker.sock",
+			remoteCommand: []string{
+				"docker", "--host", "unix:///var/run/docker.sock", "system", "dial-stdio",
+			},
+			expectedArgs: []string{
+				"--", "example.com",
+				"docker", "--host", "unix:///var/run/docker.sock", "system", "dial-stdio",
+			},
+			expectedSpec: Spec{
+				Host: "example.com",
+				Path: "/var/run/docker.sock",
+			},
+		},
+		{
+			doc: "ssh URL with username and port",
+			url: "ssh://me@example.com:10022",
 			expectedArgs: []string{
 				"-l", "me",
 				"-p", "10022",
-				"--", "foo",
+				"--", "example.com",
+			},
+			expectedSpec: Spec{
+				User: "me",
+				Host: "example.com",
+				Port: "10022",
 			},
 		},
 		{
-			url:           "ssh://me:passw0rd@foo",
+			doc: "ssh URL with username, port, and path",
+			url: "ssh://me@example.com:10022/var/run/docker.sock",
+			expectedArgs: []string{
+				"-l", "me",
+				"-p", "10022",
+				"--", "example.com",
+			},
+			expectedSpec: Spec{
+				User: "me",
+				Host: "example.com",
+				Port: "10022",
+				Path: "/var/run/docker.sock",
+			},
+		},
+		{
+			doc:           "invalid URL with password",
+			url:           "ssh://me:passw0rd@example.com",
 			expectedError: "plain-text password is not supported",
 		},
 		{
-			url: "ssh://foo/bar",
-			expectedArgs: []string{
-				"--", "foo",
-			},
-		},
-		{
-			url:           "ssh://foo?bar",
+			doc:           "invalid URL with query parameter",
+			url:           "ssh://example.com?bar",
 			expectedError: `extra query after the host: "bar"`,
 		},
 		{
-			url:           "ssh://foo#bar",
+			doc:           "invalid URL with fragment",
+			url:           "ssh://example.com#bar",
 			expectedError: `extra fragment after the host: "bar"`,
 		},
 		{
+			doc:           "invalid URL without hostname",
 			url:           "ssh://",
 			expectedError: "no host specified",
 		},
 		{
-			url:           "foo://bar",
-			expectedError: `expected scheme ssh, got "foo"`,
+			doc:           "invalid URL with unsupported scheme",
+			url:           "https://example.com",
+			expectedError: `expected scheme ssh, got "https"`,
 		},
 	}
 	for _, tc := range testCases {
-		t.Run(tc.url, func(t *testing.T) {
+		t.Run(tc.doc, func(t *testing.T) {
 			sp, err := ParseURL(tc.url)
 			if tc.expectedError == "" {
 				assert.NilError(t, err)
-				assert.Check(t, is.DeepEqual(tc.expectedArgs, sp.Args()))
+				actualArgs := sp.Args(tc.remoteCommand...)
+				assert.Check(t, is.DeepEqual(actualArgs, tc.expectedArgs))
+				assert.Check(t, is.Equal(*sp, tc.expectedSpec))
 			} else {
-				assert.ErrorContains(t, err, tc.expectedError)
+				assert.Check(t, is.Error(err, tc.expectedError))
+				assert.Check(t, is.Nil(sp))
 			}
 		})
 	}