Skip to content

Commit c9453df

Browse files
committed
fix: dry-run must not write packages cache; adapt vm e2e for pre-release CI
- config: RefreshPackagesFromRemoteDryRun() skips writePackagesCache so --dry-run has zero side effects on ~/.openboot/ - root.go: call the dry-run variant when installCfg.DryRun is set - TestVM_Journey_FirstTimeUser: use dev binary (vmCopyDevBinary) instead of brew tap install — brew formula requires a published release - TestVM_Interactive_InstallScript: skip when openboot formula is not available in Homebrew (pre-release CI environment)
1 parent 0585c5e commit c9453df

5 files changed

Lines changed: 52 additions & 23 deletions

File tree

internal/cli/root.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,11 @@ shell configuration, and macOS preferences.`,
5555
// network overhead.
5656
if cmd.Name() == "install" {
5757
updater.AutoUpgrade(version)
58-
config.RefreshPackagesFromRemote()
58+
if installCfg.DryRun {
59+
config.RefreshPackagesFromRemoteDryRun()
60+
} else {
61+
config.RefreshPackagesFromRemote()
62+
}
5963
}
6064

6165
return nil

internal/config/packages_remote.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,18 @@ type packagesCacheEntry struct {
3838
// into the global Categories slice. Safe to call multiple times; it is a no-op
3939
// if the cache is fresh. Falls back to the embedded packages.yaml silently.
4040
func RefreshPackagesFromRemote() {
41-
pkgs, err := loadRemotePackages()
41+
refreshPackages(false)
42+
}
43+
44+
// RefreshPackagesFromRemoteDryRun is identical to RefreshPackagesFromRemote
45+
// but suppresses writing the on-disk cache. Use during --dry-run so the
46+
// command has zero side effects on ~/.openboot/.
47+
func RefreshPackagesFromRemoteDryRun() {
48+
refreshPackages(true)
49+
}
50+
51+
func refreshPackages(dryRun bool) {
52+
pkgs, err := loadRemotePackages(dryRun)
4253
if err != nil || len(pkgs) == 0 {
4354
return // keep embedded fallback
4455
}
@@ -47,7 +58,7 @@ func RefreshPackagesFromRemote() {
4758
mergeRemotePackages(pkgs)
4859
}
4960

50-
func loadRemotePackages() ([]remotePackage, error) {
61+
func loadRemotePackages(dryRun bool) ([]remotePackage, error) {
5162
// Try disk cache first.
5263
if pkgs, err := readPackagesCache(); err == nil {
5364
return pkgs, nil
@@ -59,8 +70,10 @@ func loadRemotePackages() ([]remotePackage, error) {
5970
return nil, err
6071
}
6172

62-
// Write cache (best-effort).
63-
_ = writePackagesCache(pkgs)
73+
// Write cache (best-effort) — skip during dry-run to avoid disk side effects.
74+
if !dryRun {
75+
_ = writePackagesCache(pkgs)
76+
}
6477
return pkgs, nil
6578
}
6679

internal/config/packages_remote_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ func TestLoadRemotePackages_UsesCacheThenFallsToNetwork(t *testing.T) {
291291

292292
// No cache, no server → error.
293293
t.Setenv("OPENBOOT_API_URL", "http://localhost:1")
294-
_, err := loadRemotePackages()
294+
_, err := loadRemotePackages(false)
295295
assert.Error(t, err)
296296

297297
// Write fresh cache.
@@ -303,7 +303,7 @@ func TestLoadRemotePackages_UsesCacheThenFallsToNetwork(t *testing.T) {
303303
os.WriteFile(filepath.Join(dir, packagesCacheFile), data, 0600)
304304

305305
// Cache hit → no network call needed.
306-
pkgs, err := loadRemotePackages()
306+
pkgs, err := loadRemotePackages(false)
307307
require.NoError(t, err)
308308
assert.Equal(t, "cached-pkg", pkgs[0].Name)
309309
}

test/e2e/vm_interactive_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,18 @@ func TestVM_Interactive_InstallScript(t *testing.T) {
2020
}
2121

2222
vm := testutil.NewMacHost(t)
23+
24+
// This test exercises install.sh's reinstall prompt, which requires openboot
25+
// to be registered with Homebrew (`brew list openboot`). Skip if the formula
26+
// isn't available — that means we're in a pre-release CI environment where
27+
// the tap has no published formula yet.
28+
tapScript := fmt.Sprintf(
29+
"export PATH=%q && brew tap openbootdotdev/openboot 2>/dev/null; brew info openboot &>/dev/null",
30+
brewPath,
31+
)
32+
if _, err := vm.Run(tapScript); err != nil {
33+
t.Skip("openboot Homebrew formula not available — skipping brew-dependent interactive test")
34+
}
2335
vmInstallViaBrew(t, vm) // Install first (no TTY required)
2436

2537
// expect is required for interactive tests.

test/e2e/vm_user_journey_test.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,38 +40,38 @@ func TestVM_Journey_FirstTimeUser(t *testing.T) {
4040
}
4141

4242
vm := testutil.NewMacHost(t)
43-
44-
// Clean up any openboot left over from a prior test run.
45-
vm.Run(fmt.Sprintf("export PATH=%q && brew uninstall openboot 2>/dev/null || true", brewPath)) //nolint:errcheck // best-effort cleanup
43+
vmInstallHomebrew(t, vm)
44+
bin := vmCopyDevBinary(t, vm)
4645

4746
// Step 1: openboot shouldn't leak in from a prior step.
4847
t.Run("bare_system_has_no_openboot", func(t *testing.T) {
4948
out, _ := vm.Run("which openboot 2>/dev/null || echo not-found")
5049
assert.Contains(t, out, "not-found", "openboot should not exist before install")
5150
})
5251

53-
// Step 2: Install via brew tap (no TTY required; mirrors the curl|bash tap path).
54-
t.Run("installs_via_brew_tap", func(t *testing.T) {
55-
version := vmInstallViaBrew(t, vm)
56-
assert.Contains(t, version, "OpenBoot v", "should report version after install")
52+
// Step 2: Dev binary runs and reports a version.
53+
t.Run("binary_is_available", func(t *testing.T) {
54+
out, err := vm.Run(bin + " version")
55+
require.NoError(t, err)
56+
assert.Contains(t, out, "OpenBoot", "binary should report version")
5757
})
5858

5959
// Step 3: Run openboot with minimal preset
6060
t.Run("minimal_preset_installs_usable_tools", func(t *testing.T) {
61-
output, err := vmRunOpenbootWithGit(t, vm, "install --preset minimal --silent --packages-only")
61+
output, err := vmRunDevBinaryWithGit(t, vm, bin, "install --preset minimal --silent --packages-only")
6262
t.Logf("install output:\n%s", output)
6363
require.NoError(t, err, "minimal preset should succeed")
6464

6565
// User expectation: every tool should be USABLE, not just "in PATH"
6666
toolChecks := map[string]string{
67-
"jq": `echo '{"a":1}' | jq '.a'`, // Can it parse JSON?
68-
"rg": `echo 'hello world' | rg 'hello'`, // Can it search?
69-
"fd": `fd --version`, // Does it run?
70-
"bat": `echo 'test' | bat --plain`, // Can it display?
71-
"fzf": `echo 'a\nb\nc' | fzf --filter 'b'`, // Can it filter?
72-
"htop": `htop --version`, // Does it run?
73-
"tree": `tree --version`, // Does it run?
74-
"gh": `gh --version`, // Does it run?
67+
"jq": `echo '{"a":1}' | jq '.a'`,
68+
"rg": `echo 'hello world' | rg 'hello'`,
69+
"fd": `fd --version`,
70+
"bat": `echo 'test' | bat --plain`,
71+
"fzf": `echo 'a\nb\nc' | fzf --filter 'b'`,
72+
"htop": `htop --version`,
73+
"tree": `tree --version`,
74+
"gh": `gh --version`,
7575
}
7676

7777
for name, cmd := range toolChecks {

0 commit comments

Comments
 (0)