RSDK-13980: Enable cli windows tests

.github/workflows/test.yml

@@ -378,6 +378,7 @@ jobs:
           go.viam.com/rdk/web/server
           go.viam.com/rdk/module
           go.viam.com/rdk/logging
+          go.viam.com/rdk/cli
         )
         WINDOWS_PATTERN=$(IFS='|'; echo "${WORKING_WINDOWS_TESTS[*]}")
         TEST_TARGET=$(echo "$TEST_TARGET" | grep -E "^(${WINDOWS_PATTERN})$")

cli/client.go

@@ -114,6 +114,8 @@ type viamClient struct {
 	// caches
 	orgs *[]*apppb.Organization
 	locs *[]*apppb.Location
+
+	dialOverride func(ctx context.Context, fqdn string, rpcOpts []rpc.DialOption, logger logging.Logger) (*client.RobotClient, error)
 }
 
 // ListOrganizationsAction is the corresponding Action for 'organizations list'.
@@ -5235,6 +5237,9 @@ func (c *viamClient) connectToRobot(
 	if debug {
 		printf(c.c.Root().Writer, "Establishing connection...")
 	}
+	if c.dialOverride != nil {
+		return c.dialOverride(dialCtx, fqdn, rpcOpts, logger)
+	}
 	robotClient, err := client.New(dialCtx, fqdn, logger, client.WithDialOptions(rpcOpts...))
 	if err != nil {
 		return nil, errors.Wrap(err, "could not connect to machine part")

cli/client_test.go

@@ -29,6 +29,7 @@ import (
 	commonpb "go.viam.com/api/common/v1"
 	"go.viam.com/test"
 	"go.viam.com/utils/protoutils"
+	"go.viam.com/utils/rpc"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -38,6 +39,7 @@ import (
 	robotconfig "go.viam.com/rdk/config"
 	"go.viam.com/rdk/logging"
 	"go.viam.com/rdk/resource"
+	"go.viam.com/rdk/robot/client"
 	robotimpl "go.viam.com/rdk/robot/impl"
 	"go.viam.com/rdk/services/shell"
 	_ "go.viam.com/rdk/services/shell/register"
@@ -220,6 +222,12 @@ func setupWithRunningPart(
 	ac.baseURL, _, err = utils.ParseBaseURL(ac.conf.BaseURL, false)
 	test.That(t, err, test.ShouldBeNil)
 
+	ac.dialOverride = func(ctx context.Context, fqdn string, rpcOpts []rpc.DialOption, logger logging.Logger) (*client.RobotClient, error) {
+		return client.New(ctx, addr, logger,
+			client.WithDialOptions(append(rpcOpts, rpc.WithForceDirectGRPC())...),
+		)
+	}
+
 	t.Cleanup(func() {
 		test.That(t, r.Close(context.Background()), test.ShouldBeNil)
 	})
@@ -635,7 +643,7 @@ func TestDataExportTabularAction(t *testing.T) {
 		// Test that the data.ndjson file was removed.
 		filePath := utils.ResolveFile(dataFileName)
 		_, err = os.ReadFile(filePath)
-		test.That(t, err, test.ShouldBeError, fmt.Errorf("open %s: no such file or directory", filePath))
+		test.That(t, errors.Is(err, fs.ErrNotExist), test.ShouldBeTrue)
 	})
 }
 
@@ -975,6 +983,9 @@ func TestMachinesPartHistoryAction(t *testing.T) {
 }
 
 func TestShellFileCopy(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("RSDK-11617")
+	}
 	logger := logging.NewTestLogger(t)
 
 	listOrganizationsFunc := func(ctx context.Context, in *apppb.ListOrganizationsRequest,

cli/module_build_test.go

@@ -9,6 +9,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"runtime"
 	"sort"
 	"strings"
 	"testing"
@@ -265,23 +266,25 @@ func TestLocalBuild(t *testing.T) {
 	testDir := t.TempDir()
 	testChdir(t, testDir)
 
-	// write manifest and setup.sh
-	// the manifest contains a:
-	// "setup": "./setup.sh"
-	// and a "build": "make build"
-	manifestPath := createTestManifest(t, "", nil)
-	err := os.WriteFile(
-		filepath.Join(testDir, "setup.sh"),
-		[]byte("echo setup step msg"),
-		0o700,
-	)
+	setupScriptCmd, setupFile, setupContent := "./setup.sh", "setup.sh", "echo setup step msg"
+	buildCmd, buildFile, buildContent := "make build", "Makefile", "make build:\n\techo build step msg"
+	if runtime.GOOS == "windows" {
+		setupScriptCmd, setupFile, setupContent = "setup.bat", "setup.bat", "@echo setup step msg"
+		buildCmd, buildFile, buildContent = "build.bat", "build.bat", "@echo build step msg"
+	}
+
+	manifestPath := createTestManifest(t, "", map[string]any{
+		"build": map[string]any{
+			"setup": setupScriptCmd,
+			"build": buildCmd,
+			"path":  "module",
+			"arch":  []any{"linux/amd64"},
+		},
+	})
+	err := os.WriteFile(filepath.Join(testDir, setupFile), []byte(setupContent), 0o700)
 	test.That(t, err, test.ShouldBeNil)
 
-	err = os.WriteFile(
-		filepath.Join(testDir, "Makefile"),
-		[]byte("make build:\n\techo build step msg"),
-		0o700,
-	)
+	err = os.WriteFile(filepath.Join(testDir, buildFile), []byte(buildContent), 0o700)
 	test.That(t, err, test.ShouldBeNil)
 
 	// run the build local action

cli/module_generate_test.go

@@ -122,9 +122,13 @@ func TestGenerateModuleAction(t *testing.T) {
 		test.That(t, err, test.ShouldBeNil)
 		_, err = os.Stat(filepath.Join(modulePath, "src"))
 		test.That(t, err, test.ShouldBeNil)
-		_, err = os.Stat(filepath.Join(modulePath, "build.sh"))
+		scriptExt := ".sh"
+		if runtime.GOOS == "windows" {
+			scriptExt = ".bat"
+		}
+		_, err = os.Stat(filepath.Join(modulePath, "build"+scriptExt))
 		test.That(t, err, test.ShouldBeNil)
-		_, err = os.Stat(filepath.Join(modulePath, "setup.sh"))
+		_, err = os.Stat(filepath.Join(modulePath, "setup"+scriptExt))
 		test.That(t, err, test.ShouldBeNil)
 		_, err = os.Stat(filepath.Join(modulePath, ".gitignore"))
 		test.That(t, err, test.ShouldBeNil)

cli/module_registry.go

@@ -885,6 +885,7 @@ func writeManifest(manifestPath string, manifest ModuleManifest) error {
 	if err != nil {
 		return errors.Wrapf(err, "failed to create %s", manifestPath)
 	}
+	defer func() { vutils.UncheckedError(manifestFile.Close()) }()
 	if _, err := manifestFile.Write(manifestBytes); err != nil {
 		return errors.Wrapf(err, "failed to write manifest to %s", manifestPath)
 	}

cli/module_registry_test.go

@@ -267,7 +267,7 @@ func TestGetMarkdownContent(t *testing.T) {
 			name:            "non-existent file",
 			filepath:        "non_existent_file.md",
 			shouldContain:   []string{},
-			shouldErrorWith: "failed to read markdown file at non_existent_file.md: open non_existent_file.md: no such file or directory",
+			shouldErrorWith: "failed to read markdown file at non_existent_file.md: open non_existent_file.md:",
 			totalChars:      0,
 		},
 		{

cli/module_reload_test.go

@@ -379,6 +379,7 @@ func TestResolvePartId(t *testing.T) {
 	test.That(t, err, test.ShouldBeNil)
 	_, err = fi.WriteString(`{"cloud":{"app_address":"https://app.viam.com:443","id":"JSON-PART","secret":"SECRET"}}`)
 	test.That(t, err, test.ShouldBeNil)
+	test.That(t, fi.Close(), test.ShouldBeNil)
 	partID, err = resolvePartID(c.String(generalFlagPartID), path)
 	test.That(t, err, test.ShouldBeNil)
 	test.That(t, partID, test.ShouldEqual, "JSON-PART")
@@ -399,7 +400,9 @@ func TestMutateModuleConfig(t *testing.T) {
 	}
 	expectedName := "viam-labs_test-module_from_reload"
 	expectedVersion := "latest-with-prerelease"
-	remoteReloadPath := ".viam/packages-local/viam-labs_test-module_from_reload-module.tar.gz"
+	expectedEntrypoint, err := filepath.Abs(manifest.Entrypoint)
+	test.That(t, err, test.ShouldBeNil)
+	remoteReloadPath := filepath.Join(".viam", "packages-local", "viam-labs_test-module_from_reload-module.tar.gz")
 	testUser := "test@viam.com"
 	testReloadUnixTS := time.Date(2024, 3, 18, 12, 0, 0, 0, time.UTC).Unix()
 
@@ -448,7 +451,7 @@ func TestMutateModuleConfig(t *testing.T) {
 		test.That(t, err, test.ShouldBeNil)
 		test.That(t, dirty, test.ShouldBeTrue)
 		test.That(t, needsRestart, test.ShouldBeFalse)
-		test.That(t, modules[0]["reload_path"], test.ShouldEqual, manifest.Entrypoint)
+		test.That(t, modules[0]["reload_path"], test.ShouldEqual, expectedEntrypoint)
 		test.That(t, modules[0]["reload_enabled"], test.ShouldBeTrue)
 		test.That(t, modules[0]["reload_user"], test.ShouldEqual, testUser)
 		test.That(t, modules[0]["reload_time"], test.ShouldNotBeEmpty)
@@ -464,7 +467,7 @@ func TestMutateModuleConfig(t *testing.T) {
 		test.That(t, err, test.ShouldBeNil)
 		test.That(t, dirty, test.ShouldBeTrue)
 		test.That(t, needsRestart, test.ShouldBeFalse)
-		test.That(t, modules[0]["reload_path"], test.ShouldEqual, manifest.Entrypoint)
+		test.That(t, modules[0]["reload_path"], test.ShouldEqual, expectedEntrypoint)
 		test.That(t, modules[0]["reload_enabled"], test.ShouldBeTrue)
 		test.That(t, modules[0]["reload_user"], test.ShouldEqual, testUser)
 		test.That(t, modules[0]["reload_time"], test.ShouldNotBeEmpty)
@@ -475,7 +478,7 @@ func TestMutateModuleConfig(t *testing.T) {
 		modules, _, _, _ = mutateModuleConfig(c, modules, manifest, true, false, testUser, "", testReloadUnixTS)
 		test.That(t, modules[0]["module_id"], test.ShouldEqual, manifest.ModuleID)
 		test.That(t, modules[0]["name"], test.ShouldEqual, expectedName)
-		test.That(t, modules[0]["reload_path"], test.ShouldEqual, manifest.Entrypoint)
+		test.That(t, modules[0]["reload_path"], test.ShouldEqual, expectedEntrypoint)
 		test.That(t, modules[0]["reload_enabled"], test.ShouldBeTrue)
 		test.That(t, modules[0]["version"], test.ShouldEqual, expectedVersion)
 		test.That(t, modules[0]["reload_user"], test.ShouldEqual, testUser)
@@ -490,7 +493,7 @@ func TestMutateModuleConfig(t *testing.T) {
 		}}
 		updatedModules, _, _, _ := mutateModuleConfig(c, modules, manifest, true, false, testUser, "", testReloadUnixTS)
 		test.That(t, len(updatedModules), test.ShouldEqual, 2)
-		test.That(t, updatedModules[1]["reload_path"], test.ShouldEqual, manifest.Entrypoint)
+		test.That(t, updatedModules[1]["reload_path"], test.ShouldEqual, expectedEntrypoint)
 		test.That(t, updatedModules[1]["reload_enabled"], test.ShouldBeTrue)
 		test.That(t, updatedModules[1]["version"], test.ShouldEqual, expectedVersion)
 		test.That(t, updatedModules[1]["reload_user"], test.ShouldEqual, testUser)

cli/test_data/.gitattributes

@@ -0,0 +1 @@
+*.md text eol=lf

cli/xacro.go

@@ -243,11 +243,9 @@ func validateOutputWritable(outputPath string) error {
 	if err != nil {
 		return err
 	}
-	defer func() {
-		if closeErr := f.Close(); closeErr != nil {
-			err = closeErr
-		}
-	}()
+	if err := f.Close(); err != nil {
+		return err
+	}
 
 	// Remove empty test file (don't leave artifacts if we just created it)
 	if info, statErr := os.Stat(outputPath); statErr == nil && info.Size() == 0 {

cli/xacro_test.go

@@ -3,6 +3,7 @@ package cli
 import (
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -108,6 +109,9 @@ func TestExtractPackageName(t *testing.T) {
 }
 
 func TestProcessXacroArgs(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("RSDK-13917")
+	}
 	tmpDir := t.TempDir()
 	pkgName := "test_pkg"
 
@@ -407,7 +411,7 @@ func TestValidateOutputWritable(t *testing.T) {
 	})
 
 	t.Run("directory does not exist", func(t *testing.T) {
-		err := validateOutputWritable("/nonexistent/dir/output.urdf")
+		err := validateOutputWritable(filepath.Join(t.TempDir(), "nonexistent", "output.urdf"))
 		test.That(t, err, test.ShouldNotBeNil)
 		test.That(t, err.Error(), test.ShouldContainSubstring, "does not exist")
 	})
@@ -428,6 +432,9 @@ func TestValidateOutputWritable(t *testing.T) {
 	})
 
 	t.Run("read-only directory", func(t *testing.T) {
+		if runtime.GOOS == "windows" {
+			t.Skip("Windows doesn't use permission bits to determine read-only directories")
+		}
 		if os.Getuid() == 0 {
 			t.Skip("Skipping read-only test when running as root")
 		}