diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 0d602e37..d54e9d54 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -59,3 +59,20 @@ jobs: # the file with `test_registry` also requires windows, so we can safely add this tag # without it running on non-windows OSes run: go test -v -tags test_registry ./... + + # User and System level tests + - name: Run Tests (User) + run: go test -v ./... + env: + DBC_TEST_LEVEL_USER: 1 + - name: Run Tests (System, Unixlike) + # Run system tests with sudo on Unixlikes to replicate what users do + if: runner.os != 'Windows' + run: sudo -E go test -v ./... + env: + DBC_TEST_LEVEL_SYSTEM: 1 + - name: Run Tests (System, Windows) + if: runner.os == 'Windows' + run: go test -v ./... + env: + DBC_TEST_LEVEL_SYSTEM: 1 diff --git a/cmd/dbc/install_test.go b/cmd/dbc/install_test.go index cf16479b..74eaa9bc 100644 --- a/cmd/dbc/install_test.go +++ b/cmd/dbc/install_test.go @@ -23,21 +23,20 @@ import ( ) func (suite *SubcommandTestSuite) TestInstall() { - m := InstallCmd{Driver: "test-driver-1", Level: config.ConfigEnv}. + m := InstallCmd{Driver: "test-driver-1", Level: suite.configLevel}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) out := suite.runCmd(m) suite.validateOutput("\r[✓] searching\r\n[✓] downloading\r\n[✓] installing\r\n[✓] verifying signature\r\n", - "\nInstalled test-driver-1 1.1.0 to "+suite.tempdir+"\n", out) - if runtime.GOOS != "windows" { - suite.FileExists(filepath.Join(suite.tempdir, "test-driver-1.toml")) - } + "\nInstalled test-driver-1 1.1.0 to "+suite.Dir()+"\n", out) + suite.driverIsInstalled("test-driver-1", true) } func (suite *SubcommandTestSuite) TestInstallDriverNotFound() { - m := InstallCmd{Driver: "foo", Level: config.ConfigEnv}. + m := InstallCmd{Driver: "foo", Level: suite.configLevel}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) suite.validateOutput("Error: could not find driver: driver `foo` not found in driver registry index\r\n\r ", "", suite.runCmdErr(m)) + suite.driverIsNotInstalled("test-driver-1") } func (suite *SubcommandTestSuite) TestInstallWithVersion() { @@ -54,13 +53,14 @@ func (suite *SubcommandTestSuite) TestInstallWithVersion() { for _, tt := range tests { suite.Run(tt.driver, func() { - m := InstallCmd{Driver: tt.driver}. + m := InstallCmd{Driver: tt.driver, Level: suite.configLevel}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) out := suite.runCmd(m) - suite.validateOutput("\r[✓] searching\r\n[✓] downloading\r\n[✓] installing\r\n[✓] verifying signature\r\n", - "\nInstalled test-driver-1 "+tt.expectedVersion+" to "+suite.tempdir+"\n", out) - m = UninstallCmd{Driver: "test-driver-1"}.GetModelCustom( + suite.validateOutput("\r[✓] searching\r\n[✓] downloading\r\n[✓] installing\r\n[✓] verifying signature\r\n", + "\nInstalled test-driver-1 "+tt.expectedVersion+" to "+suite.Dir()+"\n", out) + suite.driverIsInstalled("test-driver-1", true) + m = UninstallCmd{Driver: "test-driver-1", Level: suite.configLevel}.GetModelCustom( baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) suite.runCmd(m) }) @@ -91,59 +91,9 @@ func (suite *SubcommandTestSuite) TestReinstallUpdateVersion() { "test-driver-1.1/test-driver-1-not-valid.so.sig", "test-driver-1.toml"}, suite.getFilesInTempDir()) } -func (suite *SubcommandTestSuite) TestInstallUserFake() { - if runtime.GOOS == "windows" { - suite.T().Skip() - } - - os.Unsetenv("ADBC_DRIVER_PATH") - - m := InstallCmd{Driver: "test-driver-1"}. - GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) - installModel := m.(progressiveInstallModel) - suite.Equal(installModel.cfg.Level, config.ConfigUser) - installModel.cfg.Location = filepath.Join(suite.tempdir, "root", installModel.cfg.Location) - m = installModel // <- We need to reassign to make the change stick - suite.runCmd(m) - suite.FileExists(filepath.Join(installModel.cfg.Location, "test-driver-1.toml")) -} - -func (suite *SubcommandTestSuite) TestInstallUserFakeExplicit() { - if runtime.GOOS == "windows" { - suite.T().Skip() - } - - os.Unsetenv("ADBC_DRIVER_PATH") - - m := InstallCmd{Driver: "test-driver-1", Level: config.ConfigUser}. - GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) - installModel := m.(progressiveInstallModel) - suite.Equal(installModel.cfg.Level, config.ConfigUser) - installModel.cfg.Location = filepath.Join(suite.tempdir, "root", installModel.cfg.Location) - m = installModel // <- We need to reassign to make the change stick - suite.runCmd(m) - suite.FileExists(filepath.Join(installModel.cfg.Location, "test-driver-1.toml")) -} - -func (suite *SubcommandTestSuite) TestInstallSystemFake() { - if runtime.GOOS == "windows" { - suite.T().Skip() - } - - m := InstallCmd{Driver: "test-driver-1", Level: config.ConfigSystem}. - GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) - installModel := m.(progressiveInstallModel) - suite.Equal(installModel.cfg.Level, config.ConfigSystem) - installModel.cfg.Location = filepath.Join(suite.tempdir, "root", installModel.cfg.Location) - m = installModel // <- We need to reassign to make the change stick - suite.runCmd(m) - suite.FileExists(filepath.Join(installModel.cfg.Location, "test-driver-1.toml")) -} - func (suite *SubcommandTestSuite) TestInstallVenv() { - os.Unsetenv("ADBC_DRIVER_PATH") - os.Setenv("VIRTUAL_ENV", suite.tempdir) - defer os.Unsetenv("VIRTUAL_ENV") + suite.T().Setenv("ADBC_DRIVER_PATH", "") + suite.T().Setenv("VIRTUAL_ENV", suite.tempdir) m := InstallCmd{Driver: "test-driver-1"}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) @@ -158,9 +108,10 @@ func (suite *SubcommandTestSuite) TestInstallEnvironmentPrecedence() { driver_path := filepath.Join(suite.tempdir, "driver_path") venv_path := filepath.Join(suite.tempdir, "venv_path") conda_path := filepath.Join(suite.tempdir, "conda_path") - os.Setenv("ADBC_DRIVER_PATH", driver_path) - os.Setenv("VIRTUAL_ENV", venv_path) - os.Setenv("CONDA_PREFIX", conda_path) + + suite.T().Setenv("ADBC_DRIVER_PATH", driver_path) + suite.T().Setenv("VIRTUAL_ENV", venv_path) + suite.T().Setenv("CONDA_PREFIX", conda_path) m := InstallCmd{Driver: "test-driver-1", Level: config.ConfigEnv}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) @@ -170,26 +121,23 @@ func (suite *SubcommandTestSuite) TestInstallEnvironmentPrecedence() { suite.NoFileExists(filepath.Join(venv_path, "test-driver-1.toml")) suite.NoFileExists(filepath.Join(conda_path, "test-driver-1.toml")) - os.Unsetenv("ADBC_DRIVER_PATH") + suite.T().Setenv("ADBC_DRIVER_PATH", "") m = InstallCmd{Driver: "test-driver-1", Level: config.ConfigEnv}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) suite.runCmd(m) suite.FileExists(filepath.Join(venv_path, "etc", "adbc", "drivers", "test-driver-1.toml")) suite.NoFileExists(filepath.Join(conda_path, "etc", "adbc", "drivers", "test-driver-1.toml")) - os.Unsetenv("VIRTUAL_ENV") + suite.T().Setenv("VIRTUAL_ENV", "") m = InstallCmd{Driver: "test-driver-1", Level: config.ConfigEnv}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) suite.runCmd(m) suite.FileExists(filepath.Join(conda_path, "etc", "adbc", "drivers", "test-driver-1.toml")) - - os.Unsetenv("CONDA_PREFIX") } func (suite *SubcommandTestSuite) TestInstallCondaPrefix() { - os.Unsetenv("ADBC_DRIVER_PATH") - os.Setenv("CONDA_PREFIX", suite.tempdir) - defer os.Unsetenv("CONDA_PREFIX") + suite.T().Setenv("ADBC_DRIVER_PATH", "") + suite.T().Setenv("CONDA_PREFIX", suite.tempdir) m := InstallCmd{Driver: "test-driver-1"}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) @@ -197,32 +145,14 @@ func (suite *SubcommandTestSuite) TestInstallCondaPrefix() { "\nInstalled test-driver-1 1.1.0 to "+filepath.Join(suite.tempdir, "etc", "adbc", "drivers")+"\n", suite.runCmd(m)) } -func (suite *SubcommandTestSuite) TestInstallUserFakeExplicitLevelOverrides() { - if runtime.GOOS == "windows" { - suite.T().Skip() - } - - // If the user explicitly sets level, it should override ADBC_DRIVER_PATH - // which, when testing, is set to tempdir - m := InstallCmd{Driver: "test-driver-1", Level: config.ConfigSystem}. - GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) - installModel := m.(progressiveInstallModel) - suite.Equal(installModel.cfg.Level, config.ConfigSystem) - installModel.cfg.Location = filepath.Join(suite.tempdir, "user", installModel.cfg.Location) - m = installModel // <- We need to reassign to make the change stick - suite.runCmd(m) - suite.FileExists(filepath.Join(installModel.cfg.Location, "test-driver-1.toml")) -} - func (suite *SubcommandTestSuite) TestInstallManifestOnlyDriver() { - m := InstallCmd{Driver: "test-driver-manifest-only"}. + m := InstallCmd{Driver: "test-driver-manifest-only", Level: suite.configLevel}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) + suite.validateOutput("\r[✓] searching\r\n[✓] downloading\r\n[✓] installing\r\n[✓] verifying signature\r\n", - "\nInstalled test-driver-manifest-only 1.0.0 to "+suite.tempdir+"\n"+ + "\nInstalled test-driver-manifest-only 1.0.0 to "+suite.Dir()+"\n"+ "\nMust have libtest_driver installed to load this driver\n", suite.runCmd(m)) - if runtime.GOOS != "windows" { - suite.FileExists(filepath.Join(suite.tempdir, "test-driver-manifest-only.toml")) - } + suite.driverIsInstalled("test-driver-manifest-only", false) } func (suite *SubcommandTestSuite) TestInstallDriverNoSignature() { @@ -243,8 +173,7 @@ func (suite *SubcommandTestSuite) TestInstallDriverNoSignature() { func (suite *SubcommandTestSuite) TestInstallGitignoreDefaultBehavior() { driver_path := filepath.Join(suite.tempdir, "driver_path") ignorePath := filepath.Join(driver_path, ".gitignore") - os.Setenv("ADBC_DRIVER_PATH", driver_path) - defer os.Unsetenv("ADBC_DRIVER_PATH") + suite.T().Setenv("ADBC_DRIVER_PATH", driver_path) suite.NoFileExists(ignorePath) @@ -258,8 +187,7 @@ func (suite *SubcommandTestSuite) TestInstallGitignoreDefaultBehavior() { func (suite *SubcommandTestSuite) TestInstallGitignoreExisingDir() { driver_path := filepath.Join(suite.tempdir, "driver_path") ignorePath := filepath.Join(driver_path, ".gitignore") - os.Setenv("ADBC_DRIVER_PATH", driver_path) - defer os.Unsetenv("ADBC_DRIVER_PATH") + suite.T().Setenv("ADBC_DRIVER_PATH", driver_path) // Create the directory before we install the driver mkdirerr := os.MkdirAll(driver_path, 0o755) @@ -282,8 +210,7 @@ func (suite *SubcommandTestSuite) TestInstallGitignoreExisingDir() { func (suite *SubcommandTestSuite) TestInstallGitignorePreserveUserModified() { driver_path := filepath.Join(suite.tempdir, "driver_path") ignorePath := filepath.Join(driver_path, ".gitignore") - os.Setenv("ADBC_DRIVER_PATH", driver_path) - defer os.Unsetenv("ADBC_DRIVER_PATH") + suite.T().Setenv("ADBC_DRIVER_PATH", driver_path) suite.NoFileExists(ignorePath) @@ -316,3 +243,22 @@ func (suite *SubcommandTestSuite) TestInstallGitignorePreserveUserModified() { } suite.Equal(userContent, string(data)) } + +func (suite *SubcommandTestSuite) TestInstallCreatesSymlinks() { + if runtime.GOOS == "windows" && (suite.configLevel == config.ConfigUser || suite.configLevel == config.ConfigSystem) { + suite.T().Skip("Symlinks aren't created on Windows for User and System config levels") + } + + // Install a driver + m := InstallCmd{Driver: "test-driver-1", Level: suite.configLevel}. + GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) + _ = suite.runCmd(m) + suite.driverIsInstalled("test-driver-1", true) + + // Verify symlink is in place in the parent dir and is actually a symlink + manifestPath := filepath.Join(suite.Dir(), "..", "test-driver-1.toml") + suite.FileExists(manifestPath) + info, err := os.Lstat(manifestPath) + suite.NoError(err) + suite.Equal(os.ModeSymlink, info.Mode()&os.ModeSymlink, "Expected test-driver-1.toml to be a symlink") +} diff --git a/cmd/dbc/subcommand_test.go b/cmd/dbc/subcommand_test.go index 901bc30d..692af705 100644 --- a/cmd/dbc/subcommand_test.go +++ b/cmd/dbc/subcommand_test.go @@ -27,6 +27,7 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/columnar-tech/dbc" + "github.com/columnar-tech/dbc/config" "github.com/go-faster/yaml" "github.com/stretchr/testify/suite" ) @@ -83,6 +84,8 @@ type SubcommandTestSuite struct { openBrowserFn func(string) error fallbackDriverDocsUrl map[string]string tempdir string + + configLevel config.ConfigLevel } func (suite *SubcommandTestSuite) SetupSuite() { @@ -90,15 +93,15 @@ func (suite *SubcommandTestSuite) SetupSuite() { getDriverRegistry = getTestDriverRegistry suite.openBrowserFn = openBrowserFunc suite.fallbackDriverDocsUrl = fallbackDriverDocsUrl + + if suite.configLevel == config.ConfigUnknown { + suite.configLevel = config.ConfigEnv + } } func (suite *SubcommandTestSuite) SetupTest() { suite.tempdir = suite.T().TempDir() - suite.Require().NoError(os.Setenv("ADBC_DRIVER_PATH", suite.tempdir)) -} - -func (suite *SubcommandTestSuite) TearDownTest() { - suite.Require().NoError(os.Unsetenv("ADBC_DRIVER_PATH")) + suite.T().Setenv("ADBC_DRIVER_PATH", suite.tempdir) } func (suite *SubcommandTestSuite) TearDownSuite() { @@ -120,6 +123,15 @@ func (suite *SubcommandTestSuite) getFilesInTempDir() []string { return filelist } +// Get the base directory for where drivers are installed. Use this instead of +// hardcoding checks to suite.tempdir to make tests support other config levels. +func (suite *SubcommandTestSuite) Dir() string { + if suite.configLevel == config.ConfigEnv { + return suite.tempdir + } + return suite.configLevel.ConfigLocation() +} + func (suite *SubcommandTestSuite) runCmdErr(m tea.Model) string { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() @@ -171,6 +183,64 @@ func (suite *SubcommandTestSuite) validateOutput(expected, extra, actual string) suite.Equal(terminalPrefix+expected+terminalSuffix+extra, actual) } -func TestSubcommands(t *testing.T) { - suite.Run(t, new(SubcommandTestSuite)) +// The SubcommandTestSuite is only run for ConfigEnv by default but is +// parametrized by configLevel so tests can be run for other levels. Tests must +// opt into this behavior by instantiating subcommands with `suite.configLevel` +// like: +// +// m := InstallCmd{Driver: "foo", Level: suite.configLevel} +// ^---- here +// +// and can opt out of this behavior by specifying it separately like: +// +// m := InstallCmd{Driver: "test-driver-1", Level: config.ConfigEnv}. +// +// When any level is explicitly requested, tests are only run for that level. +// i.e., to run tests for multiple levels, each level must be specified +// separately. +func TestSubcommandsEnv(t *testing.T) { + _, env := os.LookupEnv("DBC_TEST_LEVEL_ENV") + _, user := os.LookupEnv("DBC_TEST_LEVEL_USER") + _, system := os.LookupEnv("DBC_TEST_LEVEL_SYSTEM") + + // Run if explicitly requested, or if no levels were requested (default + // behavior) + if env || (!user && !system) { + suite.Run(t, &SubcommandTestSuite{configLevel: config.ConfigEnv}) + return + } + t.Skip("skipping tests for config level: ConfigEnv") +} + +func TestSubcommandsUser(t *testing.T) { + if _, ok := os.LookupEnv("DBC_TEST_LEVEL_USER"); !ok { + t.Skip("skipping tests for config level: ConfigUser") + } + suite.Run(t, &SubcommandTestSuite{configLevel: config.ConfigUser}) +} + +func TestSubcommandsSystem(t *testing.T) { + if _, ok := os.LookupEnv("DBC_TEST_LEVEL_SYSTEM"); !ok { + t.Skip("skipping tests for config level: ConfigSystem") + } + suite.Run(t, &SubcommandTestSuite{configLevel: config.ConfigSystem}) +} + +func (suite *SubcommandTestSuite) driverIsInstalled(path string, checkShared bool) { + cfg := config.Get()[suite.configLevel] + + driver, err := config.GetDriver(cfg, path) + suite.Require().NoError(err, "driver manifest should exist for driver `%s`", path) + + if checkShared { + sharedPath := driver.Driver.Shared.Get(config.PlatformTuple()) + suite.FileExists(sharedPath, "driver shared library should exist for driver `%s`", path) + } +} + +func (suite *SubcommandTestSuite) driverIsNotInstalled(path string) { + cfg := config.Get()[suite.configLevel] + + _, err := config.GetDriver(cfg, path) + suite.Require().Error(err, "driver manifest should not exist for driver `%s`", path) } diff --git a/cmd/dbc/subcommand_unix_test.go b/cmd/dbc/subcommand_unix_test.go new file mode 100644 index 00000000..6e16b34c --- /dev/null +++ b/cmd/dbc/subcommand_unix_test.go @@ -0,0 +1,35 @@ +// Copyright 2025 Columnar Technologies Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !windows + +package main + +import ( + "os" + + "github.com/columnar-tech/dbc/config" +) + +func (suite *SubcommandTestSuite) TearDownTest() { + // Clean up filesystem after each test + _, user := os.LookupEnv("DBC_TEST_LEVEL_USER") + _, system := os.LookupEnv("DBC_TEST_LEVEL_SYSTEM") + if user { + suite.Require().NoError(os.RemoveAll(config.ConfigUser.ConfigLocation())) + } + if system { + suite.Require().NoError(os.RemoveAll(config.ConfigSystem.ConfigLocation())) + } +} diff --git a/cmd/dbc/subcommand_windows_test.go b/cmd/dbc/subcommand_windows_test.go new file mode 100644 index 00000000..bdcecaa0 --- /dev/null +++ b/cmd/dbc/subcommand_windows_test.go @@ -0,0 +1,69 @@ +// Copyright 2025 Columnar Technologies Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package main + +import ( + "errors" + "io" + "os" + + "github.com/columnar-tech/dbc/config" + "golang.org/x/sys/windows/registry" +) + +func (suite *SubcommandTestSuite) TearDownTest() { + // Clean up the registry and filesystem after each test + _, user := os.LookupEnv("DBC_TEST_LEVEL_USER") + _, system := os.LookupEnv("DBC_TEST_LEVEL_SYSTEM") + + if user { + suite.Require().NoError(deleteRegistryKeyRecursive(registry.CURRENT_USER, "SOFTWARE\\ADBC\\Drivers")) + suite.Require().NoError(os.RemoveAll(config.Get()[config.ConfigUser].Location)) + } + if system { + suite.Require().NoError(deleteRegistryKeyRecursive(registry.LOCAL_MACHINE, "SOFTWARE\\ADBC\\Drivers")) + suite.Require().NoError(os.RemoveAll(config.Get()[config.ConfigSystem].Location)) + } +} + +// recursively deletes a registry key and all its subkeys +// TODO: Somewhat duplicated with clearRegistry in registry_test.go +// This is slightly more aggressive in that it deletes the top level key too +func deleteRegistryKeyRecursive(root registry.Key, path string) error { + k, err := registry.OpenKey(root, path, registry.ALL_ACCESS) + if err != nil { + if errors.Is(err, registry.ErrNotExist) { + return nil + } + return err + } + defer k.Close() + + // Delete all subkeys + subkeys, err := k.ReadSubKeyNames(-1) + if err != nil && !errors.Is(err, io.EOF) { + return err + } + for _, subkey := range subkeys { + if err := registry.DeleteKey(k, subkey); err != nil { + return err + } + } + + // Delete the top level key + return registry.DeleteKey(root, path) +} diff --git a/cmd/dbc/sync_test.go b/cmd/dbc/sync_test.go index 97eb0e52..54a3b654 100644 --- a/cmd/dbc/sync_test.go +++ b/cmd/dbc/sync_test.go @@ -76,7 +76,7 @@ func (suite *SubcommandTestSuite) TestSyncWithVersion() { } func (suite *SubcommandTestSuite) TestSyncVirtualEnv() { - os.Unsetenv("ADBC_DRIVER_PATH") + suite.T().Setenv("ADBC_DRIVER_PATH", "") m := InitCmd{Path: filepath.Join(suite.tempdir, "dbc.toml")}.GetModel() suite.runCmd(m) @@ -84,8 +84,7 @@ func (suite *SubcommandTestSuite) TestSyncVirtualEnv() { m = AddCmd{Path: filepath.Join(suite.tempdir, "dbc.toml"), Driver: "test-driver-1"}.GetModel() suite.runCmd(m) - os.Setenv("VIRTUAL_ENV", suite.tempdir) - defer os.Unsetenv("VIRTUAL_ENV") + suite.T().Setenv("VIRTUAL_ENV", suite.tempdir) m = SyncCmd{ Path: filepath.Join(suite.tempdir, "dbc.toml"), @@ -102,7 +101,7 @@ func (suite *SubcommandTestSuite) TestSyncVirtualEnv() { } func (suite *SubcommandTestSuite) TestSyncCondaPrefix() { - os.Unsetenv("ADBC_DRIVER_PATH") + suite.T().Setenv("ADBC_DRIVER_PATH", "") m := InitCmd{Path: filepath.Join(suite.tempdir, "dbc.toml")}.GetModel() suite.runCmd(m) @@ -110,8 +109,7 @@ func (suite *SubcommandTestSuite) TestSyncCondaPrefix() { m = AddCmd{Path: filepath.Join(suite.tempdir, "dbc.toml"), Driver: "test-driver-1"}.GetModel() suite.runCmd(m) - os.Setenv("CONDA_PREFIX", suite.tempdir) - defer os.Unsetenv("CONDA_PREFIX") + suite.T().Setenv("CONDA_PREFIX", suite.tempdir) m = SyncCmd{ Path: filepath.Join(suite.tempdir, "dbc.toml"), diff --git a/cmd/dbc/uninstall_test.go b/cmd/dbc/uninstall_test.go index f8bf4427..ca8174c9 100644 --- a/cmd/dbc/uninstall_test.go +++ b/cmd/dbc/uninstall_test.go @@ -140,31 +140,28 @@ func (suite *SubcommandTestSuite) TestUninstallMultipleLocationsNonDefault() { } func (suite *SubcommandTestSuite) TestUninstallManifestOnlyDriver() { - m := InstallCmd{Driver: "test-driver-manifest-only"}. + m := InstallCmd{Driver: "test-driver-manifest-only", Level: suite.configLevel}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) + suite.validateOutput("\r[✓] searching\r\n[✓] downloading\r\n[✓] installing\r\n[✓] verifying signature\r\n", - "\nInstalled test-driver-manifest-only 1.0.0 to "+suite.tempdir+"\n"+ + "\nInstalled test-driver-manifest-only 1.0.0 to "+suite.Dir()+"\n"+ "\nMust have libtest_driver installed to load this driver\n", suite.runCmd(m)) - if runtime.GOOS != "windows" { - suite.FileExists(filepath.Join(suite.tempdir, "test-driver-manifest-only.toml")) - } + suite.driverIsInstalled("test-driver-manifest-only", false) // Verify the sidecar folder exists before we uninstall new_sidecar_path := fmt.Sprintf("test-driver-manifest-only_%s_v1.0.0", config.PlatformTuple()) - err := os.Rename(filepath.Join(suite.tempdir, "test-driver-manifest-only"), filepath.Join(suite.tempdir, new_sidecar_path)) + err := os.Rename(filepath.Join(suite.Dir(), "test-driver-manifest-only"), filepath.Join(suite.Dir(), new_sidecar_path)) if err != nil { suite.Fail(fmt.Sprintf("Failed to rename sidecar folder. Something is wrong with this test: %v", err)) } - suite.DirExists(filepath.Join(suite.tempdir, new_sidecar_path)) + suite.DirExists(filepath.Join(suite.Dir(), new_sidecar_path)) // Now uninstall and verify we clean up - m = UninstallCmd{Driver: "test-driver-manifest-only"}. + m = UninstallCmd{Driver: "test-driver-manifest-only", Level: suite.configLevel}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) suite.validateOutput("Driver `test-driver-manifest-only` uninstalled successfully!\r\n\r\n\r ", "", suite.runCmd(m)) - if runtime.GOOS != "windows" { - suite.NoFileExists(filepath.Join(suite.tempdir, "test-driver-manifest-only.toml")) - } - suite.NoDirExists(filepath.Join(suite.tempdir, new_sidecar_path)) + suite.driverIsNotInstalled("test-driver-manifest-only") + suite.NoDirExists(filepath.Join(suite.Dir(), new_sidecar_path)) } // See https://github.com/columnar-tech/dbc/issues/37 @@ -172,10 +169,11 @@ func (suite *SubcommandTestSuite) TestUninstallInvalidManifest() { if runtime.GOOS == "windows" { suite.T().Skip() } - m := InstallCmd{Driver: "test-driver-invalid-manifest", Level: config.ConfigEnv}. + + m := InstallCmd{Driver: "test-driver-invalid-manifest", Level: suite.configLevel}. GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) suite.runCmd(m) - suite.FileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest.toml")) + suite.FileExists(filepath.Join(suite.Dir(), "test-driver-invalid-manifest.toml")) // The installed manifest should have a Driver.shared set to a folder, not the .so // We only need a partial struct definition to read in the Driver.shared table @@ -185,7 +183,7 @@ func (suite *SubcommandTestSuite) TestUninstallInvalidManifest() { } } var invalidManifest partialManifest - f, err := os.Open(filepath.Join(suite.tempdir, "test-driver-invalid-manifest.toml")) + f, err := os.Open(filepath.Join(suite.Dir(), "test-driver-invalid-manifest.toml")) if err != nil { suite.Error(err) } @@ -198,17 +196,43 @@ func (suite *SubcommandTestSuite) TestUninstallInvalidManifest() { suite.DirExists(value) // and continue - m = UninstallCmd{Driver: "test-driver-invalid-manifest", Level: config.ConfigEnv}.GetModel() + m = UninstallCmd{Driver: "test-driver-invalid-manifest", Level: suite.configLevel}.GetModel() output := suite.runCmd(m) suite.validateOutput("Driver `test-driver-invalid-manifest` uninstalled successfully!\r\n\r\n\r ", "", output) - // Ensure we don't nuke the tempdir which is the original (major) issue - suite.DirExists(suite.tempdir) + // Ensure we don't nuke the installation directory which is the original (major) issue + suite.DirExists(suite.Dir()) // We do remove the manifest - suite.NoFileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest.toml")) + suite.NoFileExists(filepath.Join(suite.Dir(), "test-driver-invalid-manifest.toml")) // But we don't remove the driver shared folder in this edge case, so we assert // they're still around - suite.FileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest", "libadbc_driver_invalid_manifest.so")) + suite.FileExists(filepath.Join(suite.Dir(), "test-driver-invalid-manifest", "libadbc_driver_invalid_manifest.so")) +} + +func (suite *SubcommandTestSuite) TestUninstallRemovesSymlink() { + if runtime.GOOS == "windows" && (suite.configLevel == config.ConfigUser || suite.configLevel == config.ConfigSystem) { + suite.T().Skip("Symlinks aren't created on Windows for User and System config levels") + } + + // Install a driver + m := InstallCmd{Driver: "test-driver-1", Level: suite.configLevel}. + GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) + _ = suite.runCmd(m) + suite.driverIsInstalled("test-driver-1", true) + + // Verify symlink is in place in the parent dir and is actually a symlink + manifestPath := filepath.Join(suite.Dir(), "..", "test-driver-1.toml") + suite.FileExists(manifestPath) + info, err := os.Lstat(manifestPath) + suite.NoError(err) + suite.Equal(os.ModeSymlink, info.Mode()&os.ModeSymlink, "Expected test-driver-1.toml to be a symlink") + + // Uninstall the driver + m = UninstallCmd{Driver: "test-driver-1", Level: suite.configLevel}.GetModel() + _ = suite.runCmd(m) + + // Verify symlink is gone + suite.NoFileExists(manifestPath) } diff --git a/config/config.go b/config/config.go index 992143c4..0f5d1b03 100644 --- a/config/config.go +++ b/config/config.go @@ -163,7 +163,7 @@ func loadDir(dir string) (map[string]DriverInfo, error) { } func loadConfig(lvl ConfigLevel) Config { - cfg := Config{Level: lvl, Location: lvl.configLocation()} + cfg := Config{Level: lvl, Location: lvl.ConfigLocation()} if cfg.Location == "" { return cfg } @@ -382,6 +382,8 @@ func UninstallDriverShared(info DriverInfo) error { } } + // Special handling to clean up manifest-only drivers + // // Manifest only drivers can come with extra files such as a LICENSE and we // create a folder next to the driver manifest to store them, same as we'd // store the actual driver shared library. Above, we find the path of this @@ -389,9 +391,20 @@ func UninstallDriverShared(info DriverInfo) error { // Driver.shared is not a valid path (it's just a name), so this trick doesn't // work. We do want to clean this folder up so here we guess what it is and // try to remove it e.g., "somedriver_macos_arm64_v1.2.3." + // + // For the User and System config levels, info.FilePath is set to the + // appropriate registry key instead of the filesystem so so we handle that + // here first. + filesystemLocation := info.FilePath + if strings.Contains(info.FilePath, "HKCU\\") { + filesystemLocation = ConfigUser.ConfigLocation() + } else if strings.Contains(info.FilePath, "HKLM\\") { + filesystemLocation = ConfigSystem.ConfigLocation() + } + extra_folder := fmt.Sprintf("%s_%s_v%s", info.ID, platformTuple, info.Version) extra_folder = filepath.Clean(extra_folder) - extra_path := filepath.Join(info.FilePath, extra_folder) + extra_path := filepath.Join(filesystemLocation, extra_folder) finfo, err := os.Stat(extra_path) if err == nil && finfo.IsDir() && extra_path != "." { _ = os.RemoveAll(extra_path) diff --git a/config/config_test.go b/config/config_test.go index 77c2cd2b..5cefb115 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -23,21 +23,11 @@ import ( ) func TestConfigEnvVarHierarchy(t *testing.T) { - // Record old values and reset when done - originalAdbcConfigPath := os.Getenv("ADBC_DRIVER_PATH") - originalVirtualEnv := os.Getenv("VIRTUAL_ENV") - originalCondaPrefix := os.Getenv("CONDA_PREFIX") - defer func() { - os.Setenv("ADBC_DRIVER_PATH", originalAdbcConfigPath) - os.Setenv("VIRTUAL_ENV", originalVirtualEnv) - os.Setenv("CONDA_PREFIX", originalCondaPrefix) - }() - // Test the order is honored (ADBC_DRIVER_PATH before VIRTUAL_ENV before // CONDA_PREFIX) and unset each one (in order) to verify. - os.Setenv("ADBC_DRIVER_PATH", "some_adbc_driver_path") - os.Setenv("VIRTUAL_ENV", "some_virtual_env") - os.Setenv("CONDA_PREFIX", "some_conda_prefix") + t.Setenv("ADBC_DRIVER_PATH", "some_adbc_driver_path") + t.Setenv("VIRTUAL_ENV", "some_virtual_env") + t.Setenv("CONDA_PREFIX", "some_conda_prefix") cfg := loadConfig(ConfigEnv) assert.Equal(t, "some_adbc_driver_path"+string(filepath.ListSeparator)+ diff --git a/config/dirs_unixlike.go b/config/dirs_unixlike.go index 23e23b6f..41882af8 100644 --- a/config/dirs_unixlike.go +++ b/config/dirs_unixlike.go @@ -63,7 +63,7 @@ func init() { } } -func (c ConfigLevel) configLocation() string { +func (c ConfigLevel) ConfigLocation() string { switch c { case ConfigSystem: return systemConfigDir @@ -118,6 +118,11 @@ func UninstallDriver(_ Config, info DriverInfo) error { return fmt.Errorf("error removing manifest %s: %w", manifest, err) } + // Remove the symlink created during installation (one level up from the + // manifest) + // TODO: Remove this when the driver managers are fixed (>=1.8.1). + removeManifestSymlink(info.FilePath, info.ID) + if err := UninstallDriverShared(info); err != nil { return fmt.Errorf("failed to delete driver shared object: %w", err) } diff --git a/config/dirs_windows.go b/config/dirs_windows.go index edbdcadc..45bee850 100644 --- a/config/dirs_windows.go +++ b/config/dirs_windows.go @@ -58,7 +58,7 @@ func (c ConfigLevel) rootKeyString() string { } } -func (c ConfigLevel) configLocation() string { +func (c ConfigLevel) ConfigLocation() string { var prefix string switch c { case ConfigSystem: @@ -162,7 +162,7 @@ func driverInfoFromKey(k registry.Key, driverName string, lvl ConfigLevel) (di D } func loadRegistryConfig(lvl ConfigLevel) Config { - ret := Config{Level: lvl, Location: lvl.configLocation()} + ret := Config{Level: lvl, Location: lvl.ConfigLocation()} k, err := registry.OpenKey(lvl.key(), regKeyADBC, registry.READ) if err != nil { return ret @@ -345,6 +345,9 @@ func UninstallDriver(cfg Config, info DriverInfo) error { if err := os.Remove(manifest); err != nil { return fmt.Errorf("error removing manifest %s: %w", manifest, err) } + + // TODO: Remove this when the driver managers are fixed (>=1.8.1). + removeManifestSymlink(info.FilePath, info.ID) } return nil diff --git a/config/driver.go b/config/driver.go index 03787920..6df4f237 100644 --- a/config/driver.go +++ b/config/driver.go @@ -164,6 +164,28 @@ func loadDriverFromManifest(prefix, driverName string) (DriverInfo, error) { return m.DriverInfo, nil } +// Create a symlink to manifestPath in the parent dir +func createManifestSymlink(location, driverID, manifestPath string) { + parentDir := filepath.Dir(filepath.Clean(location)) + safeDriverID := filepath.Base(driverID) + symlink := filepath.Join(parentDir, safeDriverID+".toml") + + if filepath.Dir(symlink) == parentDir { + os.Symlink(manifestPath, symlink) + } +} + +// Remove the symlink to manifestPath in the parent dir +func removeManifestSymlink(filePath, driverID string) { + parentDir := filepath.Dir(filepath.Clean(filePath)) + safeDriverID := filepath.Base(driverID) + symlink := filepath.Join(parentDir, safeDriverID+".toml") + + if filepath.Dir(symlink) == parentDir { + os.Remove(symlink) + } +} + func createDriverManifest(location string, driver DriverInfo) error { if _, err := os.Stat(location); errors.Is(err, fs.ErrNotExist) { if err := os.MkdirAll(location, 0755); err != nil { @@ -186,8 +208,7 @@ func createDriverManifest(location string, driver DriverInfo) error { // installing. // // TODO: Remove this when the driver managers are fixed (>=1.8.1). - symlink := filepath.Join(location, "..", driver.ID+".toml") - os.Symlink(manifest_path, symlink) + createManifestSymlink(location, driver.ID, manifest_path) toEncode := tomlDriverInfo{ ManifestVersion: currentManifestVersion, diff --git a/config/unixlike_test.go b/config/unixlike_test.go index 93015509..f7dceddd 100644 --- a/config/unixlike_test.go +++ b/config/unixlike_test.go @@ -28,9 +28,9 @@ func TestSystemConfigDir(t *testing.T) { os := runtime.GOOS if os == "darwin" { - assert.Equal(t, "/Library/Application Support/ADBC/Drivers", ConfigSystem.configLocation()) + assert.Equal(t, "/Library/Application Support/ADBC/Drivers", ConfigSystem.ConfigLocation()) } else { - assert.Equal(t, "/etc/adbc/drivers", ConfigSystem.configLocation()) + assert.Equal(t, "/etc/adbc/drivers", ConfigSystem.ConfigLocation()) } } @@ -41,6 +41,6 @@ func TestSystemConfigWithEnv(t *testing.T) { prefix = os.Getenv("CONDA_PREFIX") } if prefix != "" { - assert.Equal(t, prefix+"/etc/adbc/drivers", ConfigSystem.configLocation()) + assert.Equal(t, prefix+"/etc/adbc/drivers", ConfigSystem.ConfigLocation()) } }