Merge pull request 'refactor(config): parse and watch the config with kvconf' (#23) from refactor/kvconf into main
Reviewed-on: https://git.bnema.dev/bnema/vev/pulls/23
8 files · +318 -297
M docs/configuration.md +2 -1
| @@ -3,7 +3,8 @@ | |||
| 3 | 3 | vev reads `~/.config/vev/config` (`$XDG_CONFIG_HOME` is respected). | |
| 4 | 4 | ||
| 5 | 5 | - No file means defaults. | |
| 6 | - | - Changes apply live within a couple of seconds. | |
| 6 | + | - Changes apply live as soon as the file is saved. | |
| 7 | + | - `#` starts a comment at the start of a line or after a space, except inside a quoted word. The file must be UTF-8 and at most 1 MiB; otherwise vev logs a warning and uses the defaults. | |
| 7 | 8 | - Exception: `web.listen` and `web.origin` need a gateway restart. See [browser terminal](web-terminal.md#listener-and-public-origin). | |
| 8 | 9 | - Invalid values log a warning and fall back to the default. | |
| 9 | 10 | ||
| @@ -3,7 +3,8 @@ | |||||
| 3 | vev reads `~/.config/vev/config` (`$XDG_CONFIG_HOME` is respected). | 3 | vev reads `~/.config/vev/config` (`$XDG_CONFIG_HOME` is respected). | ||
| 4 | 4 | ||||
| 5 | - No file means defaults. | 5 | - No file means defaults. | ||
| 6 | - | - Changes apply live within a couple of seconds. | 6 | + | - Changes apply live as soon as the file is saved. |
| 7 | + | - `#` starts a comment at the start of a line or after a space, except inside a quoted word. The file must be UTF-8 and at most 1 MiB; otherwise vev logs a warning and uses the defaults. | |||
| 7 | - Exception: `web.listen` and `web.origin` need a gateway restart. See [browser terminal](web-terminal.md#listener-and-public-origin). | 8 | - Exception: `web.listen` and `web.origin` need a gateway restart. See [browser terminal](web-terminal.md#listener-and-public-origin). | ||
| 8 | - Invalid values log a warning and fall back to the default. | 9 | - Invalid values log a warning and fall back to the default. | ||
| 9 | 10 | ||||
M go.mod +1 -0
| @@ -3,6 +3,7 @@ module github.com/bnema/vev | |||
| 3 | 3 | go 1.27 | |
| 4 | 4 | ||
| 5 | 5 | require ( | |
| 6 | + | github.com/bnema/kvconf v0.1.2 | |
| 6 | 7 | github.com/bnema/vev-vt v0.7.0 | |
| 7 | 8 | github.com/coder/websocket v1.8.15 | |
| 8 | 9 | github.com/quic-go/quic-go v0.62.0 | |
| @@ -3,6 +3,7 @@ module github.com/bnema/vev | |||||
| 3 | go 1.27 | 3 | go 1.27 | ||
| 4 | 4 | ||||
| 5 | require ( | 5 | require ( | ||
| 6 | + | github.com/bnema/kvconf v0.1.2 | |||
| 6 | github.com/bnema/vev-vt v0.7.0 | 7 | github.com/bnema/vev-vt v0.7.0 | ||
| 7 | github.com/coder/websocket v1.8.15 | 8 | github.com/coder/websocket v1.8.15 | ||
| 8 | github.com/quic-go/quic-go v0.62.0 | 9 | github.com/quic-go/quic-go v0.62.0 | ||
M go.sum +2 -0 generated file: collapsed
M internal/adapters/config/config.go +92 -105
| @@ -1,7 +1,7 @@ | |||
| 1 | 1 | package config | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | - | "bufio" | |
| 4 | + | "bytes" | |
| 5 | 5 | "context" | |
| 6 | 6 | "errors" | |
| 7 | 7 | "fmt" | |
| @@ -12,13 +12,11 @@ import ( | |||
| 12 | 12 | "strings" | |
| 13 | 13 | "time" | |
| 14 | 14 | ||
| 15 | + | "github.com/bnema/kvconf" | |
| 15 | 16 | "github.com/bnema/vev/internal/domain" | |
| 16 | 17 | "github.com/bnema/vev/internal/domain/terminalcap" | |
| 17 | - | "github.com/bnema/vev/internal/ports" | |
| 18 | 18 | ) | |
| 19 | 19 | ||
| 20 | - | const pollInterval = 2 * time.Second | |
| 21 | - | ||
| 22 | 20 | var ( | |
| 23 | 21 | processNamePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._+-]{0,127}$`) | |
| 24 | 22 | percentagePattern = regexp.MustCompile(`^[0-9]{1,3}%$`) | |
| @@ -43,29 +41,23 @@ func Parse(r io.Reader) (domain.Config, []domain.Warning, error) { | |||
| 43 | 41 | seenScrollbackKeys := make(map[string]bool) | |
| 44 | 42 | seenWebKeys := make(map[string]bool) | |
| 45 | 43 | ||
| 46 | - | scanner := bufio.NewScanner(r) | |
| 47 | - | lineNo := 0 | |
| 48 | - | for scanner.Scan() { | |
| 49 | - | lineNo++ | |
| 50 | - | line := strings.TrimSpace(strings.TrimSuffix(scanner.Text(), "\r")) | |
| 51 | - | if line == "" || strings.HasPrefix(line, "#") { | |
| 52 | - | continue | |
| 53 | - | } | |
| 54 | - | line = stripInlineComment(line) | |
| 55 | - | if line == "" { | |
| 56 | - | continue | |
| 57 | - | } | |
| 44 | + | entries, syntax, err := kvconf.Parse(r, parseOptions) | |
| 45 | + | if err != nil { | |
| 46 | + | return domain.Defaults(), nil, err | |
| 47 | + | } | |
| 48 | + | pending := syntaxWarnings(syntax) | |
| 58 | 49 | ||
| 59 | - | key, value, ok := strings.Cut(line, "=") | |
| 60 | - | if !ok { | |
| 61 | - | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: "missing '='"}) | |
| 62 | - | continue | |
| 50 | + | for _, entry := range entries { | |
| 51 | + | // Interleave line-syntax warnings with domain warnings in file order. | |
| 52 | + | for len(pending) > 0 && pending[0].Line < entry.Line { | |
| 53 | + | warnings = append(warnings, pending[0]) | |
| 54 | + | pending = pending[1:] | |
| 63 | 55 | } | |
| 64 | - | key = strings.TrimSpace(key) | |
| 65 | - | value = strings.TrimSpace(value) | |
| 66 | - | if key == "" { | |
| 67 | - | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: "missing key"}) | |
| 68 | - | continue | |
| 56 | + | lineNo, key, value := entry.Line, entry.Key, entry.Value | |
| 57 | + | // "key = # note" has an empty value: kvconf only strips a '#' that | |
| 58 | + | // follows other text. | |
| 59 | + | if strings.HasPrefix(value, "#") { | |
| 60 | + | value = "" | |
| 69 | 61 | } | |
| 70 | 62 | ||
| 71 | 63 | switch { | |
| @@ -278,84 +270,106 @@ func Parse(r io.Reader) (domain.Config, []domain.Warning, error) { | |||
| 278 | 270 | } | |
| 279 | 271 | } | |
| 280 | 272 | } | |
| 281 | - | if err := scanner.Err(); err != nil { | |
| 282 | - | return cfg, warnings, err | |
| 283 | - | } | |
| 273 | + | warnings = append(warnings, pending...) | |
| 284 | 274 | return cfg, warnings, nil | |
| 285 | 275 | } | |
| 286 | 276 | ||
| 277 | + | // parseOptions keeps quotes and duplicates in the kvconf entries: vev unquotes | |
| 278 | + | // per key and reports its own duplicate warnings. | |
| 279 | + | var parseOptions = kvconf.Options{RawValues: true, AllowDuplicates: true} | |
| 280 | + | ||
| 281 | + | // syntaxWarnings maps kvconf line-syntax warnings to domain warnings. | |
| 282 | + | func syntaxWarnings(in []kvconf.Warning) []domain.Warning { | |
| 283 | + | out := make([]domain.Warning, 0, len(in)) | |
| 284 | + | for _, w := range in { | |
| 285 | + | msg := w.Msg | |
| 286 | + | if msg == "expected key = value" { | |
| 287 | + | msg = "missing '='" | |
| 288 | + | } | |
| 289 | + | out = append(out, domain.Warning{Line: w.Line, Msg: msg}) | |
| 290 | + | } | |
| 291 | + | return out | |
| 292 | + | } | |
| 293 | + | ||
| 287 | 294 | // Load reads path and parses it. Missing config files are treated as defaults. | |
| 288 | 295 | func Load(path string) (domain.Config, []domain.Warning, error) { | |
| 289 | - | f, err := os.Open(path) | |
| 296 | + | data, err := kvconf.ReadFile(path, parseOptions) | |
| 290 | 297 | if errors.Is(err, os.ErrNotExist) { | |
| 291 | 298 | return domain.Defaults(), nil, nil | |
| 292 | 299 | } | |
| 293 | 300 | if err != nil { | |
| 294 | 301 | return domain.Defaults(), nil, err | |
| 295 | 302 | } | |
| 296 | - | cfg, warnings, parseErr := Parse(f) | |
| 297 | - | if closeErr := f.Close(); parseErr == nil && closeErr != nil { | |
| 298 | - | return cfg, warnings, closeErr | |
| 299 | - | } | |
| 300 | - | return cfg, warnings, parseErr | |
| 303 | + | return Parse(bytes.NewReader(data)) | |
| 301 | 304 | } | |
| 302 | 305 | ||
| 303 | - | // Watch polls path for mtime/size changes every two seconds and calls onChange | |
| 304 | - | // with each successfully loaded config. Missing files are allowed and reload to | |
| 305 | - | // defaults when the watched state changes. | |
| 306 | - | func Watch(ctx context.Context, clock ports.Clock, path string, onChange func(domain.Config, []domain.Warning)) error { | |
| 307 | - | last, err := fileStamp(path) | |
| 306 | + | // Watch follows path with native file notifications (kvconf.Watch) and calls | |
| 307 | + | // onChange with the config reloaded after each change. Missing files are | |
| 308 | + | // allowed and reload to defaults. A load failure reports defaults plus a | |
| 309 | + | // warning; the same failure is not reported twice in a row. Watch returns | |
| 310 | + | // ctx.Err() when ctx ends. | |
| 311 | + | func Watch(ctx context.Context, path string, onChange func(domain.Config, []domain.Warning)) error { | |
| 312 | + | return watch(ctx, path, kvconf.WatchOptions{}, onChange) | |
| 313 | + | } | |
| 314 | + | ||
| 315 | + | // fallbackPoll is the polling interval used when native watching fails. | |
| 316 | + | const fallbackPoll = 2 * time.Second | |
| 317 | + | ||
| 318 | + | // kvWatch is kvconf.Watch; tests replace it to make native watching fail. | |
| 319 | + | var kvWatch = kvconf.Watch | |
| 320 | + | ||
| 321 | + | func watch(ctx context.Context, path string, opts kvconf.WatchOptions, onChange func(domain.Config, []domain.Warning)) error { | |
| 308 | 322 | lastErrMsg := "" | |
| 309 | - | if err != nil { | |
| 310 | - | lastErrMsg = err.Error() | |
| 311 | - | onChange(domain.Defaults(), []domain.Warning{{Msg: lastErrMsg}}) | |
| 323 | + | reportErr := func(err error) { | |
| 324 | + | if msg := err.Error(); msg != lastErrMsg { | |
| 325 | + | lastErrMsg = msg | |
| 326 | + | onChange(domain.Defaults(), []domain.Warning{{Msg: msg}}) | |
| 327 | + | } | |
| 312 | 328 | } | |
| 313 | - | timer := clock.NewTimer(pollInterval) | |
| 314 | - | defer timer.Stop() | |
| 329 | + | if _, err := os.Stat(path); err != nil && !errors.Is(err, os.ErrNotExist) { | |
| 330 | + | reportErr(err) | |
| 331 | + | } | |
| 332 | + | ||
| 333 | + | changed := make(chan struct{}, 1) | |
| 334 | + | watchErr := make(chan error, 1) | |
| 335 | + | go func() { watchErr <- kvWatch(ctx, path, opts, changed) }() | |
| 315 | 336 | ||
| 316 | 337 | for { | |
| 317 | 338 | select { | |
| 318 | 339 | case <-ctx.Done(): | |
| 340 | + | <-watchErr | |
| 319 | 341 | return ctx.Err() | |
| 320 | - | case <-timer.C(): | |
| 321 | - | current, statErr := fileStamp(path) | |
| 322 | - | if statErr != nil { | |
| 323 | - | if statErr.Error() != lastErrMsg { | |
| 324 | - | lastErrMsg = statErr.Error() | |
| 325 | - | onChange(domain.Defaults(), []domain.Warning{{Msg: lastErrMsg}}) | |
| 326 | - | } | |
| 327 | - | } else if current != last || lastErrMsg != "" { | |
| 342 | + | case err := <-watchErr: | |
| 343 | + | if ctx.Err() != nil { | |
| 344 | + | return ctx.Err() | |
| 345 | + | } | |
| 346 | + | if err == nil { | |
| 347 | + | err = errors.New("config watcher stopped") | |
| 348 | + | } | |
| 349 | + | if opts.Poll > 0 { | |
| 350 | + | return err | |
| 351 | + | } | |
| 352 | + | // The OS ran out of watches (inotify limits): poll like before | |
| 353 | + | // rather than lose live reload until the daemon restarts. The | |
| 354 | + | // warning rides on the current config, which must not reset. | |
| 355 | + | opts.Poll = fallbackPoll | |
| 356 | + | go func() { watchErr <- kvWatch(ctx, path, opts, changed) }() | |
| 357 | + | if cfg, warnings, err2 := Load(path); err2 == nil { | |
| 328 | 358 | lastErrMsg = "" | |
| 329 | - | last = current | |
| 330 | - | cfg, warnings, err := Load(path) | |
| 331 | - | if err != nil { | |
| 332 | - | cfg = domain.Defaults() | |
| 333 | - | warnings = append(warnings, domain.Warning{Msg: err.Error()}) | |
| 334 | - | } | |
| 335 | - | onChange(cfg, warnings) | |
| 336 | - | } | |
| 337 | - | timer.Reset(pollInterval) | |
| 359 | + | onChange(cfg, append(warnings, domain.Warning{Msg: fmt.Sprintf("config watcher: %v; polling every %s", err, fallbackPoll)})) | |
| 360 | + | } | |
| 361 | + | case <-changed: | |
| 362 | + | cfg, warnings, err := Load(path) | |
| 363 | + | if err != nil { | |
| 364 | + | reportErr(err) | |
| 365 | + | continue | |
| 366 | + | } | |
| 367 | + | lastErrMsg = "" | |
| 368 | + | onChange(cfg, warnings) | |
| 338 | 369 | } | |
| 339 | 370 | } | |
| 340 | 371 | } | |
| 341 | 372 | ||
| 342 | - | type stamp struct { | |
| 343 | - | modTime time.Time | |
| 344 | - | size int64 | |
| 345 | - | exists bool | |
| 346 | - | } | |
| 347 | - | ||
| 348 | - | func fileStamp(path string) (stamp, error) { | |
| 349 | - | st, err := os.Stat(path) | |
| 350 | - | if errors.Is(err, os.ErrNotExist) { | |
| 351 | - | return stamp{}, nil | |
| 352 | - | } | |
| 353 | - | if err != nil { | |
| 354 | - | return stamp{}, err | |
| 355 | - | } | |
| 356 | - | return stamp{modTime: st.ModTime(), size: st.Size(), exists: true}, nil | |
| 357 | - | } | |
| 358 | - | ||
| 359 | 373 | func parseEchoPredict(value string) (domain.EchoPredictMode, bool) { | |
| 360 | 374 | switch strings.ToLower(value) { | |
| 361 | 375 | case "adaptive": | |
| @@ -474,30 +488,3 @@ func parseTheme(value string) (domain.ThemeMode, bool) { | |||
| 474 | 488 | return domain.ThemeAuto, false | |
| 475 | 489 | } | |
| 476 | 490 | } | |
| 477 | - | ||
| 478 | - | func stripInlineComment(line string) string { | |
| 479 | - | inQuote := false | |
| 480 | - | escaped := false | |
| 481 | - | for i, r := range line { | |
| 482 | - | if escaped { | |
| 483 | - | escaped = false | |
| 484 | - | continue | |
| 485 | - | } | |
| 486 | - | if inQuote && r == '\\' { | |
| 487 | - | escaped = true | |
| 488 | - | continue | |
| 489 | - | } | |
| 490 | - | if r == '"' { | |
| 491 | - | inQuote = !inQuote | |
| 492 | - | continue | |
| 493 | - | } | |
| 494 | - | if !inQuote && r == '#' && (i == 0 || isSpace(rune(line[i-1]))) { | |
| 495 | - | return strings.TrimSpace(line[:i]) | |
| 496 | - | } | |
| 497 | - | } | |
| 498 | - | return line | |
| 499 | - | } | |
| 500 | - | ||
| 501 | - | func isSpace(r rune) bool { | |
| 502 | - | return r == ' ' || r == '\t' || r == '\r' || r == '\n' | |
| 503 | - | } | |
| @@ -1,7 +1,7 @@ | |||||
| 1 | package config | 1 | package config | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | - | "bufio" | 4 | + | "bytes" |
| 5 | "context" | 5 | "context" | ||
| 6 | "errors" | 6 | "errors" | ||
| 7 | "fmt" | 7 | "fmt" | ||
| @@ -12,13 +12,11 @@ import ( | |||||
| 12 | "strings" | 12 | "strings" | ||
| 13 | "time" | 13 | "time" | ||
| 14 | 14 | ||||
| 15 | + | "github.com/bnema/kvconf" | |||
| 15 | "github.com/bnema/vev/internal/domain" | 16 | "github.com/bnema/vev/internal/domain" | ||
| 16 | "github.com/bnema/vev/internal/domain/terminalcap" | 17 | "github.com/bnema/vev/internal/domain/terminalcap" | ||
| 17 | - | "github.com/bnema/vev/internal/ports" | |||
| 18 | ) | 18 | ) | ||
| 19 | 19 | ||||
| 20 | - | const pollInterval = 2 * time.Second | |||
| 21 | - | ||||
| 22 | var ( | 20 | var ( | ||
| 23 | processNamePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._+-]{0,127}$`) | 21 | processNamePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._+-]{0,127}$`) | ||
| 24 | percentagePattern = regexp.MustCompile(`^[0-9]{1,3}%$`) | 22 | percentagePattern = regexp.MustCompile(`^[0-9]{1,3}%$`) | ||
| @@ -43,29 +41,23 @@ func Parse(r io.Reader) (domain.Config, []domain.Warning, error) { | |||||
| 43 | seenScrollbackKeys := make(map[string]bool) | 41 | seenScrollbackKeys := make(map[string]bool) | ||
| 44 | seenWebKeys := make(map[string]bool) | 42 | seenWebKeys := make(map[string]bool) | ||
| 45 | 43 | ||||
| 46 | - | scanner := bufio.NewScanner(r) | 44 | + | entries, syntax, err := kvconf.Parse(r, parseOptions) |
| 47 | - | lineNo := 0 | 45 | + | if err != nil { |
| 48 | - | for scanner.Scan() { | 46 | + | return domain.Defaults(), nil, err |
| 49 | - | lineNo++ | 47 | + | } |
| 50 | - | line := strings.TrimSpace(strings.TrimSuffix(scanner.Text(), "\r")) | 48 | + | pending := syntaxWarnings(syntax) |
| 51 | - | if line == "" || strings.HasPrefix(line, "#") { | |||
| 52 | - | continue | |||
| 53 | - | } | |||
| 54 | - | line = stripInlineComment(line) | |||
| 55 | - | if line == "" { | |||
| 56 | - | continue | |||
| 57 | - | } | |||
| 58 | 49 | ||||
| 59 | - | key, value, ok := strings.Cut(line, "=") | 50 | + | for _, entry := range entries { |
| 60 | - | if !ok { | 51 | + | // Interleave line-syntax warnings with domain warnings in file order. |
| 61 | - | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: "missing '='"}) | 52 | + | for len(pending) > 0 && pending[0].Line < entry.Line { |
| 62 | - | continue | 53 | + | warnings = append(warnings, pending[0]) |
| 54 | + | pending = pending[1:] | |||
| 63 | } | 55 | } | ||
| 64 | - | key = strings.TrimSpace(key) | 56 | + | lineNo, key, value := entry.Line, entry.Key, entry.Value |
| 65 | - | value = strings.TrimSpace(value) | 57 | + | // "key = # note" has an empty value: kvconf only strips a '#' that |
| 66 | - | if key == "" { | 58 | + | // follows other text. |
| 67 | - | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: "missing key"}) | 59 | + | if strings.HasPrefix(value, "#") { |
| 68 | - | continue | 60 | + | value = "" |
| 69 | } | 61 | } | ||
| 70 | 62 | ||||
| 71 | switch { | 63 | switch { | ||
| @@ -278,84 +270,106 @@ func Parse(r io.Reader) (domain.Config, []domain.Warning, error) { | |||||
| 278 | } | 270 | } | ||
| 279 | } | 271 | } | ||
| 280 | } | 272 | } | ||
| 281 | - | if err := scanner.Err(); err != nil { | 273 | + | warnings = append(warnings, pending...) |
| 282 | - | return cfg, warnings, err | |||
| 283 | - | } | |||
| 284 | return cfg, warnings, nil | 274 | return cfg, warnings, nil | ||
| 285 | } | 275 | } | ||
| 286 | 276 | ||||
| 277 | + | // parseOptions keeps quotes and duplicates in the kvconf entries: vev unquotes | |||
| 278 | + | // per key and reports its own duplicate warnings. | |||
| 279 | + | var parseOptions = kvconf.Options{RawValues: true, AllowDuplicates: true} | |||
| 280 | + | ||||
| 281 | + | // syntaxWarnings maps kvconf line-syntax warnings to domain warnings. | |||
| 282 | + | func syntaxWarnings(in []kvconf.Warning) []domain.Warning { | |||
| 283 | + | out := make([]domain.Warning, 0, len(in)) | |||
| 284 | + | for _, w := range in { | |||
| 285 | + | msg := w.Msg | |||
| 286 | + | if msg == "expected key = value" { | |||
| 287 | + | msg = "missing '='" | |||
| 288 | + | } | |||
| 289 | + | out = append(out, domain.Warning{Line: w.Line, Msg: msg}) | |||
| 290 | + | } | |||
| 291 | + | return out | |||
| 292 | + | } | |||
| 293 | + | ||||
| 287 | // Load reads path and parses it. Missing config files are treated as defaults. | 294 | // Load reads path and parses it. Missing config files are treated as defaults. | ||
| 288 | func Load(path string) (domain.Config, []domain.Warning, error) { | 295 | func Load(path string) (domain.Config, []domain.Warning, error) { | ||
| 289 | - | f, err := os.Open(path) | 296 | + | data, err := kvconf.ReadFile(path, parseOptions) |
| 290 | if errors.Is(err, os.ErrNotExist) { | 297 | if errors.Is(err, os.ErrNotExist) { | ||
| 291 | return domain.Defaults(), nil, nil | 298 | return domain.Defaults(), nil, nil | ||
| 292 | } | 299 | } | ||
| 293 | if err != nil { | 300 | if err != nil { | ||
| 294 | return domain.Defaults(), nil, err | 301 | return domain.Defaults(), nil, err | ||
| 295 | } | 302 | } | ||
| 296 | - | cfg, warnings, parseErr := Parse(f) | 303 | + | return Parse(bytes.NewReader(data)) |
| 297 | - | if closeErr := f.Close(); parseErr == nil && closeErr != nil { | |||
| 298 | - | return cfg, warnings, closeErr | |||
| 299 | - | } | |||
| 300 | - | return cfg, warnings, parseErr | |||
| 301 | } | 304 | } | ||
| 302 | 305 | ||||
| 303 | - | // Watch polls path for mtime/size changes every two seconds and calls onChange | 306 | + | // Watch follows path with native file notifications (kvconf.Watch) and calls |
| 304 | - | // with each successfully loaded config. Missing files are allowed and reload to | 307 | + | // onChange with the config reloaded after each change. Missing files are |
| 305 | - | // defaults when the watched state changes. | 308 | + | // allowed and reload to defaults. A load failure reports defaults plus a |
| 306 | - | func Watch(ctx context.Context, clock ports.Clock, path string, onChange func(domain.Config, []domain.Warning)) error { | 309 | + | // warning; the same failure is not reported twice in a row. Watch returns |
| 307 | - | last, err := fileStamp(path) | 310 | + | // ctx.Err() when ctx ends. |
| 311 | + | func Watch(ctx context.Context, path string, onChange func(domain.Config, []domain.Warning)) error { | |||
| 312 | + | return watch(ctx, path, kvconf.WatchOptions{}, onChange) | |||
| 313 | + | } | |||
| 314 | + | ||||
| 315 | + | // fallbackPoll is the polling interval used when native watching fails. | |||
| 316 | + | const fallbackPoll = 2 * time.Second | |||
| 317 | + | ||||
| 318 | + | // kvWatch is kvconf.Watch; tests replace it to make native watching fail. | |||
| 319 | + | var kvWatch = kvconf.Watch | |||
| 320 | + | ||||
| 321 | + | func watch(ctx context.Context, path string, opts kvconf.WatchOptions, onChange func(domain.Config, []domain.Warning)) error { | |||
| 308 | lastErrMsg := "" | 322 | lastErrMsg := "" | ||
| 309 | - | if err != nil { | 323 | + | reportErr := func(err error) { |
| 310 | - | lastErrMsg = err.Error() | 324 | + | if msg := err.Error(); msg != lastErrMsg { |
| 311 | - | onChange(domain.Defaults(), []domain.Warning{{Msg: lastErrMsg}}) | 325 | + | lastErrMsg = msg |
| 326 | + | onChange(domain.Defaults(), []domain.Warning{{Msg: msg}}) | |||
| 327 | + | } | |||
| 312 | } | 328 | } | ||
| 313 | - | timer := clock.NewTimer(pollInterval) | 329 | + | if _, err := os.Stat(path); err != nil && !errors.Is(err, os.ErrNotExist) { |
| 314 | - | defer timer.Stop() | 330 | + | reportErr(err) |
| 331 | + | } | |||
| 332 | + | ||||
| 333 | + | changed := make(chan struct{}, 1) | |||
| 334 | + | watchErr := make(chan error, 1) | |||
| 335 | + | go func() { watchErr <- kvWatch(ctx, path, opts, changed) }() | |||
| 315 | 336 | ||||
| 316 | for { | 337 | for { | ||
| 317 | select { | 338 | select { | ||
| 318 | case <-ctx.Done(): | 339 | case <-ctx.Done(): | ||
| 340 | + | <-watchErr | |||
| 319 | return ctx.Err() | 341 | return ctx.Err() | ||
| 320 | - | case <-timer.C(): | 342 | + | case err := <-watchErr: |
| 321 | - | current, statErr := fileStamp(path) | 343 | + | if ctx.Err() != nil { |
| 322 | - | if statErr != nil { | 344 | + | return ctx.Err() |
| 323 | - | if statErr.Error() != lastErrMsg { | 345 | + | } |
| 324 | - | lastErrMsg = statErr.Error() | 346 | + | if err == nil { |
| 325 | - | onChange(domain.Defaults(), []domain.Warning{{Msg: lastErrMsg}}) | 347 | + | err = errors.New("config watcher stopped") |
| 326 | - | } | 348 | + | } |
| 327 | - | } else if current != last || lastErrMsg != "" { | 349 | + | if opts.Poll > 0 { |
| 350 | + | return err | |||
| 351 | + | } | |||
| 352 | + | // The OS ran out of watches (inotify limits): poll like before | |||
| 353 | + | // rather than lose live reload until the daemon restarts. The | |||
| 354 | + | // warning rides on the current config, which must not reset. | |||
| 355 | + | opts.Poll = fallbackPoll | |||
| 356 | + | go func() { watchErr <- kvWatch(ctx, path, opts, changed) }() | |||
| 357 | + | if cfg, warnings, err2 := Load(path); err2 == nil { | |||
| 328 | lastErrMsg = "" | 358 | lastErrMsg = "" | ||
| 329 | - | last = current | 359 | + | onChange(cfg, append(warnings, domain.Warning{Msg: fmt.Sprintf("config watcher: %v; polling every %s", err, fallbackPoll)})) |
| 330 | - | cfg, warnings, err := Load(path) | 360 | + | } |
| 331 | - | if err != nil { | 361 | + | case <-changed: |
| 332 | - | cfg = domain.Defaults() | 362 | + | cfg, warnings, err := Load(path) |
| 333 | - | warnings = append(warnings, domain.Warning{Msg: err.Error()}) | 363 | + | if err != nil { |
| 334 | - | } | 364 | + | reportErr(err) |
| 335 | - | onChange(cfg, warnings) | 365 | + | continue |
| 336 | - | } | 366 | + | } |
| 337 | - | timer.Reset(pollInterval) | 367 | + | lastErrMsg = "" |
| 368 | + | onChange(cfg, warnings) | |||
| 338 | } | 369 | } | ||
| 339 | } | 370 | } | ||
| 340 | } | 371 | } | ||
| 341 | 372 | ||||
| 342 | - | type stamp struct { | |||
| 343 | - | modTime time.Time | |||
| 344 | - | size int64 | |||
| 345 | - | exists bool | |||
| 346 | - | } | |||
| 347 | - | ||||
| 348 | - | func fileStamp(path string) (stamp, error) { | |||
| 349 | - | st, err := os.Stat(path) | |||
| 350 | - | if errors.Is(err, os.ErrNotExist) { | |||
| 351 | - | return stamp{}, nil | |||
| 352 | - | } | |||
| 353 | - | if err != nil { | |||
| 354 | - | return stamp{}, err | |||
| 355 | - | } | |||
| 356 | - | return stamp{modTime: st.ModTime(), size: st.Size(), exists: true}, nil | |||
| 357 | - | } | |||
| 358 | - | ||||
| 359 | func parseEchoPredict(value string) (domain.EchoPredictMode, bool) { | 373 | func parseEchoPredict(value string) (domain.EchoPredictMode, bool) { | ||
| 360 | switch strings.ToLower(value) { | 374 | switch strings.ToLower(value) { | ||
| 361 | case "adaptive": | 375 | case "adaptive": | ||
| @@ -474,30 +488,3 @@ func parseTheme(value string) (domain.ThemeMode, bool) { | |||||
| 474 | return domain.ThemeAuto, false | 488 | return domain.ThemeAuto, false | ||
| 475 | } | 489 | } | ||
| 476 | } | 490 | } | ||
| 477 | - | ||||
| 478 | - | func stripInlineComment(line string) string { | |||
| 479 | - | inQuote := false | |||
| 480 | - | escaped := false | |||
| 481 | - | for i, r := range line { | |||
| 482 | - | if escaped { | |||
| 483 | - | escaped = false | |||
| 484 | - | continue | |||
| 485 | - | } | |||
| 486 | - | if inQuote && r == '\\' { | |||
| 487 | - | escaped = true | |||
| 488 | - | continue | |||
| 489 | - | } | |||
| 490 | - | if r == '"' { | |||
| 491 | - | inQuote = !inQuote | |||
| 492 | - | continue | |||
| 493 | - | } | |||
| 494 | - | if !inQuote && r == '#' && (i == 0 || isSpace(rune(line[i-1]))) { | |||
| 495 | - | return strings.TrimSpace(line[:i]) | |||
| 496 | - | } | |||
| 497 | - | } | |||
| 498 | - | return line | |||
| 499 | - | } | |||
| 500 | - | ||||
| 501 | - | func isSpace(r rune) bool { | |||
| 502 | - | return r == ' ' || r == '\t' || r == '\r' || r == '\n' | |||
| 503 | - | } | |||
M internal/adapters/config/config_test.go +208 -138
| @@ -7,11 +7,12 @@ import ( | |||
| 7 | 7 | "path/filepath" | |
| 8 | 8 | "reflect" | |
| 9 | 9 | "strings" | |
| 10 | + | "sync/atomic" | |
| 10 | 11 | "testing" | |
| 11 | 12 | "time" | |
| 12 | 13 | ||
| 14 | + | "github.com/bnema/kvconf" | |
| 13 | 15 | "github.com/bnema/vev/internal/domain" | |
| 14 | - | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |
| 15 | 16 | "github.com/stretchr/testify/require" | |
| 16 | 17 | ) | |
| 17 | 18 | ||
| @@ -124,6 +125,20 @@ func TestParse(t *testing.T) { | |||
| 124 | 125 | }, | |
| 125 | 126 | wantWarnings: []domain.Warning{{Line: 2, Msg: "duplicate key \"floating.command\""}}, | |
| 126 | 127 | }, | |
| 128 | + | { | |
| 129 | + | name: "a value that is only a comment is empty", | |
| 130 | + | input: "floating.command = echo ready\nfloating.command = # default shell\n", | |
| 131 | + | want: domain.Config{ | |
| 132 | + | Theme: domain.ThemeAuto, | |
| 133 | + | Floating: domain.FloatingConfig{ | |
| 134 | + | Width: 80, | |
| 135 | + | Height: 80, | |
| 136 | + | }, | |
| 137 | + | BindingEntries: []domain.ConfigEntry{}, | |
| 138 | + | Codes: map[string]string{}, | |
| 139 | + | }, | |
| 140 | + | wantWarnings: []domain.Warning{{Line: 2, Msg: "duplicate key \"floating.command\""}}, | |
| 141 | + | }, | |
| 127 | 142 | { | |
| 128 | 143 | name: "floating percentage endpoints", | |
| 129 | 144 | input: "floating.width = 1%\nfloating.height = 100%\n", | |
| @@ -833,179 +848,234 @@ func TestLoadMissingFileReturnsDefaults(t *testing.T) { | |||
| 833 | 848 | } | |
| 834 | 849 | } | |
| 835 | 850 | ||
| 836 | - | func TestWatchReloadsChangedFileWithFakeClock(t *testing.T) { | |
| 837 | - | t.Parallel() | |
| 838 | - | ||
| 839 | - | path := filepath.Join(t.TempDir(), "config") | |
| 840 | - | if err := os.WriteFile(path, []byte("theme = dark\n"), 0o600); err != nil { | |
| 841 | - | t.Fatal(err) | |
| 842 | - | } | |
| 851 | + | type watchChange struct { | |
| 852 | + | cfg domain.Config | |
| 853 | + | warnings []domain.Warning | |
| 854 | + | } | |
| 843 | 855 | ||
| 844 | - | clk := newWatchClockMock(t) | |
| 856 | + | // startWatch runs watch on path and returns the onChange stream once the | |
| 857 | + | // watches are installed. The returned stop cancels it and checks its result. | |
| 858 | + | func startWatch(t *testing.T, path string) (<-chan watchChange, func()) { | |
| 859 | + | t.Helper() | |
| 845 | 860 | ctx, cancel := context.WithCancel(context.Background()) | |
| 846 | - | defer cancel() | |
| 861 | + | t.Cleanup(cancel) | |
| 847 | 862 | ||
| 848 | - | type change struct { | |
| 849 | - | cfg domain.Config | |
| 850 | - | warnings []domain.Warning | |
| 851 | - | } | |
| 852 | - | changes := make(chan change, 2) | |
| 863 | + | changes := make(chan watchChange, 8) | |
| 864 | + | ready := make(chan struct{}) | |
| 853 | 865 | done := make(chan error, 1) | |
| 854 | 866 | go func() { | |
| 855 | - | done <- Watch(ctx, clk.clock, path, func(cfg domain.Config, warnings []domain.Warning) { | |
| 856 | - | changes <- change{cfg: cfg, warnings: warnings} | |
| 857 | - | }) | |
| 867 | + | done <- watch(ctx, path, kvconf.WatchOptions{Debounce: 10 * time.Millisecond, Ready: ready}, | |
| 868 | + | func(cfg domain.Config, warnings []domain.Warning) { | |
| 869 | + | changes <- watchChange{cfg: cfg, warnings: warnings} | |
| 870 | + | }) | |
| 858 | 871 | }() | |
| 859 | - | ||
| 860 | - | clk.waitForTimer() | |
| 861 | - | if err := os.WriteFile(path, []byte("theme = light\nbad line\n"), 0o600); err != nil { | |
| 862 | - | t.Fatal(err) | |
| 863 | - | } | |
| 864 | - | clk.fire() | |
| 865 | - | ||
| 866 | - | var got change | |
| 867 | 872 | select { | |
| 868 | - | case got = <-changes: | |
| 869 | - | case <-time.After(2 * time.Second): | |
| 870 | - | t.Fatal("timed out waiting for config reload") | |
| 871 | - | } | |
| 872 | - | if got.cfg.Theme != domain.ThemeLight { | |
| 873 | - | t.Fatalf("Watch() theme = %v, want %v", got.cfg.Theme, domain.ThemeLight) | |
| 873 | + | case <-ready: | |
| 874 | + | case err := <-done: | |
| 875 | + | t.Fatalf("watch() returned before ready: %v", err) | |
| 876 | + | case <-time.After(5 * time.Second): | |
| 877 | + | t.Fatal("timed out waiting for the watcher to be ready") | |
| 874 | 878 | } | |
| 875 | - | if !reflect.DeepEqual(got.warnings, []domain.Warning{{Line: 2, Msg: "missing '='"}}) { | |
| 876 | - | t.Fatalf("Watch() warnings = %#v", got.warnings) | |
| 879 | + | return changes, func() { | |
| 880 | + | t.Helper() | |
| 881 | + | cancel() | |
| 882 | + | select { | |
| 883 | + | case err := <-done: | |
| 884 | + | require.ErrorIs(t, err, context.Canceled) | |
| 885 | + | case <-time.After(5 * time.Second): | |
| 886 | + | t.Fatal("timed out waiting for watch() to return") | |
| 887 | + | } | |
| 877 | 888 | } | |
| 889 | + | } | |
| 878 | 890 | ||
| 879 | - | cancel() | |
| 880 | - | if err := <-done; !errors.Is(err, context.Canceled) { | |
| 881 | - | t.Fatalf("Watch() error = %v, want context.Canceled", err) | |
| 891 | + | // writeAtomic replaces path in one step, like most editors: the watcher | |
| 892 | + | // never sees a truncated file, whatever the scheduling. | |
| 893 | + | func writeAtomic(t *testing.T, path string, data []byte) { | |
| 894 | + | t.Helper() | |
| 895 | + | tmp := path + ".tmp" | |
| 896 | + | require.NoError(t, os.WriteFile(tmp, data, 0o600)) | |
| 897 | + | require.NoError(t, os.Rename(tmp, path)) | |
| 898 | + | } | |
| 899 | + | ||
| 900 | + | func nextChange(t *testing.T, changes <-chan watchChange) watchChange { | |
| 901 | + | t.Helper() | |
| 902 | + | select { | |
| 903 | + | case got := <-changes: | |
| 904 | + | return got | |
| 905 | + | case <-time.After(5 * time.Second): | |
| 906 | + | t.Fatal("timed out waiting for config reload") | |
| 907 | + | return watchChange{} | |
| 882 | 908 | } | |
| 883 | 909 | } | |
| 884 | 910 | ||
| 885 | - | func TestWatchUsesDefaultsWhenReloadLoadFails(t *testing.T) { | |
| 911 | + | func TestWatchReloadsOnChange(t *testing.T) { | |
| 886 | 912 | t.Parallel() | |
| 887 | 913 | ||
| 888 | - | path := filepath.Join(t.TempDir(), "config") | |
| 889 | - | if err := os.WriteFile(path, []byte("theme = dark\n"), 0o600); err != nil { | |
| 890 | - | t.Fatal(err) | |
| 891 | - | } | |
| 892 | - | ||
| 893 | - | clk := newWatchClockMock(t) | |
| 894 | - | ctx, cancel := context.WithCancel(context.Background()) | |
| 895 | - | defer cancel() | |
| 914 | + | light := domain.Defaults() | |
| 915 | + | light.Theme = domain.ThemeLight | |
| 896 | 916 | ||
| 897 | - | type change struct { | |
| 898 | - | cfg domain.Config | |
| 899 | - | warnings []domain.Warning | |
| 917 | + | tests := []struct { | |
| 918 | + | name string | |
| 919 | + | initial *string // nil: the file does not exist yet | |
| 920 | + | write func(t *testing.T, path string) | |
| 921 | + | wantConfig domain.Config | |
| 922 | + | wantWarnings []domain.Warning | |
| 923 | + | wantAnyWarn bool | |
| 924 | + | }{ | |
| 925 | + | { | |
| 926 | + | name: "changed file reloads with parse warnings", | |
| 927 | + | initial: ptr("theme = dark\n"), | |
| 928 | + | write: func(t *testing.T, path string) { | |
| 929 | + | writeAtomic(t, path, []byte("theme = light\nbad line\n")) | |
| 930 | + | }, | |
| 931 | + | wantConfig: light, | |
| 932 | + | wantWarnings: []domain.Warning{{Line: 2, Msg: "missing '='"}}, | |
| 933 | + | }, | |
| 934 | + | { | |
| 935 | + | name: "created file loads", | |
| 936 | + | initial: nil, | |
| 937 | + | write: func(t *testing.T, path string) { | |
| 938 | + | writeAtomic(t, path, []byte("theme = light\n")) | |
| 939 | + | }, | |
| 940 | + | wantConfig: light, | |
| 941 | + | }, | |
| 942 | + | { | |
| 943 | + | name: "deleted file reloads to defaults", | |
| 944 | + | initial: ptr("theme = light\n"), | |
| 945 | + | write: func(t *testing.T, path string) { | |
| 946 | + | require.NoError(t, os.Remove(path)) | |
| 947 | + | }, | |
| 948 | + | wantConfig: domain.Defaults(), | |
| 949 | + | }, | |
| 950 | + | { | |
| 951 | + | name: "load failure reports defaults and a warning", | |
| 952 | + | initial: ptr("theme = light\n"), | |
| 953 | + | write: func(t *testing.T, path string) { | |
| 954 | + | // Larger than kvconf.DefaultMaxBytes. | |
| 955 | + | writeAtomic(t, path, []byte(strings.Repeat("# x\n", kvconf.DefaultMaxBytes/4+1))) | |
| 956 | + | }, | |
| 957 | + | wantConfig: domain.Defaults(), | |
| 958 | + | wantAnyWarn: true, | |
| 959 | + | }, | |
| 960 | + | { | |
| 961 | + | name: "invalid UTF-8 reports defaults and a warning", | |
| 962 | + | initial: ptr("theme = light\n"), | |
| 963 | + | write: func(t *testing.T, path string) { | |
| 964 | + | writeAtomic(t, path, []byte("theme = \xff\n")) | |
| 965 | + | }, | |
| 966 | + | wantConfig: domain.Defaults(), | |
| 967 | + | wantAnyWarn: true, | |
| 968 | + | }, | |
| 900 | 969 | } | |
| 901 | - | changes := make(chan change, 2) | |
| 902 | - | done := make(chan error, 1) | |
| 903 | - | go func() { | |
| 904 | - | done <- Watch(ctx, clk.clock, path, func(cfg domain.Config, warnings []domain.Warning) { | |
| 905 | - | changes <- change{cfg: cfg, warnings: warnings} | |
| 906 | - | }) | |
| 907 | - | }() | |
| 970 | + | for _, tt := range tests { | |
| 971 | + | t.Run(tt.name, func(t *testing.T) { | |
| 972 | + | t.Parallel() | |
| 908 | 973 | ||
| 909 | - | clk.waitForTimer() | |
| 910 | - | tooLong := strings.Repeat("x", 70*1024) | |
| 911 | - | if err := os.WriteFile(path, []byte(tooLong), 0o600); err != nil { | |
| 912 | - | t.Fatal(err) | |
| 974 | + | path := filepath.Join(t.TempDir(), "config") | |
| 975 | + | if tt.initial != nil { | |
| 976 | + | writeAtomic(t, path, []byte(*tt.initial)) | |
| 977 | + | } | |
| 978 | + | changes, stop := startWatch(t, path) | |
| 979 | + | tt.write(t, path) | |
| 980 | + | ||
| 981 | + | got := nextChange(t, changes) | |
| 982 | + | require.Equal(t, tt.wantConfig, got.cfg) | |
| 983 | + | if tt.wantAnyWarn { | |
| 984 | + | require.NotEmpty(t, got.warnings) | |
| 985 | + | } else { | |
| 986 | + | require.Equal(t, tt.wantWarnings, got.warnings) | |
| 987 | + | } | |
| 988 | + | stop() | |
| 989 | + | }) | |
| 913 | 990 | } | |
| 914 | - | clk.fire() | |
| 991 | + | } | |
| 915 | 992 | ||
| 916 | - | got := <-changes | |
| 917 | - | require.Equal(t, domain.Defaults(), got.cfg) | |
| 918 | - | require.NotEmpty(t, got.warnings) | |
| 993 | + | // A load error is reported once until a good load, then again. | |
| 994 | + | func TestWatchReportsLoadErrorOnce(t *testing.T) { | |
| 995 | + | t.Parallel() | |
| 919 | 996 | ||
| 920 | - | cancel() | |
| 921 | - | if err := <-done; !errors.Is(err, context.Canceled) { | |
| 922 | - | t.Fatalf("Watch() error = %v, want context.Canceled", err) | |
| 923 | - | } | |
| 997 | + | path := filepath.Join(t.TempDir(), "config") | |
| 998 | + | writeAtomic(t, path, []byte("theme = light\n")) | |
| 999 | + | changes, stop := startWatch(t, path) | |
| 1000 | + | invalid := []byte("theme = \xff\n") | |
| 1001 | + | ||
| 1002 | + | writeAtomic(t, path, invalid) | |
| 1003 | + | got := nextChange(t, changes) | |
| 1004 | + | require.Equal(t, domain.Defaults(), got.cfg) | |
| 1005 | + | require.Len(t, got.warnings, 1) | |
| 1006 | + | ||
| 1007 | + | // The same failure again is not reported; the next good load is. | |
| 1008 | + | writeAtomic(t, path, invalid) | |
| 1009 | + | writeAtomic(t, path, []byte("theme = dark\n")) | |
| 1010 | + | got = nextChange(t, changes) | |
| 1011 | + | require.Equal(t, domain.ThemeDark, got.cfg.Theme) | |
| 1012 | + | require.Empty(t, got.warnings) | |
| 1013 | + | ||
| 1014 | + | // After a good load, the failure is news again. | |
| 1015 | + | writeAtomic(t, path, invalid) | |
| 1016 | + | got = nextChange(t, changes) | |
| 1017 | + | require.Len(t, got.warnings, 1) | |
| 1018 | + | stop() | |
| 924 | 1019 | } | |
| 925 | 1020 | ||
| 926 | - | func TestWatchDoesNotRepeatPersistentStatError(t *testing.T) { | |
| 1021 | + | // A stat error at startup other than a missing file is reported. | |
| 1022 | + | func TestWatchReportsStartupStatError(t *testing.T) { | |
| 927 | 1023 | t.Parallel() | |
| 928 | 1024 | ||
| 1025 | + | // A name longer than NAME_MAX makes every stat fail. | |
| 929 | 1026 | path := filepath.Join(t.TempDir(), strings.Repeat("x", 5000)) | |
| 930 | - | clk := newWatchClockMock(t) | |
| 931 | 1027 | ctx, cancel := context.WithCancel(context.Background()) | |
| 932 | 1028 | defer cancel() | |
| 933 | - | ||
| 934 | - | changes := make(chan []domain.Warning, 4) | |
| 1029 | + | changes := make(chan watchChange, 4) | |
| 935 | 1030 | done := make(chan error, 1) | |
| 936 | 1031 | go func() { | |
| 937 | - | done <- Watch(ctx, clk.clock, path, func(_ domain.Config, warnings []domain.Warning) { | |
| 938 | - | changes <- warnings | |
| 1032 | + | done <- watch(ctx, path, kvconf.WatchOptions{Poll: time.Hour}, func(cfg domain.Config, warnings []domain.Warning) { | |
| 1033 | + | changes <- watchChange{cfg: cfg, warnings: warnings} | |
| 939 | 1034 | }) | |
| 940 | 1035 | }() | |
| 941 | - | ||
| 942 | - | first := <-changes | |
| 943 | - | require.Len(t, first, 1) | |
| 944 | - | clk.waitForTimer() | |
| 945 | - | clk.fire() | |
| 946 | - | clk.fire() | |
| 947 | - | ||
| 948 | - | select { | |
| 949 | - | case extra := <-changes: | |
| 950 | - | t.Fatalf("Watch() repeated persistent stat error: %#v", extra) | |
| 951 | - | case <-time.After(100 * time.Millisecond): | |
| 952 | - | } | |
| 953 | - | ||
| 1036 | + | got := nextChange(t, changes) | |
| 1037 | + | require.Equal(t, domain.Defaults(), got.cfg) | |
| 1038 | + | require.Len(t, got.warnings, 1) | |
| 954 | 1039 | cancel() | |
| 955 | - | if err := <-done; !errors.Is(err, context.Canceled) { | |
| 956 | - | t.Fatalf("Watch() error = %v, want context.Canceled", err) | |
| 957 | - | } | |
| 958 | - | } | |
| 959 | - | ||
| 960 | - | type watchClockMock struct { | |
| 961 | - | t *testing.T | |
| 962 | - | clock *portsmocks.MockClock | |
| 963 | - | ticks chan time.Time | |
| 964 | - | ready chan struct{} | |
| 965 | - | resets chan struct{} | |
| 966 | - | } | |
| 967 | - | ||
| 968 | - | func newWatchClockMock(t *testing.T) *watchClockMock { | |
| 969 | - | t.Helper() | |
| 970 | - | timer := portsmocks.NewMockTimer(t) | |
| 971 | - | clock := portsmocks.NewMockClock(t) | |
| 972 | - | m := &watchClockMock{ | |
| 973 | - | t: t, | |
| 974 | - | clock: clock, | |
| 975 | - | ticks: make(chan time.Time), | |
| 976 | - | ready: make(chan struct{}), | |
| 977 | - | resets: make(chan struct{}), | |
| 978 | - | } | |
| 979 | - | timer.EXPECT().C().Return(m.ticks) | |
| 980 | - | timer.EXPECT().Reset(pollInterval).Run(func(time.Duration) { | |
| 981 | - | m.resets <- struct{}{} | |
| 982 | - | }).Return(false) | |
| 983 | - | timer.EXPECT().Stop().Return(true) | |
| 984 | - | clock.EXPECT().NewTimer(pollInterval).Run(func(time.Duration) { | |
| 985 | - | close(m.ready) | |
| 986 | - | }).Return(timer).Once() | |
| 987 | - | return m | |
| 988 | - | } | |
| 989 | - | ||
| 990 | - | func (m *watchClockMock) waitForTimer() { | |
| 991 | - | m.t.Helper() | |
| 992 | 1040 | select { | |
| 993 | - | case <-m.ready: | |
| 994 | - | case <-time.After(time.Second): | |
| 995 | - | m.t.Fatal("timed out waiting for watcher timer creation") | |
| 1041 | + | case <-done: | |
| 1042 | + | case <-time.After(5 * time.Second): | |
| 1043 | + | t.Fatal("timed out waiting for watch() to return") | |
| 996 | 1044 | } | |
| 997 | 1045 | } | |
| 998 | 1046 | ||
| 999 | - | func (m *watchClockMock) fire() { | |
| 1000 | - | m.t.Helper() | |
| 1001 | - | select { | |
| 1002 | - | case m.ticks <- time.Now(): | |
| 1003 | - | case <-time.After(time.Second): | |
| 1004 | - | m.t.Fatal("timed out firing watcher timer") | |
| 1005 | - | } | |
| 1006 | - | select { | |
| 1007 | - | case <-m.resets: | |
| 1008 | - | case <-time.After(time.Second): | |
| 1009 | - | m.t.Fatal("timed out waiting for watcher timer reset") | |
| 1047 | + | func ptr[T any](v T) *T { return &v } | |
| 1048 | + | ||
| 1049 | + | // When native watching fails (inotify limits), watch keeps the current | |
| 1050 | + | // config, warns once and polls instead of giving up. | |
| 1051 | + | // Not parallel: it replaces kvWatch. | |
| 1052 | + | func TestWatchFallsBackToPolling(t *testing.T) { | |
| 1053 | + | path := filepath.Join(t.TempDir(), "config") | |
| 1054 | + | writeAtomic(t, path, []byte("theme = light\n")) | |
| 1055 | + | var polled atomic.Bool | |
| 1056 | + | orig := kvWatch | |
| 1057 | + | t.Cleanup(func() { kvWatch = orig }) | |
| 1058 | + | kvWatch = func(ctx context.Context, path string, opts kvconf.WatchOptions, out chan<- struct{}) error { | |
| 1059 | + | if opts.Poll == 0 { | |
| 1060 | + | return errors.New("too many open files") | |
| 1061 | + | } | |
| 1062 | + | polled.Store(true) | |
| 1063 | + | <-ctx.Done() | |
| 1064 | + | return ctx.Err() | |
| 1010 | 1065 | } | |
| 1066 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 1067 | + | changes := make(chan watchChange, 4) | |
| 1068 | + | done := make(chan error, 1) | |
| 1069 | + | go func() { | |
| 1070 | + | done <- watch(ctx, path, kvconf.WatchOptions{}, func(cfg domain.Config, warnings []domain.Warning) { | |
| 1071 | + | changes <- watchChange{cfg: cfg, warnings: warnings} | |
| 1072 | + | }) | |
| 1073 | + | }() | |
| 1074 | + | got := nextChange(t, changes) | |
| 1075 | + | require.Equal(t, domain.ThemeLight, got.cfg.Theme, "the current config is kept") | |
| 1076 | + | require.Len(t, got.warnings, 1) | |
| 1077 | + | require.Contains(t, got.warnings[0].Msg, "polling every") | |
| 1078 | + | cancel() | |
| 1079 | + | require.ErrorIs(t, <-done, context.Canceled) | |
| 1080 | + | require.True(t, polled.Load()) | |
| 1011 | 1081 | } | |
| @@ -7,11 +7,12 @@ import ( | |||||
| 7 | "path/filepath" | 7 | "path/filepath" | ||
| 8 | "reflect" | 8 | "reflect" | ||
| 9 | "strings" | 9 | "strings" | ||
| 10 | + | "sync/atomic" | |||
| 10 | "testing" | 11 | "testing" | ||
| 11 | "time" | 12 | "time" | ||
| 12 | 13 | ||||
| 14 | + | "github.com/bnema/kvconf" | |||
| 13 | "github.com/bnema/vev/internal/domain" | 15 | "github.com/bnema/vev/internal/domain" | ||
| 14 | - | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |||
| 15 | "github.com/stretchr/testify/require" | 16 | "github.com/stretchr/testify/require" | ||
| 16 | ) | 17 | ) | ||
| 17 | 18 | ||||
| @@ -124,6 +125,20 @@ func TestParse(t *testing.T) { | |||||
| 124 | }, | 125 | }, | ||
| 125 | wantWarnings: []domain.Warning{{Line: 2, Msg: "duplicate key \"floating.command\""}}, | 126 | wantWarnings: []domain.Warning{{Line: 2, Msg: "duplicate key \"floating.command\""}}, | ||
| 126 | }, | 127 | }, | ||
| 128 | + | { | |||
| 129 | + | name: "a value that is only a comment is empty", | |||
| 130 | + | input: "floating.command = echo ready\nfloating.command = # default shell\n", | |||
| 131 | + | want: domain.Config{ | |||
| 132 | + | Theme: domain.ThemeAuto, | |||
| 133 | + | Floating: domain.FloatingConfig{ | |||
| 134 | + | Width: 80, | |||
| 135 | + | Height: 80, | |||
| 136 | + | }, | |||
| 137 | + | BindingEntries: []domain.ConfigEntry{}, | |||
| 138 | + | Codes: map[string]string{}, | |||
| 139 | + | }, | |||
| 140 | + | wantWarnings: []domain.Warning{{Line: 2, Msg: "duplicate key \"floating.command\""}}, | |||
| 141 | + | }, | |||
| 127 | { | 142 | { | ||
| 128 | name: "floating percentage endpoints", | 143 | name: "floating percentage endpoints", | ||
| 129 | input: "floating.width = 1%\nfloating.height = 100%\n", | 144 | input: "floating.width = 1%\nfloating.height = 100%\n", | ||
| @@ -833,179 +848,234 @@ func TestLoadMissingFileReturnsDefaults(t *testing.T) { | |||||
| 833 | } | 848 | } | ||
| 834 | } | 849 | } | ||
| 835 | 850 | ||||
| 836 | - | func TestWatchReloadsChangedFileWithFakeClock(t *testing.T) { | 851 | + | type watchChange struct { |
| 837 | - | t.Parallel() | 852 | + | cfg domain.Config |
| 838 | - | 853 | + | warnings []domain.Warning | |
| 839 | - | path := filepath.Join(t.TempDir(), "config") | 854 | + | } |
| 840 | - | if err := os.WriteFile(path, []byte("theme = dark\n"), 0o600); err != nil { | |||
| 841 | - | t.Fatal(err) | |||
| 842 | - | } | |||
| 843 | 855 | ||||
| 844 | - | clk := newWatchClockMock(t) | 856 | + | // startWatch runs watch on path and returns the onChange stream once the |
| 857 | + | // watches are installed. The returned stop cancels it and checks its result. | |||
| 858 | + | func startWatch(t *testing.T, path string) (<-chan watchChange, func()) { | |||
| 859 | + | t.Helper() | |||
| 845 | ctx, cancel := context.WithCancel(context.Background()) | 860 | ctx, cancel := context.WithCancel(context.Background()) | ||
| 846 | - | defer cancel() | 861 | + | t.Cleanup(cancel) |
| 847 | 862 | ||||
| 848 | - | type change struct { | 863 | + | changes := make(chan watchChange, 8) |
| 849 | - | cfg domain.Config | 864 | + | ready := make(chan struct{}) |
| 850 | - | warnings []domain.Warning | |||
| 851 | - | } | |||
| 852 | - | changes := make(chan change, 2) | |||
| 853 | done := make(chan error, 1) | 865 | done := make(chan error, 1) | ||
| 854 | go func() { | 866 | go func() { | ||
| 855 | - | done <- Watch(ctx, clk.clock, path, func(cfg domain.Config, warnings []domain.Warning) { | 867 | + | done <- watch(ctx, path, kvconf.WatchOptions{Debounce: 10 * time.Millisecond, Ready: ready}, |
| 856 | - | changes <- change{cfg: cfg, warnings: warnings} | 868 | + | func(cfg domain.Config, warnings []domain.Warning) { |
| 857 | - | }) | 869 | + | changes <- watchChange{cfg: cfg, warnings: warnings} |
| 870 | + | }) | |||
| 858 | }() | 871 | }() | ||
| 859 | - | ||||
| 860 | - | clk.waitForTimer() | |||
| 861 | - | if err := os.WriteFile(path, []byte("theme = light\nbad line\n"), 0o600); err != nil { | |||
| 862 | - | t.Fatal(err) | |||
| 863 | - | } | |||
| 864 | - | clk.fire() | |||
| 865 | - | ||||
| 866 | - | var got change | |||
| 867 | select { | 872 | select { | ||
| 868 | - | case got = <-changes: | 873 | + | case <-ready: |
| 869 | - | case <-time.After(2 * time.Second): | 874 | + | case err := <-done: |
| 870 | - | t.Fatal("timed out waiting for config reload") | 875 | + | t.Fatalf("watch() returned before ready: %v", err) |
| 871 | - | } | 876 | + | case <-time.After(5 * time.Second): |
| 872 | - | if got.cfg.Theme != domain.ThemeLight { | 877 | + | t.Fatal("timed out waiting for the watcher to be ready") |
| 873 | - | t.Fatalf("Watch() theme = %v, want %v", got.cfg.Theme, domain.ThemeLight) | |||
| 874 | } | 878 | } | ||
| 875 | - | if !reflect.DeepEqual(got.warnings, []domain.Warning{{Line: 2, Msg: "missing '='"}}) { | 879 | + | return changes, func() { |
| 876 | - | t.Fatalf("Watch() warnings = %#v", got.warnings) | 880 | + | t.Helper() |
| 881 | + | cancel() | |||
| 882 | + | select { | |||
| 883 | + | case err := <-done: | |||
| 884 | + | require.ErrorIs(t, err, context.Canceled) | |||
| 885 | + | case <-time.After(5 * time.Second): | |||
| 886 | + | t.Fatal("timed out waiting for watch() to return") | |||
| 887 | + | } | |||
| 877 | } | 888 | } | ||
| 889 | + | } | |||
| 878 | 890 | ||||
| 879 | - | cancel() | 891 | + | // writeAtomic replaces path in one step, like most editors: the watcher |
| 880 | - | if err := <-done; !errors.Is(err, context.Canceled) { | 892 | + | // never sees a truncated file, whatever the scheduling. |
| 881 | - | t.Fatalf("Watch() error = %v, want context.Canceled", err) | 893 | + | func writeAtomic(t *testing.T, path string, data []byte) { |
| 894 | + | t.Helper() | |||
| 895 | + | tmp := path + ".tmp" | |||
| 896 | + | require.NoError(t, os.WriteFile(tmp, data, 0o600)) | |||
| 897 | + | require.NoError(t, os.Rename(tmp, path)) | |||
| 898 | + | } | |||
| 899 | + | ||||
| 900 | + | func nextChange(t *testing.T, changes <-chan watchChange) watchChange { | |||
| 901 | + | t.Helper() | |||
| 902 | + | select { | |||
| 903 | + | case got := <-changes: | |||
| 904 | + | return got | |||
| 905 | + | case <-time.After(5 * time.Second): | |||
| 906 | + | t.Fatal("timed out waiting for config reload") | |||
| 907 | + | return watchChange{} | |||
| 882 | } | 908 | } | ||
| 883 | } | 909 | } | ||
| 884 | 910 | ||||
| 885 | - | func TestWatchUsesDefaultsWhenReloadLoadFails(t *testing.T) { | 911 | + | func TestWatchReloadsOnChange(t *testing.T) { |
| 886 | t.Parallel() | 912 | t.Parallel() | ||
| 887 | 913 | ||||
| 888 | - | path := filepath.Join(t.TempDir(), "config") | 914 | + | light := domain.Defaults() |
| 889 | - | if err := os.WriteFile(path, []byte("theme = dark\n"), 0o600); err != nil { | 915 | + | light.Theme = domain.ThemeLight |
| 890 | - | t.Fatal(err) | |||
| 891 | - | } | |||
| 892 | - | ||||
| 893 | - | clk := newWatchClockMock(t) | |||
| 894 | - | ctx, cancel := context.WithCancel(context.Background()) | |||
| 895 | - | defer cancel() | |||
| 896 | 916 | ||||
| 897 | - | type change struct { | 917 | + | tests := []struct { |
| 898 | - | cfg domain.Config | 918 | + | name string |
| 899 | - | warnings []domain.Warning | 919 | + | initial *string // nil: the file does not exist yet |
| 920 | + | write func(t *testing.T, path string) | |||
| 921 | + | wantConfig domain.Config | |||
| 922 | + | wantWarnings []domain.Warning | |||
| 923 | + | wantAnyWarn bool | |||
| 924 | + | }{ | |||
| 925 | + | { | |||
| 926 | + | name: "changed file reloads with parse warnings", | |||
| 927 | + | initial: ptr("theme = dark\n"), | |||
| 928 | + | write: func(t *testing.T, path string) { | |||
| 929 | + | writeAtomic(t, path, []byte("theme = light\nbad line\n")) | |||
| 930 | + | }, | |||
| 931 | + | wantConfig: light, | |||
| 932 | + | wantWarnings: []domain.Warning{{Line: 2, Msg: "missing '='"}}, | |||
| 933 | + | }, | |||
| 934 | + | { | |||
| 935 | + | name: "created file loads", | |||
| 936 | + | initial: nil, | |||
| 937 | + | write: func(t *testing.T, path string) { | |||
| 938 | + | writeAtomic(t, path, []byte("theme = light\n")) | |||
| 939 | + | }, | |||
| 940 | + | wantConfig: light, | |||
| 941 | + | }, | |||
| 942 | + | { | |||
| 943 | + | name: "deleted file reloads to defaults", | |||
| 944 | + | initial: ptr("theme = light\n"), | |||
| 945 | + | write: func(t *testing.T, path string) { | |||
| 946 | + | require.NoError(t, os.Remove(path)) | |||
| 947 | + | }, | |||
| 948 | + | wantConfig: domain.Defaults(), | |||
| 949 | + | }, | |||
| 950 | + | { | |||
| 951 | + | name: "load failure reports defaults and a warning", | |||
| 952 | + | initial: ptr("theme = light\n"), | |||
| 953 | + | write: func(t *testing.T, path string) { | |||
| 954 | + | // Larger than kvconf.DefaultMaxBytes. | |||
| 955 | + | writeAtomic(t, path, []byte(strings.Repeat("# x\n", kvconf.DefaultMaxBytes/4+1))) | |||
| 956 | + | }, | |||
| 957 | + | wantConfig: domain.Defaults(), | |||
| 958 | + | wantAnyWarn: true, | |||
| 959 | + | }, | |||
| 960 | + | { | |||
| 961 | + | name: "invalid UTF-8 reports defaults and a warning", | |||
| 962 | + | initial: ptr("theme = light\n"), | |||
| 963 | + | write: func(t *testing.T, path string) { | |||
| 964 | + | writeAtomic(t, path, []byte("theme = \xff\n")) | |||
| 965 | + | }, | |||
| 966 | + | wantConfig: domain.Defaults(), | |||
| 967 | + | wantAnyWarn: true, | |||
| 968 | + | }, | |||
| 900 | } | 969 | } | ||
| 901 | - | changes := make(chan change, 2) | 970 | + | for _, tt := range tests { |
| 902 | - | done := make(chan error, 1) | 971 | + | t.Run(tt.name, func(t *testing.T) { |
| 903 | - | go func() { | 972 | + | t.Parallel() |
| 904 | - | done <- Watch(ctx, clk.clock, path, func(cfg domain.Config, warnings []domain.Warning) { | |||
| 905 | - | changes <- change{cfg: cfg, warnings: warnings} | |||
| 906 | - | }) | |||
| 907 | - | }() | |||
| 908 | 973 | ||||
| 909 | - | clk.waitForTimer() | 974 | + | path := filepath.Join(t.TempDir(), "config") |
| 910 | - | tooLong := strings.Repeat("x", 70*1024) | 975 | + | if tt.initial != nil { |
| 911 | - | if err := os.WriteFile(path, []byte(tooLong), 0o600); err != nil { | 976 | + | writeAtomic(t, path, []byte(*tt.initial)) |
| 912 | - | t.Fatal(err) | 977 | + | } |
| 978 | + | changes, stop := startWatch(t, path) | |||
| 979 | + | tt.write(t, path) | |||
| 980 | + | ||||
| 981 | + | got := nextChange(t, changes) | |||
| 982 | + | require.Equal(t, tt.wantConfig, got.cfg) | |||
| 983 | + | if tt.wantAnyWarn { | |||
| 984 | + | require.NotEmpty(t, got.warnings) | |||
| 985 | + | } else { | |||
| 986 | + | require.Equal(t, tt.wantWarnings, got.warnings) | |||
| 987 | + | } | |||
| 988 | + | stop() | |||
| 989 | + | }) | |||
| 913 | } | 990 | } | ||
| 914 | - | clk.fire() | 991 | + | } |
| 915 | 992 | ||||
| 916 | - | got := <-changes | 993 | + | // A load error is reported once until a good load, then again. |
| 917 | - | require.Equal(t, domain.Defaults(), got.cfg) | 994 | + | func TestWatchReportsLoadErrorOnce(t *testing.T) { |
| 918 | - | require.NotEmpty(t, got.warnings) | 995 | + | t.Parallel() |
| 919 | 996 | ||||
| 920 | - | cancel() | 997 | + | path := filepath.Join(t.TempDir(), "config") |
| 921 | - | if err := <-done; !errors.Is(err, context.Canceled) { | 998 | + | writeAtomic(t, path, []byte("theme = light\n")) |
| 922 | - | t.Fatalf("Watch() error = %v, want context.Canceled", err) | 999 | + | changes, stop := startWatch(t, path) |
| 923 | - | } | 1000 | + | invalid := []byte("theme = \xff\n") |
| 1001 | + | ||||
| 1002 | + | writeAtomic(t, path, invalid) | |||
| 1003 | + | got := nextChange(t, changes) | |||
| 1004 | + | require.Equal(t, domain.Defaults(), got.cfg) | |||
| 1005 | + | require.Len(t, got.warnings, 1) | |||
| 1006 | + | ||||
| 1007 | + | // The same failure again is not reported; the next good load is. | |||
| 1008 | + | writeAtomic(t, path, invalid) | |||
| 1009 | + | writeAtomic(t, path, []byte("theme = dark\n")) | |||
| 1010 | + | got = nextChange(t, changes) | |||
| 1011 | + | require.Equal(t, domain.ThemeDark, got.cfg.Theme) | |||
| 1012 | + | require.Empty(t, got.warnings) | |||
| 1013 | + | ||||
| 1014 | + | // After a good load, the failure is news again. | |||
| 1015 | + | writeAtomic(t, path, invalid) | |||
| 1016 | + | got = nextChange(t, changes) | |||
| 1017 | + | require.Len(t, got.warnings, 1) | |||
| 1018 | + | stop() | |||
| 924 | } | 1019 | } | ||
| 925 | 1020 | ||||
| 926 | - | func TestWatchDoesNotRepeatPersistentStatError(t *testing.T) { | 1021 | + | // A stat error at startup other than a missing file is reported. |
| 1022 | + | func TestWatchReportsStartupStatError(t *testing.T) { | |||
| 927 | t.Parallel() | 1023 | t.Parallel() | ||
| 928 | 1024 | ||||
| 1025 | + | // A name longer than NAME_MAX makes every stat fail. | |||
| 929 | path := filepath.Join(t.TempDir(), strings.Repeat("x", 5000)) | 1026 | path := filepath.Join(t.TempDir(), strings.Repeat("x", 5000)) | ||
| 930 | - | clk := newWatchClockMock(t) | |||
| 931 | ctx, cancel := context.WithCancel(context.Background()) | 1027 | ctx, cancel := context.WithCancel(context.Background()) | ||
| 932 | defer cancel() | 1028 | defer cancel() | ||
| 933 | - | 1029 | + | changes := make(chan watchChange, 4) | |
| 934 | - | changes := make(chan []domain.Warning, 4) | |||
| 935 | done := make(chan error, 1) | 1030 | done := make(chan error, 1) | ||
| 936 | go func() { | 1031 | go func() { | ||
| 937 | - | done <- Watch(ctx, clk.clock, path, func(_ domain.Config, warnings []domain.Warning) { | 1032 | + | done <- watch(ctx, path, kvconf.WatchOptions{Poll: time.Hour}, func(cfg domain.Config, warnings []domain.Warning) { |
| 938 | - | changes <- warnings | 1033 | + | changes <- watchChange{cfg: cfg, warnings: warnings} |
| 939 | }) | 1034 | }) | ||
| 940 | }() | 1035 | }() | ||
| 941 | - | 1036 | + | got := nextChange(t, changes) | |
| 942 | - | first := <-changes | 1037 | + | require.Equal(t, domain.Defaults(), got.cfg) |
| 943 | - | require.Len(t, first, 1) | 1038 | + | require.Len(t, got.warnings, 1) |
| 944 | - | clk.waitForTimer() | |||
| 945 | - | clk.fire() | |||
| 946 | - | clk.fire() | |||
| 947 | - | ||||
| 948 | - | select { | |||
| 949 | - | case extra := <-changes: | |||
| 950 | - | t.Fatalf("Watch() repeated persistent stat error: %#v", extra) | |||
| 951 | - | case <-time.After(100 * time.Millisecond): | |||
| 952 | - | } | |||
| 953 | - | ||||
| 954 | cancel() | 1039 | cancel() | ||
| 955 | - | if err := <-done; !errors.Is(err, context.Canceled) { | |||
| 956 | - | t.Fatalf("Watch() error = %v, want context.Canceled", err) | |||
| 957 | - | } | |||
| 958 | - | } | |||
| 959 | - | ||||
| 960 | - | type watchClockMock struct { | |||
| 961 | - | t *testing.T | |||
| 962 | - | clock *portsmocks.MockClock | |||
| 963 | - | ticks chan time.Time | |||
| 964 | - | ready chan struct{} | |||
| 965 | - | resets chan struct{} | |||
| 966 | - | } | |||
| 967 | - | ||||
| 968 | - | func newWatchClockMock(t *testing.T) *watchClockMock { | |||
| 969 | - | t.Helper() | |||
| 970 | - | timer := portsmocks.NewMockTimer(t) | |||
| 971 | - | clock := portsmocks.NewMockClock(t) | |||
| 972 | - | m := &watchClockMock{ | |||
| 973 | - | t: t, | |||
| 974 | - | clock: clock, | |||
| 975 | - | ticks: make(chan time.Time), | |||
| 976 | - | ready: make(chan struct{}), | |||
| 977 | - | resets: make(chan struct{}), | |||
| 978 | - | } | |||
| 979 | - | timer.EXPECT().C().Return(m.ticks) | |||
| 980 | - | timer.EXPECT().Reset(pollInterval).Run(func(time.Duration) { | |||
| 981 | - | m.resets <- struct{}{} | |||
| 982 | - | }).Return(false) | |||
| 983 | - | timer.EXPECT().Stop().Return(true) | |||
| 984 | - | clock.EXPECT().NewTimer(pollInterval).Run(func(time.Duration) { | |||
| 985 | - | close(m.ready) | |||
| 986 | - | }).Return(timer).Once() | |||
| 987 | - | return m | |||
| 988 | - | } | |||
| 989 | - | ||||
| 990 | - | func (m *watchClockMock) waitForTimer() { | |||
| 991 | - | m.t.Helper() | |||
| 992 | select { | 1040 | select { | ||
| 993 | - | case <-m.ready: | 1041 | + | case <-done: |
| 994 | - | case <-time.After(time.Second): | 1042 | + | case <-time.After(5 * time.Second): |
| 995 | - | m.t.Fatal("timed out waiting for watcher timer creation") | 1043 | + | t.Fatal("timed out waiting for watch() to return") |
| 996 | } | 1044 | } | ||
| 997 | } | 1045 | } | ||
| 998 | 1046 | ||||
| 999 | - | func (m *watchClockMock) fire() { | 1047 | + | func ptr[T any](v T) *T { return &v } |
| 1000 | - | m.t.Helper() | 1048 | + | |
| 1001 | - | select { | 1049 | + | // When native watching fails (inotify limits), watch keeps the current |
| 1002 | - | case m.ticks <- time.Now(): | 1050 | + | // config, warns once and polls instead of giving up. |
| 1003 | - | case <-time.After(time.Second): | 1051 | + | // Not parallel: it replaces kvWatch. |
| 1004 | - | m.t.Fatal("timed out firing watcher timer") | 1052 | + | func TestWatchFallsBackToPolling(t *testing.T) { |
| 1005 | - | } | 1053 | + | path := filepath.Join(t.TempDir(), "config") |
| 1006 | - | select { | 1054 | + | writeAtomic(t, path, []byte("theme = light\n")) |
| 1007 | - | case <-m.resets: | 1055 | + | var polled atomic.Bool |
| 1008 | - | case <-time.After(time.Second): | 1056 | + | orig := kvWatch |
| 1009 | - | m.t.Fatal("timed out waiting for watcher timer reset") | 1057 | + | t.Cleanup(func() { kvWatch = orig }) |
| 1058 | + | kvWatch = func(ctx context.Context, path string, opts kvconf.WatchOptions, out chan<- struct{}) error { | |||
| 1059 | + | if opts.Poll == 0 { | |||
| 1060 | + | return errors.New("too many open files") | |||
| 1061 | + | } | |||
| 1062 | + | polled.Store(true) | |||
| 1063 | + | <-ctx.Done() | |||
| 1064 | + | return ctx.Err() | |||
| 1010 | } | 1065 | } | ||
| 1066 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 1067 | + | changes := make(chan watchChange, 4) | |||
| 1068 | + | done := make(chan error, 1) | |||
| 1069 | + | go func() { | |||
| 1070 | + | done <- watch(ctx, path, kvconf.WatchOptions{}, func(cfg domain.Config, warnings []domain.Warning) { | |||
| 1071 | + | changes <- watchChange{cfg: cfg, warnings: warnings} | |||
| 1072 | + | }) | |||
| 1073 | + | }() | |||
| 1074 | + | got := nextChange(t, changes) | |||
| 1075 | + | require.Equal(t, domain.ThemeLight, got.cfg.Theme, "the current config is kept") | |||
| 1076 | + | require.Len(t, got.warnings, 1) | |||
| 1077 | + | require.Contains(t, got.warnings[0].Msg, "polling every") | |||
| 1078 | + | cancel() | |||
| 1079 | + | require.ErrorIs(t, <-done, context.Canceled) | |||
| 1080 | + | require.True(t, polled.Load()) | |||
| 1011 | } | 1081 | } | ||
M internal/adapters/config/palette_anchor_test.go +7 -17
| @@ -1,7 +1,6 @@ | |||
| 1 | 1 | package config | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | - | "context" | |
| 5 | 4 | "os" | |
| 6 | 5 | "path/filepath" | |
| 7 | 6 | "strings" | |
| @@ -47,26 +46,17 @@ func TestLoadMissingPaletteAnchorDefaultsToCenter(t *testing.T) { | |||
| 47 | 46 | } | |
| 48 | 47 | ||
| 49 | 48 | func TestWatchPaletteAnchorRestoresCenteredDefaultAfterFileRemoval(t *testing.T) { | |
| 49 | + | t.Parallel() | |
| 50 | + | ||
| 50 | 51 | path := filepath.Join(t.TempDir(), "config") | |
| 51 | 52 | require.NoError(t, os.WriteFile(path, []byte("palette.anchor = top-right\n"), 0o600)) | |
| 52 | - | clk := newWatchClockMock(t) | |
| 53 | - | ctx, cancel := context.WithCancel(context.Background()) | |
| 54 | - | defer cancel() | |
| 55 | - | changes := make(chan domain.Config, 2) | |
| 56 | - | done := make(chan error, 1) | |
| 57 | - | go func() { | |
| 58 | - | done <- Watch(ctx, clk.clock, path, func(cfg domain.Config, _ []domain.Warning) { changes <- cfg }) | |
| 59 | - | }() | |
| 60 | - | clk.waitForTimer() | |
| 53 | + | changes, stop := startWatch(t, path) | |
| 61 | 54 | ||
| 62 | 55 | require.NoError(t, os.WriteFile(path, []byte("palette.anchor = bottom-left\n"), 0o600)) | |
| 63 | - | clk.fire() | |
| 64 | - | require.Equal(t, domain.AnchorBottomLeft, (<-changes).Palette.Anchor) | |
| 56 | + | require.Equal(t, domain.AnchorBottomLeft, nextChange(t, changes).cfg.Palette.Anchor) | |
| 65 | 57 | ||
| 66 | 58 | require.NoError(t, os.Remove(path)) | |
| 67 | - | clk.fire() | |
| 68 | - | got := <-changes | |
| 69 | - | require.Equal(t, domain.PaletteConfig{Anchor: domain.AnchorCenter, AnchorSet: true}, got.Palette) | |
| 70 | - | cancel() | |
| 71 | - | require.ErrorIs(t, <-done, context.Canceled) | |
| 59 | + | got := nextChange(t, changes) | |
| 60 | + | require.Equal(t, domain.PaletteConfig{Anchor: domain.AnchorCenter, AnchorSet: true}, got.cfg.Palette) | |
| 61 | + | stop() | |
| 72 | 62 | } | |
| @@ -1,7 +1,6 @@ | |||||
| 1 | package config | 1 | package config | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | - | "context" | |||
| 5 | "os" | 4 | "os" | ||
| 6 | "path/filepath" | 5 | "path/filepath" | ||
| 7 | "strings" | 6 | "strings" | ||
| @@ -47,26 +46,17 @@ func TestLoadMissingPaletteAnchorDefaultsToCenter(t *testing.T) { | |||||
| 47 | } | 46 | } | ||
| 48 | 47 | ||||
| 49 | func TestWatchPaletteAnchorRestoresCenteredDefaultAfterFileRemoval(t *testing.T) { | 48 | func TestWatchPaletteAnchorRestoresCenteredDefaultAfterFileRemoval(t *testing.T) { | ||
| 49 | + | t.Parallel() | |||
| 50 | + | ||||
| 50 | path := filepath.Join(t.TempDir(), "config") | 51 | path := filepath.Join(t.TempDir(), "config") | ||
| 51 | require.NoError(t, os.WriteFile(path, []byte("palette.anchor = top-right\n"), 0o600)) | 52 | require.NoError(t, os.WriteFile(path, []byte("palette.anchor = top-right\n"), 0o600)) | ||
| 52 | - | clk := newWatchClockMock(t) | 53 | + | changes, stop := startWatch(t, path) |
| 53 | - | ctx, cancel := context.WithCancel(context.Background()) | |||
| 54 | - | defer cancel() | |||
| 55 | - | changes := make(chan domain.Config, 2) | |||
| 56 | - | done := make(chan error, 1) | |||
| 57 | - | go func() { | |||
| 58 | - | done <- Watch(ctx, clk.clock, path, func(cfg domain.Config, _ []domain.Warning) { changes <- cfg }) | |||
| 59 | - | }() | |||
| 60 | - | clk.waitForTimer() | |||
| 61 | 54 | ||||
| 62 | require.NoError(t, os.WriteFile(path, []byte("palette.anchor = bottom-left\n"), 0o600)) | 55 | require.NoError(t, os.WriteFile(path, []byte("palette.anchor = bottom-left\n"), 0o600)) | ||
| 63 | - | clk.fire() | 56 | + | require.Equal(t, domain.AnchorBottomLeft, nextChange(t, changes).cfg.Palette.Anchor) |
| 64 | - | require.Equal(t, domain.AnchorBottomLeft, (<-changes).Palette.Anchor) | |||
| 65 | 57 | ||||
| 66 | require.NoError(t, os.Remove(path)) | 58 | require.NoError(t, os.Remove(path)) | ||
| 67 | - | clk.fire() | 59 | + | got := nextChange(t, changes) |
| 68 | - | got := <-changes | 60 | + | require.Equal(t, domain.PaletteConfig{Anchor: domain.AnchorCenter, AnchorSet: true}, got.cfg.Palette) |
| 69 | - | require.Equal(t, domain.PaletteConfig{Anchor: domain.AnchorCenter, AnchorSet: true}, got.Palette) | 61 | + | stop() |
| 70 | - | cancel() | |||
| 71 | - | require.ErrorIs(t, <-done, context.Canceled) | |||
| 72 | } | 62 | } | ||
M internal/adapters/config/theme_accent_test.go +5 -35
| @@ -1,13 +1,10 @@ | |||
| 1 | 1 | package config | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | - | "context" | |
| 5 | - | "errors" | |
| 6 | 4 | "os" | |
| 7 | 5 | "path/filepath" | |
| 8 | 6 | "strings" | |
| 9 | 7 | "testing" | |
| 10 | - | "time" | |
| 11 | 8 | ||
| 12 | 9 | "github.com/bnema/vev/internal/domain" | |
| 13 | 10 | "github.com/stretchr/testify/require" | |
| @@ -118,38 +115,11 @@ func TestWatchThemeAccentInvalidReloadFallsBackToAuto(t *testing.T) { | |||
| 118 | 115 | require.Empty(t, warnings) | |
| 119 | 116 | require.Equal(t, domain.ThemeAccent{Mode: domain.ThemeAccentSlot, Slot: 2}, initial.ThemeAccent) | |
| 120 | 117 | ||
| 121 | - | clk := newWatchClockMock(t) | |
| 122 | - | ctx, cancel := context.WithCancel(context.Background()) | |
| 123 | - | defer cancel() | |
| 124 | - | type change struct { | |
| 125 | - | cfg domain.Config | |
| 126 | - | warnings []domain.Warning | |
| 127 | - | } | |
| 128 | - | changes := make(chan change, 1) | |
| 129 | - | done := make(chan error, 1) | |
| 130 | - | go func() { | |
| 131 | - | done <- Watch(ctx, clk.clock, path, func(cfg domain.Config, warnings []domain.Warning) { | |
| 132 | - | changes <- change{cfg: cfg, warnings: warnings} | |
| 133 | - | }) | |
| 134 | - | }() | |
| 135 | - | ||
| 136 | - | clk.waitForTimer() | |
| 118 | + | changes, stop := startWatch(t, path) | |
| 137 | 119 | require.NoError(t, os.WriteFile(path, []byte("theme.accent = invalid\n"), 0o600)) | |
| 138 | - | clk.fire() | |
| 139 | 120 | ||
| 140 | - | select { | |
| 141 | - | case got := <-changes: | |
| 142 | - | require.Equal(t, domain.ThemeAccent{Mode: domain.ThemeAccentAuto}, got.cfg.ThemeAccent) | |
| 143 | - | require.Equal(t, []domain.Warning{{Line: 1, Msg: "invalid theme.accent \"invalid\""}}, got.warnings) | |
| 144 | - | case <-time.After(2 * time.Second): | |
| 145 | - | t.Fatal("timed out waiting for config reload") | |
| 146 | - | } | |
| 147 | - | ||
| 148 | - | cancel() | |
| 149 | - | select { | |
| 150 | - | case err := <-done: | |
| 151 | - | require.True(t, errors.Is(err, context.Canceled)) | |
| 152 | - | case <-time.After(2 * time.Second): | |
| 153 | - | t.Fatal("timed out waiting for config watcher shutdown") | |
| 154 | - | } | |
| 121 | + | got := nextChange(t, changes) | |
| 122 | + | require.Equal(t, domain.ThemeAccent{Mode: domain.ThemeAccentAuto}, got.cfg.ThemeAccent) | |
| 123 | + | require.Equal(t, []domain.Warning{{Line: 1, Msg: "invalid theme.accent \"invalid\""}}, got.warnings) | |
| 124 | + | stop() | |
| 155 | 125 | } | |
| @@ -1,13 +1,10 @@ | |||||
| 1 | package config | 1 | package config | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | - | "context" | |||
| 5 | - | "errors" | |||
| 6 | "os" | 4 | "os" | ||
| 7 | "path/filepath" | 5 | "path/filepath" | ||
| 8 | "strings" | 6 | "strings" | ||
| 9 | "testing" | 7 | "testing" | ||
| 10 | - | "time" | |||
| 11 | 8 | ||||
| 12 | "github.com/bnema/vev/internal/domain" | 9 | "github.com/bnema/vev/internal/domain" | ||
| 13 | "github.com/stretchr/testify/require" | 10 | "github.com/stretchr/testify/require" | ||
| @@ -118,38 +115,11 @@ func TestWatchThemeAccentInvalidReloadFallsBackToAuto(t *testing.T) { | |||||
| 118 | require.Empty(t, warnings) | 115 | require.Empty(t, warnings) | ||
| 119 | require.Equal(t, domain.ThemeAccent{Mode: domain.ThemeAccentSlot, Slot: 2}, initial.ThemeAccent) | 116 | require.Equal(t, domain.ThemeAccent{Mode: domain.ThemeAccentSlot, Slot: 2}, initial.ThemeAccent) | ||
| 120 | 117 | ||||
| 121 | - | clk := newWatchClockMock(t) | 118 | + | changes, stop := startWatch(t, path) |
| 122 | - | ctx, cancel := context.WithCancel(context.Background()) | |||
| 123 | - | defer cancel() | |||
| 124 | - | type change struct { | |||
| 125 | - | cfg domain.Config | |||
| 126 | - | warnings []domain.Warning | |||
| 127 | - | } | |||
| 128 | - | changes := make(chan change, 1) | |||
| 129 | - | done := make(chan error, 1) | |||
| 130 | - | go func() { | |||
| 131 | - | done <- Watch(ctx, clk.clock, path, func(cfg domain.Config, warnings []domain.Warning) { | |||
| 132 | - | changes <- change{cfg: cfg, warnings: warnings} | |||
| 133 | - | }) | |||
| 134 | - | }() | |||
| 135 | - | ||||
| 136 | - | clk.waitForTimer() | |||
| 137 | require.NoError(t, os.WriteFile(path, []byte("theme.accent = invalid\n"), 0o600)) | 119 | require.NoError(t, os.WriteFile(path, []byte("theme.accent = invalid\n"), 0o600)) | ||
| 138 | - | clk.fire() | |||
| 139 | 120 | ||||
| 140 | - | select { | 121 | + | got := nextChange(t, changes) |
| 141 | - | case got := <-changes: | 122 | + | require.Equal(t, domain.ThemeAccent{Mode: domain.ThemeAccentAuto}, got.cfg.ThemeAccent) |
| 142 | - | require.Equal(t, domain.ThemeAccent{Mode: domain.ThemeAccentAuto}, got.cfg.ThemeAccent) | 123 | + | require.Equal(t, []domain.Warning{{Line: 1, Msg: "invalid theme.accent \"invalid\""}}, got.warnings) |
| 143 | - | require.Equal(t, []domain.Warning{{Line: 1, Msg: "invalid theme.accent \"invalid\""}}, got.warnings) | 124 | + | stop() |
| 144 | - | case <-time.After(2 * time.Second): | |||
| 145 | - | t.Fatal("timed out waiting for config reload") | |||
| 146 | - | } | |||
| 147 | - | ||||
| 148 | - | cancel() | |||
| 149 | - | select { | |||
| 150 | - | case err := <-done: | |||
| 151 | - | require.True(t, errors.Is(err, context.Canceled)) | |||
| 152 | - | case <-time.After(2 * time.Second): | |||
| 153 | - | t.Fatal("timed out waiting for config watcher shutdown") | |||
| 154 | - | } | |||
| 155 | } | 125 | } | ||
M internal/app/daemon_run.go +1 -1
| @@ -300,7 +300,7 @@ func runDaemonOwnedWithLogger(ctx context.Context, log *slog.Logger) (retErr err | |||
| 300 | 300 | watchCtx, stopWatch := context.WithCancel(ctx) | |
| 301 | 301 | defer stopWatch() | |
| 302 | 302 | go func() { | |
| 303 | - | if err := config.Watch(watchCtx, clk, configPath, func(cfg domain.Config, warnings []domain.Warning) { | |
| 303 | + | if err := config.Watch(watchCtx, configPath, func(cfg domain.Config, warnings []domain.Warning) { | |
| 304 | 304 | logConfigWarnings(log, warnings) | |
| 305 | 305 | d.ApplyConfig(cfg) | |
| 306 | 306 | }); err != nil && !errors.Is(err, context.Canceled) { | |
| @@ -300,7 +300,7 @@ func runDaemonOwnedWithLogger(ctx context.Context, log *slog.Logger) (retErr err | |||||
| 300 | watchCtx, stopWatch := context.WithCancel(ctx) | 300 | watchCtx, stopWatch := context.WithCancel(ctx) | ||
| 301 | defer stopWatch() | 301 | defer stopWatch() | ||
| 302 | go func() { | 302 | go func() { | ||
| 303 | - | if err := config.Watch(watchCtx, clk, configPath, func(cfg domain.Config, warnings []domain.Warning) { | 303 | + | if err := config.Watch(watchCtx, configPath, func(cfg domain.Config, warnings []domain.Warning) { |
| 304 | logConfigWarnings(log, warnings) | 304 | logConfigWarnings(log, warnings) | ||
| 305 | d.ApplyConfig(cfg) | 305 | d.ApplyConfig(cfg) | ||
| 306 | }); err != nil && !errors.Is(err, context.Canceled) { | 306 | }); err != nil && !errors.Is(err, context.Canceled) { | ||