Skip to content

test(install): cover downgrade by reinstalling an older version#390

Merged
zeroshade merged 5 commits into
mainfrom
test/install-downgrade-version
May 31, 2026
Merged

test(install): cover downgrade by reinstalling an older version#390
zeroshade merged 5 commits into
mainfrom
test/install-downgrade-version

Conversation

@zeroshade
Copy link
Copy Markdown
Member

Summary

Adds TestReinstallDowngradeVersion to cmd/dbc/install_test.go, mirroring the existing TestReinstallUpdateVersion but exercising the opposite direction: install latest, then reinstall pinned to an older version.

Before this change, no test verified that downgrading a driver:

  1. Removes the previously-installed newer version (including its install directory)
  2. Lays down the older version's files on disk
  3. Leaves the driver discoverable via the manifest

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 test -run 'TestSubcommandsEnv/TestReinstall(Update|Downgrade)Version' -v ./cmd/dbc/...
=== RUN   TestSubcommandsEnv/TestReinstallDowngradeVersion
=== RUN   TestSubcommandsEnv/TestReinstallUpdateVersion
--- PASS: TestSubcommandsEnv (0.01s)
    --- PASS: TestSubcommandsEnv/TestReinstallDowngradeVersion (0.01s)
    --- PASS: TestSubcommandsEnv/TestReinstallUpdateVersion (0.00s)
PASS

go vet ./cmd/dbc/... is clean.

Notes

The expected install directory for v1.0.0 is test-driver-1/ (not test-driver-1.0/) because the directory name is derived from the tarball filename (test-driver-1.tar.gz for v1.0.0, test-driver-1.1.tar.gz for v1.1.0) per config.InstallDriver. No fixture changes are needed.

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.
@zeroshade zeroshade force-pushed the test/install-downgrade-version branch from 2ead0fb to f6bcf35 Compare May 29, 2026 15:21
@zeroshade zeroshade requested review from amoeba and ianmcook May 29, 2026 15:23
Comment thread cmd/dbc/install_test.go Outdated
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())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the test also look inside the manifest to confirm that the driver version number is <= 1.0.0?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added 6cd9512 to address this. Feel free to rework it @zeroshade.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated and got the tests passing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to take another look @amoeba

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.
Comment thread cmd/dbc/install_test.go Outdated

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")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obvious next question: can we check the registry on Windows instead of just skipping the test? :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 77e7a34.

@amoeba amoeba deployed to snapshot May 29, 2026 21:08 — with GitHub Actions Active
@zeroshade zeroshade merged commit b383f44 into main May 31, 2026
12 checks passed
@zeroshade zeroshade deleted the test/install-downgrade-version branch May 31, 2026 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants