test(install): cover downgrade by reinstalling an older version#390
Merged
Conversation
Mirror TestReinstallUpdateVersion to verify that installing a driver, then reinstalling it pinned to an older version, removes the newer install and leaves only the older version's files on disk.
2ead0fb to
f6bcf35
Compare
ianmcook
reviewed
May 29, 2026
Comment on lines
+113
to
+114
| suite.Equal([]string{"test-driver-1/test-driver-1-not-valid.so", | ||
| "test-driver-1/test-driver-1-not-valid.so.sig", "test-driver-1.toml"}, suite.getFilesInTempDir()) |
Member
There was a problem hiding this comment.
Should the test also look inside the manifest to confirm that the driver version number is <= 1.0.0?
Member
There was a problem hiding this comment.
Currently the only way this actually validates that the driver is replaced is by validating the dbc output, but hypothetically the output could be wrong.
Member
There was a problem hiding this comment.
I added 6cd9512 to address this. Feel free to rework it @zeroshade.
Member
Author
There was a problem hiding this comment.
Updated and got the tests passing.
driverIsInstalledWithVersion looks up the manifest at suite.configLevel, but the InstallCmd was running without an explicit Level so it always wrote to suite.tempdir via ADBC_DRIVER_PATH. At User/System levels these paths diverge and the manifest lookup fails. Match the canonical pattern used elsewhere in install_test.go: - pass Level: suite.configLevel to InstallCmd - assert against suite.Dir() instead of suite.tempdir - add a getFilesInDir(dir) helper so the file-list assertion can walk the level-aware install directory
On Windows, manifests at User/System config levels are stored in the Windows registry rather than as .toml files on disk, so the file-list assertion does not apply. Mirrors TestInstallCreatesSymlinks.
ianmcook
reviewed
May 29, 2026
|
|
||
| func (suite *SubcommandTestSuite) TestReinstallDowngradeVersion() { | ||
| if runtime.GOOS == "windows" && (suite.configLevel == config.ConfigUser || suite.configLevel == config.ConfigSystem) { | ||
| suite.T().Skip("Driver manifest is stored in the registry on Windows for User and System config levels, so the file-list assertion does not apply") |
Member
There was a problem hiding this comment.
Obvious next question: can we check the registry on Windows instead of just skipping the test? :)
amoeba
approved these changes
May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
TestReinstallDowngradeVersiontocmd/dbc/install_test.go, mirroring the existingTestReinstallUpdateVersionbut exercising the opposite direction: install latest, then reinstall pinned to an older version.Before this change, no test verified that downgrading a driver:
The conflict-and-replace path (
Removed conflicting driver: …) was only covered for upgrades (1.0.0 → 1.1.0). This adds symmetric coverage for downgrades (1.1.0 → 1.0.0), exercising the same code path with the version comparison reversed.Test plan
go vet ./cmd/dbc/...is clean.Notes
The expected install directory for v1.0.0 is
test-driver-1/(nottest-driver-1.0/) because the directory name is derived from the tarball filename (test-driver-1.tar.gzfor v1.0.0,test-driver-1.1.tar.gzfor v1.1.0) perconfig.InstallDriver. No fixture changes are needed.