diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8d72187585a..13033bbf021 100644 --- a/.github/workflows/test.yml +++ b/.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})$") diff --git a/cli/client.go b/cli/client.go index 2f509734382..ff6c16fbc96 100644 --- a/cli/client.go +++ b/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") diff --git a/cli/client_test.go b/cli/client_test.go index 2025055eb71..2fc17e51aa9 100644 --- a/cli/client_test.go +++ b/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, diff --git a/cli/module_build_test.go b/cli/module_build_test.go index 62299f95be3..c1c330659a8 100644 --- a/cli/module_build_test.go +++ b/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 = "setup.bat", "setup.bat" + 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 diff --git a/cli/module_generate_test.go b/cli/module_generate_test.go index e97b7537acb..c1618c6b662 100644 --- a/cli/module_generate_test.go +++ b/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) diff --git a/cli/module_registry.go b/cli/module_registry.go index da6e3ba9225..24409edde0c 100644 --- a/cli/module_registry.go +++ b/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) } diff --git a/cli/module_registry_test.go b/cli/module_registry_test.go index 6e8c469907e..1d113bbcdde 100644 --- a/cli/module_registry_test.go +++ b/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, }, { diff --git a/cli/module_reload_test.go b/cli/module_reload_test.go index 5976f759685..ec65f98e391 100644 --- a/cli/module_reload_test.go +++ b/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) diff --git a/cli/test_data/.gitattributes b/cli/test_data/.gitattributes new file mode 100644 index 00000000000..1fe247887b0 --- /dev/null +++ b/cli/test_data/.gitattributes @@ -0,0 +1 @@ +*.md text eol=lf diff --git a/cli/xacro.go b/cli/xacro.go index 81d77a7e2f9..2c6eeb1126b 100644 --- a/cli/xacro.go +++ b/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 { diff --git a/cli/xacro_test.go b/cli/xacro_test.go index 3ca0080ded3..a83314f5653 100644 --- a/cli/xacro_test.go +++ b/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") }