fix(daemon): read multi-section config from one snapshot and test ApplyConfig publication
- commit
- 0f5321a0778e9670834fc16295e7ea406db34e73
- author
- brice · · 2026-10-08 08:57 UTC
- committer
- brice · · 2026-10-08 09:31 UTC
- parent
- e16502c
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +40 -86
M internal/usecase/daemon/config_test.go +34 -82
| @@ -1,7 +1,6 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | - | "sync" | |
| 5 | 4 | "testing" | |
| 6 | 5 | ||
| 7 | 6 | "github.com/stretchr/testify/require" | |
| @@ -172,95 +171,48 @@ func updateRuntimeConfig(d *Daemon, mutate func(*runtimeConfig)) { | |||
| 172 | 171 | d.config.Store(&next) | |
| 173 | 172 | } | |
| 174 | 173 | ||
| 175 | - | func storeThemeForTest(d *Daemon, cfg domain.Config) { | |
| 176 | - | updateRuntimeConfig(d, func(rc *runtimeConfig) { | |
| 177 | - | rc.theme = themeConfigSnapshot{mode: cfg.Theme, paletteOff: !cfg.ThemePalette, accent: cfg.ThemeAccent} | |
| 178 | - | }) | |
| 179 | - | } | |
| 180 | - | ||
| 181 | - | func TestThemeConfigSnapshotIsAtomic(t *testing.T) { | |
| 182 | - | d := newTestDaemon(t, nil, stubClock{}) | |
| 183 | - | ||
| 184 | - | // A nil atomic pointer is the zero/default snapshot: auto theme with | |
| 185 | - | // terminal palette inheritance enabled. | |
| 186 | - | require.Equal(t, themeConfigSnapshot{}, d.currentThemeConfig()) | |
| 174 | + | // TestApplyConfigPublishesOneSnapshot checks ApplyConfig publishes every | |
| 175 | + | // section of one config in a single snapshot, and that the snapshot in force | |
| 176 | + | // before any reload equals the one built from vev's defaults. | |
| 177 | + | func TestApplyConfigPublishesOneSnapshot(t *testing.T) { | |
| 178 | + | custom := domain.Defaults() | |
| 179 | + | custom.Theme = domain.ThemeDark | |
| 180 | + | custom.ThemePalette = false | |
| 181 | + | custom.Codes = map[string]string{"new-tab": "nt"} | |
| 182 | + | custom.Snapshot = domain.SnapshotConfig{RestoreProcesses: []string{"htop"}, RestoreProcessesSet: true} | |
| 183 | + | custom.Copy.WordSeparators = "-" | |
| 184 | + | custom.Palette = domain.PaletteConfig{Anchor: custom.Palette.Anchor, AnchorSet: true} | |
| 185 | + | custom.Floating.Width++ | |
| 186 | + | custom.Nav.OverflowTabs = !custom.Nav.OverflowTabs | |
| 187 | + | custom.Tabs.TerminalTitle = !custom.Tabs.TerminalTitle | |
| 188 | + | custom.Ephemeral.CloseOnExit = !custom.Ephemeral.CloseOnExit | |
| 189 | + | custom.Scrollback = domain.ScrollbackConfig{Megabytes: 3, Lines: 300} | |
| 187 | 190 | ||
| 188 | 191 | for _, tt := range []struct { | |
| 189 | 192 | name string | |
| 190 | - | mode domain.ThemeMode | |
| 191 | - | gate bool | |
| 193 | + | cfg domain.Config | |
| 194 | + | want *runtimeConfig | |
| 192 | 195 | }{ | |
| 193 | - | {name: "auto palette enabled", mode: domain.ThemeAuto, gate: true}, | |
| 194 | - | {name: "auto palette disabled", mode: domain.ThemeAuto, gate: false}, | |
| 195 | - | {name: "dark palette enabled", mode: domain.ThemeDark, gate: true}, | |
| 196 | - | {name: "dark palette disabled", mode: domain.ThemeDark, gate: false}, | |
| 197 | - | {name: "light palette enabled", mode: domain.ThemeLight, gate: true}, | |
| 198 | - | {name: "light palette disabled", mode: domain.ThemeLight, gate: false}, | |
| 196 | + | {name: "defaults match the pre-reload snapshot", cfg: domain.Defaults(), want: defaultRuntimeConfig()}, | |
| 197 | + | {name: "every section comes from one config", cfg: custom, want: &runtimeConfig{ | |
| 198 | + | codeOverrides: map[string]string{"new-tab": "NT"}, | |
| 199 | + | restoreProcessAllowlist: map[string]struct{}{"htop": {}}, | |
| 200 | + | floating: custom.Floating, | |
| 201 | + | copy: custom.Copy, | |
| 202 | + | palette: custom.Palette, | |
| 203 | + | nav: custom.Nav, | |
| 204 | + | tabs: custom.Tabs, | |
| 205 | + | ephemeral: custom.Ephemeral, | |
| 206 | + | scrollback: custom.Scrollback, | |
| 207 | + | theme: themeConfigSnapshot{mode: domain.ThemeDark, paletteOff: true, accent: custom.ThemeAccent}, | |
| 208 | + | }}, | |
| 199 | 209 | } { | |
| 200 | 210 | t.Run(tt.name, func(t *testing.T) { | |
| 201 | - | storeThemeForTest(d, domain.Config{Theme: tt.mode, ThemePalette: tt.gate}) | |
| 202 | - | require.Equal(t, themeConfigSnapshot{mode: tt.mode, paletteOff: !tt.gate}, d.currentThemeConfig()) | |
| 211 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 212 | + | d.ApplyConfig(tt.cfg) | |
| 213 | + | require.Equal(t, tt.want, d.config.Load()) | |
| 203 | 214 | }) | |
| 204 | 215 | } | |
| 205 | - | ||
| 206 | - | // Publish complementary configurations concurrently. Every load must be a | |
| 207 | - | // complete published snapshot, never mode from one update and gate from | |
| 208 | - | // another. Coordination establishes concurrent readers/writers without | |
| 209 | - | // timing sleeps. | |
| 210 | - | first := domain.Config{Theme: domain.ThemeAuto, ThemePalette: false} | |
| 211 | - | second := domain.Config{Theme: domain.ThemeDark, ThemePalette: true} | |
| 212 | - | allowed := map[themeConfigSnapshot]struct{}{ | |
| 213 | - | {mode: domain.ThemeAuto, paletteOff: true}: {}, | |
| 214 | - | {mode: domain.ThemeDark, paletteOff: false}: {}, | |
| 215 | - | } | |
| 216 | - | firstPublished := make(chan struct{}) | |
| 217 | - | firstObserved := make(chan struct{}) | |
| 218 | - | secondPublished := make(chan struct{}) | |
| 219 | - | secondObserved := make(chan struct{}) | |
| 220 | - | observed := make(chan themeConfigSnapshot, 2) | |
| 221 | - | invalid := make(chan themeConfigSnapshot, 1) | |
| 222 | - | var wg sync.WaitGroup | |
| 223 | - | wg.Add(2) | |
| 224 | - | go func() { | |
| 225 | - | defer wg.Done() | |
| 226 | - | storeThemeForTest(d, first) | |
| 227 | - | close(firstPublished) | |
| 228 | - | <-firstObserved | |
| 229 | - | storeThemeForTest(d, second) | |
| 230 | - | close(secondPublished) | |
| 231 | - | <-secondObserved | |
| 232 | - | for range 10_000 { | |
| 233 | - | storeThemeForTest(d, second) | |
| 234 | - | storeThemeForTest(d, first) | |
| 235 | - | } | |
| 236 | - | }() | |
| 237 | - | go func() { | |
| 238 | - | defer wg.Done() | |
| 239 | - | <-firstPublished | |
| 240 | - | observed <- d.currentThemeConfig() | |
| 241 | - | close(firstObserved) | |
| 242 | - | <-secondPublished | |
| 243 | - | observed <- d.currentThemeConfig() | |
| 244 | - | close(secondObserved) | |
| 245 | - | for range 20_000 { | |
| 246 | - | snapshot := d.currentThemeConfig() | |
| 247 | - | if _, ok := allowed[snapshot]; !ok { | |
| 248 | - | select { | |
| 249 | - | case invalid <- snapshot: | |
| 250 | - | default: | |
| 251 | - | } | |
| 252 | - | return | |
| 253 | - | } | |
| 254 | - | } | |
| 255 | - | }() | |
| 256 | - | wg.Wait() | |
| 257 | - | require.Equal(t, themeConfigSnapshot{mode: domain.ThemeAuto, paletteOff: true}, <-observed) | |
| 258 | - | require.Equal(t, themeConfigSnapshot{mode: domain.ThemeDark, paletteOff: false}, <-observed) | |
| 259 | - | select { | |
| 260 | - | case snapshot := <-invalid: | |
| 261 | - | t.Fatalf("observed mixed theme configuration: %#v", snapshot) | |
| 262 | - | default: | |
| 263 | - | } | |
| 264 | 216 | } | |
| 265 | 217 | ||
| 266 | 218 | func TestApplyConfigPublishesCopyConfig(t *testing.T) { | |
| @@ -1,7 +1,6 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | - | "sync" | |||
| 5 | "testing" | 4 | "testing" | ||
| 6 | 5 | ||||
| 7 | "github.com/stretchr/testify/require" | 6 | "github.com/stretchr/testify/require" | ||
| @@ -172,95 +171,48 @@ func updateRuntimeConfig(d *Daemon, mutate func(*runtimeConfig)) { | |||||
| 172 | d.config.Store(&next) | 171 | d.config.Store(&next) | ||
| 173 | } | 172 | } | ||
| 174 | 173 | ||||
| 175 | - | func storeThemeForTest(d *Daemon, cfg domain.Config) { | 174 | + | // TestApplyConfigPublishesOneSnapshot checks ApplyConfig publishes every |
| 176 | - | updateRuntimeConfig(d, func(rc *runtimeConfig) { | 175 | + | // section of one config in a single snapshot, and that the snapshot in force |
| 177 | - | rc.theme = themeConfigSnapshot{mode: cfg.Theme, paletteOff: !cfg.ThemePalette, accent: cfg.ThemeAccent} | 176 | + | // before any reload equals the one built from vev's defaults. |
| 178 | - | }) | 177 | + | func TestApplyConfigPublishesOneSnapshot(t *testing.T) { |
| 179 | - | } | 178 | + | custom := domain.Defaults() |
| 180 | - | 179 | + | custom.Theme = domain.ThemeDark | |
| 181 | - | func TestThemeConfigSnapshotIsAtomic(t *testing.T) { | 180 | + | custom.ThemePalette = false |
| 182 | - | d := newTestDaemon(t, nil, stubClock{}) | 181 | + | custom.Codes = map[string]string{"new-tab": "nt"} |
| 183 | - | 182 | + | custom.Snapshot = domain.SnapshotConfig{RestoreProcesses: []string{"htop"}, RestoreProcessesSet: true} | |
| 184 | - | // A nil atomic pointer is the zero/default snapshot: auto theme with | 183 | + | custom.Copy.WordSeparators = "-" |
| 185 | - | // terminal palette inheritance enabled. | 184 | + | custom.Palette = domain.PaletteConfig{Anchor: custom.Palette.Anchor, AnchorSet: true} |
| 186 | - | require.Equal(t, themeConfigSnapshot{}, d.currentThemeConfig()) | 185 | + | custom.Floating.Width++ |
| 186 | + | custom.Nav.OverflowTabs = !custom.Nav.OverflowTabs | |||
| 187 | + | custom.Tabs.TerminalTitle = !custom.Tabs.TerminalTitle | |||
| 188 | + | custom.Ephemeral.CloseOnExit = !custom.Ephemeral.CloseOnExit | |||
| 189 | + | custom.Scrollback = domain.ScrollbackConfig{Megabytes: 3, Lines: 300} | |||
| 187 | 190 | ||||
| 188 | for _, tt := range []struct { | 191 | for _, tt := range []struct { | ||
| 189 | name string | 192 | name string | ||
| 190 | - | mode domain.ThemeMode | 193 | + | cfg domain.Config |
| 191 | - | gate bool | 194 | + | want *runtimeConfig |
| 192 | }{ | 195 | }{ | ||
| 193 | - | {name: "auto palette enabled", mode: domain.ThemeAuto, gate: true}, | 196 | + | {name: "defaults match the pre-reload snapshot", cfg: domain.Defaults(), want: defaultRuntimeConfig()}, |
| 194 | - | {name: "auto palette disabled", mode: domain.ThemeAuto, gate: false}, | 197 | + | {name: "every section comes from one config", cfg: custom, want: &runtimeConfig{ |
| 195 | - | {name: "dark palette enabled", mode: domain.ThemeDark, gate: true}, | 198 | + | codeOverrides: map[string]string{"new-tab": "NT"}, |
| 196 | - | {name: "dark palette disabled", mode: domain.ThemeDark, gate: false}, | 199 | + | restoreProcessAllowlist: map[string]struct{}{"htop": {}}, |
| 197 | - | {name: "light palette enabled", mode: domain.ThemeLight, gate: true}, | 200 | + | floating: custom.Floating, |
| 198 | - | {name: "light palette disabled", mode: domain.ThemeLight, gate: false}, | 201 | + | copy: custom.Copy, |
| 202 | + | palette: custom.Palette, | |||
| 203 | + | nav: custom.Nav, | |||
| 204 | + | tabs: custom.Tabs, | |||
| 205 | + | ephemeral: custom.Ephemeral, | |||
| 206 | + | scrollback: custom.Scrollback, | |||
| 207 | + | theme: themeConfigSnapshot{mode: domain.ThemeDark, paletteOff: true, accent: custom.ThemeAccent}, | |||
| 208 | + | }}, | |||
| 199 | } { | 209 | } { | ||
| 200 | t.Run(tt.name, func(t *testing.T) { | 210 | t.Run(tt.name, func(t *testing.T) { | ||
| 201 | - | storeThemeForTest(d, domain.Config{Theme: tt.mode, ThemePalette: tt.gate}) | 211 | + | d := newTestDaemon(t, nil, stubClock{}) |
| 202 | - | require.Equal(t, themeConfigSnapshot{mode: tt.mode, paletteOff: !tt.gate}, d.currentThemeConfig()) | 212 | + | d.ApplyConfig(tt.cfg) |
| 213 | + | require.Equal(t, tt.want, d.config.Load()) | |||
| 203 | }) | 214 | }) | ||
| 204 | } | 215 | } | ||
| 205 | - | ||||
| 206 | - | // Publish complementary configurations concurrently. Every load must be a | |||
| 207 | - | // complete published snapshot, never mode from one update and gate from | |||
| 208 | - | // another. Coordination establishes concurrent readers/writers without | |||
| 209 | - | // timing sleeps. | |||
| 210 | - | first := domain.Config{Theme: domain.ThemeAuto, ThemePalette: false} | |||
| 211 | - | second := domain.Config{Theme: domain.ThemeDark, ThemePalette: true} | |||
| 212 | - | allowed := map[themeConfigSnapshot]struct{}{ | |||
| 213 | - | {mode: domain.ThemeAuto, paletteOff: true}: {}, | |||
| 214 | - | {mode: domain.ThemeDark, paletteOff: false}: {}, | |||
| 215 | - | } | |||
| 216 | - | firstPublished := make(chan struct{}) | |||
| 217 | - | firstObserved := make(chan struct{}) | |||
| 218 | - | secondPublished := make(chan struct{}) | |||
| 219 | - | secondObserved := make(chan struct{}) | |||
| 220 | - | observed := make(chan themeConfigSnapshot, 2) | |||
| 221 | - | invalid := make(chan themeConfigSnapshot, 1) | |||
| 222 | - | var wg sync.WaitGroup | |||
| 223 | - | wg.Add(2) | |||
| 224 | - | go func() { | |||
| 225 | - | defer wg.Done() | |||
| 226 | - | storeThemeForTest(d, first) | |||
| 227 | - | close(firstPublished) | |||
| 228 | - | <-firstObserved | |||
| 229 | - | storeThemeForTest(d, second) | |||
| 230 | - | close(secondPublished) | |||
| 231 | - | <-secondObserved | |||
| 232 | - | for range 10_000 { | |||
| 233 | - | storeThemeForTest(d, second) | |||
| 234 | - | storeThemeForTest(d, first) | |||
| 235 | - | } | |||
| 236 | - | }() | |||
| 237 | - | go func() { | |||
| 238 | - | defer wg.Done() | |||
| 239 | - | <-firstPublished | |||
| 240 | - | observed <- d.currentThemeConfig() | |||
| 241 | - | close(firstObserved) | |||
| 242 | - | <-secondPublished | |||
| 243 | - | observed <- d.currentThemeConfig() | |||
| 244 | - | close(secondObserved) | |||
| 245 | - | for range 20_000 { | |||
| 246 | - | snapshot := d.currentThemeConfig() | |||
| 247 | - | if _, ok := allowed[snapshot]; !ok { | |||
| 248 | - | select { | |||
| 249 | - | case invalid <- snapshot: | |||
| 250 | - | default: | |||
| 251 | - | } | |||
| 252 | - | return | |||
| 253 | - | } | |||
| 254 | - | } | |||
| 255 | - | }() | |||
| 256 | - | wg.Wait() | |||
| 257 | - | require.Equal(t, themeConfigSnapshot{mode: domain.ThemeAuto, paletteOff: true}, <-observed) | |||
| 258 | - | require.Equal(t, themeConfigSnapshot{mode: domain.ThemeDark, paletteOff: false}, <-observed) | |||
| 259 | - | select { | |||
| 260 | - | case snapshot := <-invalid: | |||
| 261 | - | t.Fatalf("observed mixed theme configuration: %#v", snapshot) | |||
| 262 | - | default: | |||
| 263 | - | } | |||
| 264 | } | 216 | } | ||
| 265 | 217 | ||||
| 266 | func TestApplyConfigPublishesCopyConfig(t *testing.T) { | 218 | func TestApplyConfigPublishesCopyConfig(t *testing.T) { | ||
M internal/usecase/daemon/copy_mouse_input.go +3 -2
| @@ -59,8 +59,9 @@ func (d *Daemon) handleFreshCopyPress(sess *session, ac *attachedClient, tb *tab | |||
| 59 | 59 | if routed == nil { | |
| 60 | 60 | return false | |
| 61 | 61 | } | |
| 62 | + | cfg := d.currentRuntimeConfig() | |
| 62 | 63 | tb.mu.Lock() | |
| 63 | - | geometry, ok := hitTestCopyMouseGeometryLocked(tb, d.currentFloatingConfig(), ev.Col, ev.Row) | |
| 64 | + | geometry, ok := hitTestCopyMouseGeometryLocked(tb, cfg.floating, ev.Col, ev.Row) | |
| 64 | 65 | tb.mu.Unlock() | |
| 65 | 66 | if !ok || geometry.pane != routed { | |
| 66 | 67 | ac.overlays.copyMu.Lock() | |
| @@ -76,7 +77,7 @@ func (d *Daemon) handleFreshCopyPress(sess *session, ac *attachedClient, tb *tab | |||
| 76 | 77 | if mouseMode != 0 || altScreen { | |
| 77 | 78 | return false // child forwarding retains its existing raw-byte path. | |
| 78 | 79 | } | |
| 79 | - | document := scopy.NewDocument(snapshot, d.currentCopyConfig().WordSeparators) | |
| 80 | + | document := scopy.NewDocument(snapshot, cfg.copy.WordSeparators) | |
| 80 | 81 | mapped, ok := mapCopyMouse(ev, geometry, max(document.Len()-document.Height(), 0), document, false) | |
| 81 | 82 | if !ok { | |
| 82 | 83 | ac.overlays.copyMu.Lock() | |
| @@ -59,8 +59,9 @@ func (d *Daemon) handleFreshCopyPress(sess *session, ac *attachedClient, tb *tab | |||||
| 59 | if routed == nil { | 59 | if routed == nil { | ||
| 60 | return false | 60 | return false | ||
| 61 | } | 61 | } | ||
| 62 | + | cfg := d.currentRuntimeConfig() | |||
| 62 | tb.mu.Lock() | 63 | tb.mu.Lock() | ||
| 63 | - | geometry, ok := hitTestCopyMouseGeometryLocked(tb, d.currentFloatingConfig(), ev.Col, ev.Row) | 64 | + | geometry, ok := hitTestCopyMouseGeometryLocked(tb, cfg.floating, ev.Col, ev.Row) |
| 64 | tb.mu.Unlock() | 65 | tb.mu.Unlock() | ||
| 65 | if !ok || geometry.pane != routed { | 66 | if !ok || geometry.pane != routed { | ||
| 66 | ac.overlays.copyMu.Lock() | 67 | ac.overlays.copyMu.Lock() | ||
| @@ -76,7 +77,7 @@ func (d *Daemon) handleFreshCopyPress(sess *session, ac *attachedClient, tb *tab | |||||
| 76 | if mouseMode != 0 || altScreen { | 77 | if mouseMode != 0 || altScreen { | ||
| 77 | return false // child forwarding retains its existing raw-byte path. | 78 | return false // child forwarding retains its existing raw-byte path. | ||
| 78 | } | 79 | } | ||
| 79 | - | document := scopy.NewDocument(snapshot, d.currentCopyConfig().WordSeparators) | 80 | + | document := scopy.NewDocument(snapshot, cfg.copy.WordSeparators) |
| 80 | mapped, ok := mapCopyMouse(ev, geometry, max(document.Len()-document.Height(), 0), document, false) | 81 | mapped, ok := mapCopyMouse(ev, geometry, max(document.Len()-document.Height(), 0), document, false) | ||
| 81 | if !ok { | 82 | if !ok { | ||
| 82 | ac.overlays.copyMu.Lock() | 83 | ac.overlays.copyMu.Lock() | ||
M internal/usecase/daemon/render.go +3 -2
| @@ -445,8 +445,9 @@ func (d *Daemon) paintLocked(entry *session, ac *attachedClient, tb *tab, reset | |||
| 445 | 445 | repaintAttachedClients = sess.ackAttention(tb, ac, attentionVisible) | |
| 446 | 446 | } | |
| 447 | 447 | ||
| 448 | - | paletteCfg := d.currentPaletteConfig() | |
| 449 | - | floatingCfg := d.currentFloatingConfig() | |
| 448 | + | // One snapshot keeps palette and floating settings from the same reload. | |
| 449 | + | cfg := d.currentRuntimeConfig() | |
| 450 | + | paletteCfg, floatingCfg := cfg.palette, cfg.floating | |
| 450 | 451 | // Title refresh may inspect process state, so it remains before the capture | |
| 451 | 452 | // boundary and outside tab/pane ownership locks. | |
| 452 | 453 | if local { | |
| @@ -445,8 +445,9 @@ func (d *Daemon) paintLocked(entry *session, ac *attachedClient, tb *tab, reset | |||||
| 445 | repaintAttachedClients = sess.ackAttention(tb, ac, attentionVisible) | 445 | repaintAttachedClients = sess.ackAttention(tb, ac, attentionVisible) | ||
| 446 | } | 446 | } | ||
| 447 | 447 | ||||
| 448 | - | paletteCfg := d.currentPaletteConfig() | 448 | + | // One snapshot keeps palette and floating settings from the same reload. |
| 449 | - | floatingCfg := d.currentFloatingConfig() | 449 | + | cfg := d.currentRuntimeConfig() |
| 450 | + | paletteCfg, floatingCfg := cfg.palette, cfg.floating | |||
| 450 | // Title refresh may inspect process state, so it remains before the capture | 451 | // Title refresh may inspect process state, so it remains before the capture | ||
| 451 | // boundary and outside tab/pane ownership locks. | 452 | // boundary and outside tab/pane ownership locks. | ||
| 452 | if local { | 453 | if local { | ||